[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: Fri, 19 Sep 2008 10:51:33 +1000
Cc: Russell Cattelan <cattelan@xxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <63352.>
References: <48D19A83.4040608@xxxxxxxxxxx> <48D1CD46.4010104@xxxxxxx> <48D1DCD5.7040502@xxxxxxxxxxx> <48D218AE.9090400@xxxxxxx> <48D2C97A.1070703@xxxxxxxxxxx> <63352.>
Reply-to: lachlan@xxxxxxx
User-agent: Thunderbird (X11/20080707)
Eric Sandeen wrote:
Russell Cattelan 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;

Cool I'll give it some run through when I done traveling.

I still think compact_full should simply be eliminated since
it really doesn't help, and it's obviously confusing code.
Or we should make sure it works and get rid of compact_pages
since compact_full behaves just like compact_pages when not
doing partial moves.

I'd agree with that, at least as far as reevaluating this packing stuff -
given the seriousness of the bug when you do hit it, and how rarely it's
ever hit, apparently this chunk of code is almost never run ....

I agree too.  If any code is difficult to reach it's also difficult to test.
We've had numerous reports of extent corruption that could be explained by
this bug but we have not been able to reproduce the symptoms let alone devise
a reliable test case.

What real benefit does compact_full have over compact_pages?
Are there corner cases where compact_pages is not good enough?

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