Re: [PATCH] (and bad attr2 bug) - pack xfs_sb_t for 64-bit arches

[Russell Cattelan <cattelan@thebarn.com>]

David Chinner wrote:

>
>@@ -135,9 +136,12 @@
>        __uint8_t       sb_shared_vn;   /* shared version number */
>        xfs_extlen_t    sb_inoalignmt;  /* inode chunk alignment, fsblocks */
>        __uint32_t      sb_unit;        /* stripe or raid unit */
>-       __uint32_t      sb_width;       /* stripe or raid width */
>+       __uint32_t      sb_width;       /* stripe or raid width */
>        __uint8_t       sb_dirblklog;   /* log2 of dir block size (fsbs) */
>-        __uint8_t       sb_dummy[7];    /* padding */
>+       __uint8_t       sb_logsectlog;  /* log2 of the log sector size */
>+       __uint16_t      sb_logsectsize; /* sector size for the log, bytes */
>+       __uint32_t      sb_logsunit;    /* stripe unit size for the log */
>+       __uint32_t      sb_features2;   /* additional feature bits */
> } xfs_sb_t;
>
>So before the sector size > 512 bytes code, there was padding to push the
>superblock out to 64bit alignement so that sb_dirblklog was correctly aligned.
>The xfs_sb_info structure:
>
>     { offsetof(xfs_sb_t, sb_unit),      0 },
>     { offsetof(xfs_sb_t, sb_width),     0 },
>     { offsetof(xfs_sb_t, sb_dirblklog),         0 },
>-    { offsetof(xfs_sb_t, sb_dummy),     1 },
>+    { offsetof(xfs_sb_t, sb_logsectlog), 0 },
>+    { offsetof(xfs_sb_t, sb_logsectsize),0 },
>     { offsetof(xfs_sb_t, sb_logsunit),  0 },
>+    { offsetof(xfs_sb_t, sb_features2),         0 },
>     { sizeof(xfs_sb_t),                 0 }
> };
>
>had the sb_dummy field as "no translate" so it effectively ignored it
>but it ensured that sb_dirblklog was sized correctly.
>
>The real bug here was whoever removed the dummy field and did not
>replace that with a comment ot say that the xfs_sb strucutre needs
>to be padded to 64 bits to ensure translation worked properly
>on 64 bit systems.
>
>I'd prefer explicit padding (with warning comments) over packing
>the structure. Thoughts?
>  
>
That seems safer to me and the comments will make the next  person to 
muck with
the structure think about padding.

>Cheers,
>
>Dave.
>  
>