From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Tue, 23 Feb 2016 10:15:58 +0000 Subject: [Cluster-devel] [PATCH 2/4] GFS2: Don't filter out I_FREEING inodes anymore In-Reply-To: <1450465350-10060-3-git-send-email-rpeterso@redhat.com> References: <1450465350-10060-1-git-send-email-rpeterso@redhat.com> <1450465350-10060-3-git-send-email-rpeterso@redhat.com> Message-ID: <56CC315E.4020706@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Hi, On 18/12/15 19:02, Bob Peterson wrote: > This patch basically reverts a very old patch from 2008, > 7a9f53b3c1875bef22ad4588e818bc046ef183da, with the title > "Alternate gfs2_iget to avoid looking up inodes being freed". > The original patch was designed to avoid a deadlock caused by lock > ordering with try_rgrp_unlink. The patch forced the function to not > find inodes that were being removed by VFS. The problem is, that > made it impossible for nodes to delete their own unlinked dinodes > after a certain point in time, because the inode needed was not found > by this filtering process. There is no longer a need for the patch, > since function try_rgrp_unlink no longer locks the inode: All it does > is queue the glock onto the delete work_queue, so there should be no > more deadlock. This looks good. For older kernels though I think the issue may still exist due to the way workqueues used to work, but for upstream and modern kernels this is a nice clean up, Signed-off-by: Steven Whitehouse Steve. > Signed-off-by: Bob Peterson > --- > fs/gfs2/export.c | 2 +- > fs/gfs2/glock.c | 2 +- > fs/gfs2/inode.c | 59 ++++---------------------------------------------------- > fs/gfs2/inode.h | 2 +- > 4 files changed, 7 insertions(+), 58 deletions(-) > > diff --git a/fs/gfs2/export.c b/fs/gfs2/export.c > index 5d15e94..d5bda85 100644 > --- a/fs/gfs2/export.c > +++ b/fs/gfs2/export.c > @@ -137,7 +137,7 @@ static struct dentry *gfs2_get_dentry(struct super_block *sb, > struct gfs2_sbd *sdp = sb->s_fs_info; > struct inode *inode; > > - inode = gfs2_ilookup(sb, inum->no_addr, 0); > + inode = gfs2_ilookup(sb, inum->no_addr); > if (inode) { > if (GFS2_I(inode)->i_no_formal_ino != inum->no_formal_ino) { > iput(inode); > diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c > index cb5ed3a..3e16d82 100644 > --- a/fs/gfs2/glock.c > +++ b/fs/gfs2/glock.c > @@ -582,7 +582,7 @@ static void delete_work_func(struct work_struct *work) > /* Note: Unsafe to dereference ip as we don't hold right refs/locks */ > > if (ip) > - inode = gfs2_ilookup(sdp->sd_vfs, no_addr, 1); > + inode = gfs2_ilookup(sdp->sd_vfs, no_addr); > else > inode = gfs2_lookup_by_inum(sdp, no_addr, NULL, GFS2_BLKST_UNLINKED); > if (inode && !IS_ERR(inode)) { > diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c > index 6cdafed..2adc1ee 100644 > --- a/fs/gfs2/inode.c > +++ b/fs/gfs2/inode.c > @@ -37,61 +37,9 @@ > #include "super.h" > #include "glops.h" > > -struct gfs2_skip_data { > - u64 no_addr; > - int skipped; > - int non_block; > -}; > - > -static int iget_test(struct inode *inode, void *opaque) > -{ > - struct gfs2_inode *ip = GFS2_I(inode); > - struct gfs2_skip_data *data = opaque; > - > - if (ip->i_no_addr == data->no_addr) { > - if (data->non_block && > - inode->i_state & (I_FREEING|I_CLEAR|I_WILL_FREE)) { > - data->skipped = 1; > - return 0; > - } > - return 1; > - } > - return 0; > -} > - > -static int iget_set(struct inode *inode, void *opaque) > +struct inode *gfs2_ilookup(struct super_block *sb, u64 no_addr) > { > - struct gfs2_inode *ip = GFS2_I(inode); > - struct gfs2_skip_data *data = opaque; > - > - if (data->skipped) > - return -ENOENT; > - inode->i_ino = (unsigned long)(data->no_addr); > - ip->i_no_addr = data->no_addr; > - return 0; > -} > - > -struct inode *gfs2_ilookup(struct super_block *sb, u64 no_addr, int non_block) > -{ > - unsigned long hash = (unsigned long)no_addr; > - struct gfs2_skip_data data; > - > - data.no_addr = no_addr; > - data.skipped = 0; > - data.non_block = non_block; > - return ilookup5(sb, hash, iget_test, &data); > -} > - > -static struct inode *gfs2_iget(struct super_block *sb, u64 no_addr, > - int non_block) > -{ > - struct gfs2_skip_data data; > - unsigned long hash = (unsigned long)no_addr; > - > - data.no_addr = no_addr; > - data.skipped = 0; > - data.non_block = non_block; > - return iget5_locked(sb, hash, iget_test, iget_set, &data); > + return ilookup(sb, (unsigned long)no_addr); > } > > /** > @@ -145,8 +93,9 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type, > struct gfs2_glock *io_gl = NULL; > int error; > > - inode = gfs2_iget(sb, no_addr, non_block); > + inode = iget_locked(sb, (unsigned long)no_addr); > ip = GFS2_I(inode); > + ip->i_no_addr = no_addr; > > if (!inode) > return ERR_PTR(-ENOMEM); > diff --git a/fs/gfs2/inode.h b/fs/gfs2/inode.h > index ba4d949..22c27a8 100644 > --- a/fs/gfs2/inode.h > +++ b/fs/gfs2/inode.h > @@ -99,7 +99,7 @@ extern struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned type, > extern struct inode *gfs2_lookup_by_inum(struct gfs2_sbd *sdp, u64 no_addr, > u64 *no_formal_ino, > unsigned int blktype); > -extern struct inode *gfs2_ilookup(struct super_block *sb, u64 no_addr, int nonblock); > +extern struct inode *gfs2_ilookup(struct super_block *sb, u64 no_addr); > > extern int gfs2_inode_refresh(struct gfs2_inode *ip); >