From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bob Peterson Date: Wed, 27 Apr 2016 13:10:17 -0400 (EDT) Subject: [Cluster-devel] [GFS2 PATCH 3/3 v2] GFS2: Add retry loop to delete_work_func In-Reply-To: <1461771337-17317-4-git-send-email-rpeterso@redhat.com> References: <1461771337-17317-1-git-send-email-rpeterso@redhat.com> <1461771337-17317-4-git-send-email-rpeterso@redhat.com> Message-ID: <853479097.58344937.1461777017183.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 Hi, I made a slight correction to the third patch I posted previously. I added the condition " || IS_ERR(inode)" to the delete_work_func, which needs to be there. This is the corrected version. The delete work function, delete_work_func, often doesn't find the inode needed to free an inode that's been marked unlinked. That's because it only tries gfs2_ilookup once, and it's often not found because of two things: The fact that gfs2_lookup_by_inum is only called in an else condition, and the fact that the non-blocking lookup often encounters inodes that are being I_FREEd by the vfs. This patch allows it to retry the lookup when under that circumstance, otherwise call the gfs2_lookup_by_inum. Signed-off-by: Bob Peterson --- diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c index 672de35..0266b7c 100644 --- a/fs/gfs2/glock.c +++ b/fs/gfs2/glock.c @@ -571,13 +571,15 @@ out_unlock: return; } +#define LOOKUP_RETRIES 30 + 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; + struct inode *inode = NULL; u64 no_addr = gl->gl_name.ln_number; + int retries; /* If someone's using this glock to create a new dinode, the block must have been freed by another node, then re-used, in which case our @@ -585,13 +587,24 @@ 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) + for (retries = 0; retries < LOOKUP_RETRIES; retries++) { inode = gfs2_ilookup(sdp->sd_vfs, no_addr, 1); - else - inode = gfs2_lookup_by_inum(sdp, no_addr, NULL, GFS2_BLKST_UNLINKED); + + if (inode == ERR_PTR(-EAGAIN)) + msleep(10); + else + break; + } + if (inode == NULL || IS_ERR(inode)) { + for (retries = 0; retries < LOOKUP_RETRIES; retries++) { + inode = gfs2_lookup_by_inum(sdp, no_addr, NULL, + GFS2_BLKST_UNLINKED); + if (inode == ERR_PTR(-EAGAIN)) + msleep(10); + else + break; + } + } if (inode && !IS_ERR(inode)) { d_prune_aliases(inode); iput(inode); diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c index 48c1418..3cd06c7 100644 --- a/fs/gfs2/inode.c +++ b/fs/gfs2/inode.c @@ -37,6 +37,11 @@ #include "super.h" #include "glops.h" +enum { + LOOKUP_UNLOCKED = 0, + LOOKUP_LOCKED = 1, +}; + struct gfs2_skip_data { u64 no_addr; int skipped; @@ -71,27 +76,34 @@ static int iget_set(struct inode *inode, void *opaque) return 0; } -struct inode *gfs2_ilookup(struct super_block *sb, u64 no_addr, int non_block) +static struct inode *inode_lookup_common(struct super_block *sb, u64 no_addr, + int non_block, int locked) { unsigned long hash = (unsigned long)no_addr; - struct gfs2_skip_data data; + struct gfs2_skip_data data = {.no_addr = no_addr, .skipped = 0, + .non_block = non_block}; + struct inode *inode; - data.no_addr = no_addr; - data.skipped = 0; - data.non_block = non_block; - return ilookup5(sb, hash, iget_test, &data); + if (locked == LOOKUP_LOCKED) + inode = iget5_locked(sb, hash, iget_test, iget_set, &data); + else + inode = ilookup5(sb, hash, iget_test, &data); + + if (non_block && data.skipped) + return ERR_PTR(-EAGAIN); + + return inode; +} + +struct inode *gfs2_ilookup(struct super_block *sb, u64 no_addr, int non_block) +{ + return inode_lookup_common(sb, no_addr, non_block, LOOKUP_UNLOCKED); } 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 inode_lookup_common(sb, no_addr, non_block, LOOKUP_LOCKED); } /** @@ -148,6 +160,8 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type, inode = gfs2_iget(sb, no_addr, non_block); if (!inode) return ERR_PTR(-ENOMEM); + if (inode == ERR_PTR(-EAGAIN)) + return inode; ip = GFS2_I(inode); ip->i_no_addr = no_addr;