xfs
[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:17:53 +1000
Cc: Russell Cattelan <cattelan@xxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <48D3456A.9090808@xxxxxxxxxxx>
References: <48D19A83.4040608@xxxxxxxxxxx> <48D1CD46.4010104@xxxxxxx> <48D1DCD5.7040502@xxxxxxxxxxx> <48D218AE.9090400@xxxxxxx> <48D2C97A.1070703@xxxxxxxxxxx> <63352.131.252.241.230.1221776406.squirrel@xxxxxxxxxxx> <48D2F795.3080104@xxxxxxx> <48D31B9A.1080803@xxxxxxx> <48D3456A.9090808@xxxxxxxxxxx>
Reply-to: lachlan@xxxxxxx
User-agent: Thunderbird 2.0.0.16 (X11/20080707)
Eric Sandeen wrote:
Lachlan McIlroy wrote:
Here's a patch to remove xfs_iext_irec_compact_full() like Russell
did in his original patch - are you guys happy with this?

I'm putting it through it's paces now and so far it looks good.

I'll have to think more about it, honestly.  Probably fine, but I've not
looked at all the surrounding code, I was so far just looking for the
original bug.

(FWIW, compact_full *does* get called reasonably frequently, but the
memmove case is what's hard to hit...)
xfs_iext_irec_compact_full() is probably getting called frequently from
xfs_iext_indirect_to_direct() which we only ever call when the number of
extents will fit into one extent buffer so we will never need the memmove
case in xfs_iext_irec_compact_full() and we can safely call
xfs_iext_irec_compact_pages() instead.

I wonder why xfs_iext_irec_compact_pages() is doing a memmove() instead
of a memcpy().


-Eric

--- a/fs/xfs/xfs_inode.c        2008-09-19 13:08:08.000000000 +1000
+++ b/fs/xfs/xfs_inode.c        2008-09-19 13:16:34.000000000 +1000
@@ -4157,7 +4166,7 @@ xfs_iext_indirect_to_direct(
        ASSERT(nextents <= XFS_LINEAR_EXTS);
        size = nextents * sizeof(xfs_bmbt_rec_t);
- xfs_iext_irec_compact_full(ifp);
+       xfs_iext_irec_compact_pages(ifp);
        ASSERT(ifp->if_real_bytes == XFS_IEXT_BUFSZ);
ep = ifp->if_u1.if_ext_irec->er_extbuf;

...


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