From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Tue, 23 Feb 2016 10:25:35 +0000 Subject: [Cluster-devel] [PATCH 4/4] GFS2: Make deletes use regular inode_lookup with special code path In-Reply-To: <1450465350-10060-5-git-send-email-rpeterso@redhat.com> References: <1450465350-10060-1-git-send-email-rpeterso@redhat.com> <1450465350-10060-5-git-send-email-rpeterso@redhat.com> Message-ID: <56CC339F.8010609@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 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. > Signed-off-by: Bob Peterson > --- > fs/gfs2/dir.c | 2 +- > fs/gfs2/glock.c | 9 +-------- > fs/gfs2/inode.c | 32 ++++++++++++++++++++++++++++---- > fs/gfs2/inode.h | 3 ++- > fs/gfs2/ops_fstype.c | 2 +- > 5 files changed, 33 insertions(+), 15 deletions(-) > > diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c > index 2bef023..6a92592 100644 > --- a/fs/gfs2/dir.c > +++ b/fs/gfs2/dir.c > @@ -1660,7 +1660,7 @@ struct inode *gfs2_dir_search(struct inode *dir, const struct qstr *name, > brelse(bh); > if (fail_on_exist) > return ERR_PTR(-EEXIST); > - inode = gfs2_inode_lookup(dir->i_sb, dtype, addr, formal_ino); > + inode = gfs2_inode_lookup(dir->i_sb, dtype, addr, formal_ino, 0); > if (!IS_ERR(inode)) > GFS2_I(inode)->i_rahead = rahead; > return inode; > diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c > index 3e16d82..795c2f3 100644 > --- a/fs/gfs2/glock.c > +++ b/fs/gfs2/glock.c > @@ -568,7 +568,6 @@ static void delete_work_func(struct work_struct *work) > { > struct gfs2_glock *gl = container_of(work, struct gfs2_glock, gl_delete); > struct gfs2_sbd *sdp = gl->gl_name.ln_sbd; > - struct gfs2_inode *ip; > struct inode *inode; > u64 no_addr = gl->gl_name.ln_number; > > @@ -578,13 +577,7 @@ static void delete_work_func(struct work_struct *work) > if (test_bit(GLF_INODE_CREATING, &gl->gl_flags)) > goto out; > > - ip = gl->gl_object; > - /* Note: Unsafe to dereference ip as we don't hold right refs/locks */ > - > - if (ip) > - inode = gfs2_ilookup(sdp->sd_vfs, no_addr); > - else > - inode = gfs2_lookup_by_inum(sdp, no_addr, NULL, GFS2_BLKST_UNLINKED); > + inode = gfs2_inode_lookup(sdp->sd_vfs, DT_UNKNOWN, no_addr, 0, 1); > if (inode && !IS_ERR(inode)) { > d_prune_aliases(inode); > iput(inode); > diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c > index 01b6d1a..0357862 100644 > --- a/fs/gfs2/inode.c > +++ b/fs/gfs2/inode.c > @@ -75,17 +75,37 @@ static void gfs2_set_iop(struct inode *inode) > } > } > > +static int verify_unlinked(struct gfs2_inode *ip) > +{ > + struct gfs2_holder i_gh; > + struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode); > + int error; > + > + error = gfs2_glock_nq_init(ip->i_gl, LM_ST_EXCLUSIVE, GL_SKIP, &i_gh); > + if (error) > + return error; > + > + error = gfs2_check_blk_type(sdp, ip->i_no_addr, GFS2_BLKST_UNLINKED); > + if (error == 0) > + error = gfs2_inode_refresh(ip); > + > + gfs2_glock_dq_uninit(&i_gh); > + return error; > +} > + > /** > * gfs2_inode_lookup - Lookup an inode > * @sb: The super block > - * @no_addr: The inode number > * @type: The type of the inode > + * @no_addr: The inode number > + * @no_formal_ino: The formal inode number > + * @for_del: Is this lookup for deleting purposes? > * > * Returns: A VFS inode, or an error > */ > > struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type, > - u64 no_addr, u64 no_formal_ino) > + u64 no_addr, u64 no_formal_ino, int for_del) > { > struct inode *inode; > struct gfs2_inode *ip; > @@ -121,7 +141,11 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type, > gfs2_glock_put(io_gl); > io_gl = NULL; > > - if (type == DT_UNKNOWN) { > + if (for_del) { > + error = verify_unlinked(ip); > + if (error) > + goto fail_refresh; > + } else if (type == DT_UNKNOWN) { > /* Inode glock must be locked already */ > error = gfs2_inode_refresh(GFS2_I(inode)); > if (error) > @@ -169,7 +193,7 @@ struct inode *gfs2_lookup_by_inum(struct gfs2_sbd *sdp, u64 no_addr, > if (error) > goto fail; > > - inode = gfs2_inode_lookup(sb, DT_UNKNOWN, no_addr, 0); > + inode = gfs2_inode_lookup(sb, DT_UNKNOWN, no_addr, 0, 0); > if (IS_ERR(inode)) > goto fail; > > diff --git a/fs/gfs2/inode.h b/fs/gfs2/inode.h > index e1af0d4..d6233e9 100644 > --- a/fs/gfs2/inode.h > +++ b/fs/gfs2/inode.h > @@ -94,7 +94,8 @@ err: > } > > extern struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned type, > - u64 no_addr, u64 no_formal_ino); > + u64 no_addr, u64 no_formal_ino, > + int for_del); > extern struct inode *gfs2_lookup_by_inum(struct gfs2_sbd *sdp, u64 no_addr, > u64 *no_formal_ino, > unsigned int blktype); > diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c > index 9ed522c..7aacdf2 100644 > --- a/fs/gfs2/ops_fstype.c > +++ b/fs/gfs2/ops_fstype.c > @@ -454,7 +454,7 @@ static int gfs2_lookup_root(struct super_block *sb, struct dentry **dptr, > struct dentry *dentry; > struct inode *inode; > > - inode = gfs2_inode_lookup(sb, DT_DIR, no_addr, 0); > + inode = gfs2_inode_lookup(sb, DT_DIR, no_addr, 0, 0); > if (IS_ERR(inode)) { > fs_err(sdp, "can't read in %s inode: %ld\n", name, PTR_ERR(inode)); > return PTR_ERR(inode);