xfs
[Top] [All Lists]

Re: REVIEW: Fix for incore extent corruption.

To: "Russell Cattelan" <cattelan@xxxxxxxxxxx>
Subject: Re: REVIEW: Fix for incore extent corruption.
From: "Eric Sandeen" <sandeen@xxxxxxxxxxx>
Date: Thu, 18 Sep 2008 17:20:06 -0500 (CDT)
Cc: lachlan@xxxxxxx, xfs@xxxxxxxxxxx
Importance: Normal
In-reply-to: <48D2C97A.1070703@xxxxxxxxxxx>
References: <48D19A83.4040608@xxxxxxxxxxx> <48D1CD46.4010104@xxxxxxx> <48D1DCD5.7040502@xxxxxxxxxxx> <48D218AE.9090400@xxxxxxx> <48D2C97A.1070703@xxxxxxxxxxx>
User-agent: SquirrelMail/1.4.8-4.0.1.el5.centos.2
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) *
>>                     sizeof(xfs_bmbt_rec_t));
>> +                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 ....

-Eric

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