lkml.org 
[lkml]   [2019]   [Sep]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v6 01/24] erofs: add on-disk layout
Hi!

> > +struct erofs_super_block {
> > +/* 0 */__le32 magic; /* in the little endian */
> > +/* 4 */__le32 checksum; /* crc32c(super_block) */
> > +/* 8 */__le32 features; /* (aka. feature_compat) */
> > +/* 12 */__u8 blkszbits; /* support block_size == PAGE_SIZE only */
>
> Please remove all the byte offset comments. That is something that can
> easily be checked with gdb or pahole.

I don't think I agree. gdb will tell you byte offsets _on one
architecture_. But filesystem is supposed to be portable between them.

> > +/* 64 */__u8 volume_name[16]; /* volume name */
> > +/* 80 */__le32 requirements; /* (aka. feature_incompat) */
> > +
> > +/* 84 */__u8 reserved2[44];
> > +} __packed; /* 128 bytes */
>
> Please don't add __packed. In this case I think you don't need it
> (but double check with pahole), but even if you would need it using
> proper padding fields and making sure all fields are naturally aligned
> will give you much better code generation on architectures that don't
> support native unaligned access.

This is on-disk structure, right?

drivers/staging/erofs/super.c: struct erofs_super_block *layout;
drivers/staging/erofs/super.c: layout = (struct erofs_super_block
*)((u8 *)bh->b_data

So __packed is right thing to do. If architecture accesses that
slowly, that's ungood, but different structures between architectures
would be really bad.

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2019-09-02 10:40    [W:0.139 / U:6.752 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site