From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bob Peterson Date: Wed, 24 Feb 2016 15:42:14 -0500 (EST) Subject: [Cluster-devel] [PATCH 4/4] GFS2: Make deletes use regular inode_lookup with special code path In-Reply-To: <56CC339F.8010609@redhat.com> References: <1450465350-10060-1-git-send-email-rpeterso@redhat.com> <1450465350-10060-5-git-send-email-rpeterso@redhat.com> <56CC339F.8010609@redhat.com> Message-ID: <1064069915.29336433.1456346534130.JavaMail.zimbra@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit ----- 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