[Cluster-devel] [PATCH 4/4] GFS2: Make deletes use regular inode_lookup with special code path

[Bob Peterson <rpeterso@redhat.com>]

----- Original Message -----
> Hi,
> 
> On 18/12/15 19:02, Bob Peterson wrote:
> > This patch changes the inode reclaiming code, delete_work_func, use
> > the normal gfs2_inode_lookup, but with a special parameter, for_del
> > which causes additional checks to be made.
> I assume that this was just intended to be a clean up rather than
> something that fixes a bug? At least I can't see any obvious changes to
> the operations, just the way in which they are carried out. I think it
> would be neater to turn the non_block parameter of gfs2_inode_lookup()
> into a flags argument, rather than adding an additional argument which
> is only a bool.
> 
> I'm not sure that factoring out some of the checks from
> gfs2_lookup_by_inum() buys us that much though, since that function
> still has to remain in its current form for NFS anyway,
> 
> Steve.

Hi Steve,

Actually, no, it's not a cleanup at all. Without this patch, I can
recreate one of the problems I'm trying to solve here, which is:

[nate_bob1] [fsck] Unlinked inode found at block 483992 (0x76298).
[nate_bob1] [fsck] Block 483993 (0x76299) bitmap says 1 (Data) but FSCK saw 0 (Free)
[nate_bob1] [fsck] Bitmap at block 483993 (0x76299) left inconsistent
[nate_bob1] [fsck] Block 483994 (0x7629a) bitmap says 1 (Data) but FSCK saw 0 (Free)
[nate_bob1] [fsck] Bitmap at block 483994 (0x7629a) left inconsistent
[nate_bob1] [fsck] Block 483995 (0x7629b) bitmap says 1 (Data) but FSCK saw 0 (Free)
[nate_bob1] [fsck] Bitmap at block 483995 (0x7629b) left inconsistent
[nate_bob1] [fsck] Block 483996 (0x7629c) bitmap says 1 (Data) but FSCK saw 0 (Free)
[nate_bob1] [fsck] Bitmap at block 483996 (0x7629c) left inconsistent
[nate_bob1] [fsck] Block 483997 (0x7629d) bitmap says 1 (Data) but FSCK saw 0 (Free)
[nate_bob1] [fsck] Bitmap at block 483997 (0x7629d) left inconsistent
etc.

Turns out if you use the regular inode lookup path, the problem goes away.

Initially (several months ago) I had written a new inode lookup function
just for the transition from unlinked to free. I debugged it and got it
working fairly well, only to discover that my code did the exact same things
in the exact same order as the original lookup. So I deleted my new function
in favor of using the original, hence, this patch.

My concern, of course, is that NFS may also be vulnerable to this problem,
but I didn't want to mess with the NFS path any more than I had to, so I
left the original inum lookup there for it to use. At some point we should
try to determine if NFS has an issue related to this. I suspect it does.

Regards,

Bob Peterson
Red Hat File Systems