xfs
[Top] [All Lists]

Re: [PATCH 1/4] xfs: Don't wrap growfs AGFL indexes

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/4] xfs: Don't wrap growfs AGFL indexes
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Thu, 7 Apr 2016 16:50:43 -0700
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1459836310-12619-2-git-send-email-david@xxxxxxxxxxxxx>
References: <1459836310-12619-1-git-send-email-david@xxxxxxxxxxxxx> <1459836310-12619-2-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.24 (2015-08-30)
On Tue, Apr 05, 2016 at 04:05:07PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> Commit 96f859d ("libxfs: pack the agfl header structure so
> XFS_AGFL_SIZE is correct") allowed the freelist to use the empty
> slot at the end of the freelist on 64 bit systems that was not
> being used due to sizeof() rounding up the structure size.
> 
> This has caused versions of xfs_repair prior to 4.5.0 (which also
> has the fix) to report this as a corruption once the filesystem has
> been grown. Older kernels can also have problems (seen from a whacky
> container/vm management environment) mounting filesystems grown on a
> system with a newer kernel than the vm/container it is deployed on.
> 
> To avoid this problem, change the initial free list indexes not to
> wrap across the end of the AGFL, hence avoiding the initialisation
> of agf_fllast to the last index in the AGFL.

I have to admit that it's been a while that I looked at the AGFL
code, but I simply don't understand what's happening in this patch.
Diff slightly reorder:

> -             agf->agf_flfirst = 0;
> +             agf->agf_flfirst = cpu_to_be32(1);

So flfirst moves from 0 to 1.

> -             agf->agf_fllast = cpu_to_be32(XFS_AGFL_SIZE(mp) - 1);
> +             agf->agf_fllast = 0;

And last from size - 1 to 0.  In my naive reading this introduces
wrapping and doesn't remove it.  What do I miss?

<Prev in Thread] Current Thread [Next in Thread>