Re: [PATCH 2/3] xfs: fix implicit padding in directory and attr CRC formats

[Dave Chinner <david@fromorbit.com>]

On Wed, Jun 12, 2013 at 07:58:19PM -0500, Ben Myers wrote:
> On Wed, Jun 12, 2013 at 12:19:07PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Michael L. Semon has been testing CRC patches ona 32 bit system and
> 						on a
> 
> > been seeing assert failures in the directory code from xfs/080.
> > Thanks to Michael's heroic efforts with printk debugging, we found
> > that the problem was that the last free space being left in the
> > directory structure was too small to fit a unused tag structure and
> > it was being corrupted and attempting to log a region out of bounds.
> > Hence the assert failure looked something like:
> > 
> > .....
> > #5 calling xfs_dir2_data_log_unused() 36 32
> > #1 4092 4095 4096
> > #2 8182 8183 4096
>      first? 
>           last?
>                bp->b_length?

Yup.

> > 
> > Note that this only affects 32 bit systems with CRCs enabled.
> > Everything else is just fine. Note that filesystems created before
> 					 CRC enabled filesystems
> 
> I suggest this be added to head off any confusion.

Sure.  Do I need to resubmit this, or are you going to just modify
the commit message yourself before applying it?

> > index 995f1f5..7826782 100644
> > --- a/fs/xfs/xfs_dir2_format.h
> > +++ b/fs/xfs/xfs_dir2_format.h
> > @@ -266,6 +266,7 @@ struct xfs_dir3_blk_hdr {
> >  struct xfs_dir3_data_hdr {
> >  	struct xfs_dir3_blk_hdr	hdr;
> >  	xfs_dir2_data_free_t	best_free[XFS_DIR2_DATA_FD_COUNT];
> > +	__be32			pad;	/* 64 bit alignment */
> 
> I counted these up and it looks fine.  Nice work gents.

pahole is a much better way of determining structure size - it tells
you exactly what the compiler did, rather than having to assume what
the compiler is going to do...

$ pahole fs/xfs/xfs_dir2_data.o |less
.....
struct xfs_dir3_blk_hdr {
        __be32                     magic;                /*     0     4 */
        __be32                     crc;                  /*     4     4 */
        __be64                     blkno;                /*     8     8 */
        __be64                     lsn;                  /*    16     8 */
        uuid_t                     uuid;                 /*    24    16 */
        __be64                     owner;                /*    40     8 */

        /* size: 48, cachelines: 1, members: 6 */
        /* last cacheline: 48 bytes */
};
struct xfs_dir3_data_hdr {
        struct xfs_dir3_blk_hdr    hdr;                  /*     0    48 */
        xfs_dir2_data_free_t       best_free[3];         /*    48    12 */
        __be32                     pad;                  /*    60     4 */
        /* --- cacheline 1 boundary (64 bytes) --- */

        /* size: 64, cachelines: 1, members: 3 */
};
....


Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs