All of lore.kernel.org
 help / color / mirror / Atom feed
* [Cluster-devel] [PATCH] gfs2: Use TRY lock in gfs2_inode_lookup for UNLINKED inodes
@ 2022-08-25 14:53 Bob Peterson
  2022-08-25 15:59 ` Andreas Gruenbacher
  0 siblings, 1 reply; 2+ messages in thread
From: Bob Peterson @ 2022-08-25 14:53 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Before this patch, delete_work_func() would check for the GLF_DEMOTE
flag on the iopen glock and if set, it would perform special processing.
However, there was a race whereby the GLF_DEMOTE flag could be set by
another process after the check. Then when it called
gfs2_lookup_by_inum() which calls gfs2_inode_lookup(), it tried to lock
the iopen glock in SH mode, but the GLF_DEMOTE flag prevented the
request from being granted. But the iopen glock could never be demoted
because that happens when the inode is evicted, and the evict was never
completed because of the failed lookup.

To fix that, change function gfs2_inode_lookup() so that when
GFS2_BLKST_UNLINKED inodes are searched, it uses the LM_FLAG_TRY flag
for the iopen glock.  If the locking request fails, fail
gfs2_inode_lookup() with -EAGAIN so that delete_work_func() can retry
the operation later.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/gfs2/glock.c |  8 +++++---
 fs/gfs2/inode.c | 10 ++++++++--
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 0b36a16659b6..f1973a442955 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -1018,16 +1018,18 @@ static void delete_work_func(struct work_struct *work)
 			if (gfs2_queue_delete_work(gl, 5 * HZ))
 				return;
 		}
-		goto out;
 	}
 
 	inode = gfs2_lookup_by_inum(sdp, no_addr, gl->gl_no_formal_ino,
 				    GFS2_BLKST_UNLINKED);
-	if (!IS_ERR_OR_NULL(inode)) {
+	if (IS_ERR(inode)) {
+		if (PTR_ERR(inode) == -EAGAIN &&
+			(gfs2_queue_delete_work(gl, 5 * HZ)))
+				return;
+	} else {
 		d_prune_aliases(inode);
 		iput(inode);
 	}
-out:
 	gfs2_glock_put(gl);
 }
 
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index c8ec876f33ea..56ded979988c 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -130,6 +130,7 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type,
 	if (inode->i_state & I_NEW) {
 		struct gfs2_sbd *sdp = GFS2_SB(inode);
 		struct gfs2_glock *io_gl;
+		int extra_flags = 0;
 
 		error = gfs2_glock_get(sdp, no_addr, &gfs2_inode_glops, CREATE,
 				       &ip->i_gl);
@@ -141,9 +142,12 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type,
 		if (unlikely(error))
 			goto fail;
 
-		if (blktype != GFS2_BLKST_UNLINKED)
+		if (blktype == GFS2_BLKST_UNLINKED)
+			extra_flags |= LM_FLAG_TRY;
+		else
 			gfs2_cancel_delete_work(io_gl);
-		error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED, GL_EXACT,
+		error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED,
+					   GL_EXACT | extra_flags,
 					   &ip->i_iopen_gh);
 		gfs2_glock_put(io_gl);
 		if (unlikely(error))
@@ -210,6 +214,8 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type,
 	return inode;
 
 fail:
+	if (error == GLR_TRYFAILED)
+		error = -EAGAIN;
 	if (gfs2_holder_initialized(&ip->i_iopen_gh))
 		gfs2_glock_dq_uninit(&ip->i_iopen_gh);
 	if (gfs2_holder_initialized(&i_gh))
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [Cluster-devel] [PATCH] gfs2: Use TRY lock in gfs2_inode_lookup for UNLINKED inodes
  2022-08-25 14:53 [Cluster-devel] [PATCH] gfs2: Use TRY lock in gfs2_inode_lookup for UNLINKED inodes Bob Peterson
@ 2022-08-25 15:59 ` Andreas Gruenbacher
  0 siblings, 0 replies; 2+ messages in thread
From: Andreas Gruenbacher @ 2022-08-25 15:59 UTC (permalink / raw)
  To: cluster-devel.redhat.com

On Thu, Aug 25, 2022 at 4:53 PM Bob Peterson <rpeterso@redhat.com> wrote:
> Before this patch, delete_work_func() would check for the GLF_DEMOTE
> flag on the iopen glock and if set, it would perform special processing.
> However, there was a race whereby the GLF_DEMOTE flag could be set by
> another process after the check. Then when it called
> gfs2_lookup_by_inum() which calls gfs2_inode_lookup(), it tried to lock
> the iopen glock in SH mode, but the GLF_DEMOTE flag prevented the
> request from being granted. But the iopen glock could never be demoted
> because that happens when the inode is evicted, and the evict was never
> completed because of the failed lookup.
>
> To fix that, change function gfs2_inode_lookup() so that when
> GFS2_BLKST_UNLINKED inodes are searched, it uses the LM_FLAG_TRY flag
> for the iopen glock.  If the locking request fails, fail
> gfs2_inode_lookup() with -EAGAIN so that delete_work_func() can retry
> the operation later.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/gfs2/glock.c |  8 +++++---
>  fs/gfs2/inode.c | 10 ++++++++--
>  2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> index 0b36a16659b6..f1973a442955 100644
> --- a/fs/gfs2/glock.c
> +++ b/fs/gfs2/glock.c
> @@ -1018,16 +1018,18 @@ static void delete_work_func(struct work_struct *work)
>                         if (gfs2_queue_delete_work(gl, 5 * HZ))
>                                 return;
>                 }
> -               goto out;
>         }
>
>         inode = gfs2_lookup_by_inum(sdp, no_addr, gl->gl_no_formal_ino,
>                                     GFS2_BLKST_UNLINKED);
> -       if (!IS_ERR_OR_NULL(inode)) {
> +       if (IS_ERR(inode)) {
> +               if (PTR_ERR(inode) == -EAGAIN &&
> +                       (gfs2_queue_delete_work(gl, 5 * HZ)))
> +                               return;
> +       } else {
>                 d_prune_aliases(inode);
>                 iput(inode);
>         }
> -out:
>         gfs2_glock_put(gl);
>  }
>
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index c8ec876f33ea..56ded979988c 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -130,6 +130,7 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type,
>         if (inode->i_state & I_NEW) {
>                 struct gfs2_sbd *sdp = GFS2_SB(inode);
>                 struct gfs2_glock *io_gl;
> +               int extra_flags = 0;
>
>                 error = gfs2_glock_get(sdp, no_addr, &gfs2_inode_glops, CREATE,
>                                        &ip->i_gl);
> @@ -141,9 +142,12 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type,
>                 if (unlikely(error))
>                         goto fail;
>
> -               if (blktype != GFS2_BLKST_UNLINKED)
> +               if (blktype == GFS2_BLKST_UNLINKED)
> +                       extra_flags |= LM_FLAG_TRY;
> +               else
>                         gfs2_cancel_delete_work(io_gl);
> -               error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED, GL_EXACT,
> +               error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED,
> +                                          GL_EXACT | extra_flags,
>                                            &ip->i_iopen_gh);
>                 gfs2_glock_put(io_gl);
>                 if (unlikely(error))
> @@ -210,6 +214,8 @@ struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int type,
>         return inode;
>
>  fail:
> +       if (error == GLR_TRYFAILED)
> +               error = -EAGAIN;
>         if (gfs2_holder_initialized(&ip->i_iopen_gh))
>                 gfs2_glock_dq_uninit(&ip->i_iopen_gh);
>         if (gfs2_holder_initialized(&i_gh))
> --
> 2.37.2
>

Pushed to for-next.

Thanks,
Andreas


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-08-25 15:59 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-25 14:53 [Cluster-devel] [PATCH] gfs2: Use TRY lock in gfs2_inode_lookup for UNLINKED inodes Bob Peterson
2022-08-25 15:59 ` Andreas Gruenbacher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.