[Cluster-devel] [PATCH] gfs2: Use TRY lock in gfs2_inode_lookup for UNLINKED inodes

[Andreas Gruenbacher <agruenba@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