[Top] [All Lists]

Re: REVIEW: Fix for incore extent corruption.

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: REVIEW: Fix for incore extent corruption.
From: Lachlan McIlroy <lachlan@xxxxxxx>
Date: Mon, 22 Sep 2008 12:08:36 +1000
Cc: Russell Cattelan <cattelan@xxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <48D34E66.5090006@xxxxxxxxxxx>
References: <48D19A83.4040608@xxxxxxxxxxx> <48D1CD46.4010104@xxxxxxx> <48D1DCD5.7040502@xxxxxxxxxxx> <48D218AE.9090400@xxxxxxx> <59243.> <48D2F874.4060608@xxxxxxx> <48D34E66.5090006@xxxxxxxxxxx>
Reply-to: lachlan@xxxxxxx
User-agent: Thunderbird (X11/20080707)
Eric Sandeen wrote:
Lachlan McIlroy wrote:
Eric Sandeen wrote:
Lachlan McIlroy wrote:
Russell, this fixes xfs_iext_irec_compact_full().  If we don't move
all the records from the next page into the current page then we need
to update the er_extoff of the modified page as we move the remaining
extents up.  Would you mind giving it a go?

--- a/fs/xfs/xfs_inode.c        2008-09-18 18:48:46.000000000 +1000
+++ b/fs/xfs/xfs_inode.c        2008-09-18 18:57:18.000000000 +1000
@@ -4623,6 +4623,7 @@ xfs_iext_irec_compact_full(
                                        (XFS_LINEAR_EXTS -
                                                erp_next->er_extcount) *
+                               erp_next->er_extoff += ext_diff;
Lachlan, I concur.  I spent way too long last night looking at this and
arrived at the same conclusion about the root cause of the problem, but
didn't hae *quite* the right solution.  I blame it on 2am ;)  Your fix
looks right.

(though I'd probably move the erp_next changes into the else clause? Otherwise you're changing it then freeing it.)
I don't understand what you mean by that.  Could you elaborate?

Sorry I mis-read where the above hunk went... that makes sense as-is above.

For clarity having the erp_next->er_extoff and er_extcount adjustments
together *might* make sense but no big deal.

That did occur to me.  I thought if I put it with the er_extcount adjustment
then it might look like the er_extoff line was missing from
xfs_iext_irec_compact_pages() because it does the er_extcount adjustment too
but never needs to adjust er_extoff.  Doesn't really matter if we get rid of

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