[Cluster-devel] [GFS2 v3 PATCH 09/13] gfs2: split glock instantiation off from do_promote

[Andreas Gruenbacher <agruenba@redhat.com>]

On Mon, Oct 11, 2021 at 9:41 PM Bob Peterson <rpeterso@redhat.com> wrote:
> Before this patch, function do_promote had a section of code that did
> the actual instantiation.  This patch splits that off into its own
> function, gfs2_instantiate, which prepares us for the next patch that
> will use that function.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/gfs2/glock.c | 47 +++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 37 insertions(+), 10 deletions(-)
>
> diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> index 1d64b45d1ea9..39bfac34e8a4 100644
> --- a/fs/gfs2/glock.c
> +++ b/fs/gfs2/glock.c
> @@ -380,6 +380,33 @@ static void do_error(struct gfs2_glock *gl, const int ret)
>         }
>  }
>
> +/**
> + * gfs2_instantiate - Call the glops instantiate function
> + * @gl: The glock
> + *
> + * Returns: 0 if instantiate was successful, 2 if type specific operation is
> + * underway, or error.
> + */
> +static int gfs2_instantiate(struct gfs2_holder *gh)
> +{
> +       struct gfs2_glock *gl = gh->gh_gl;
> +       const struct gfs2_glock_operations *glops = gl->gl_ops;
> +       int ret;
> +
> +       ret = glops->go_instantiate(gh);
> +       switch(ret) {
> +       case 0:
> +               break;
> +       case 1:
> +               ret = 2;
> +               break;
> +       default:
> +               gh->gh_error = ret;
> +               break;
> +       }
> +       return ret;
> +}

Moving the return value tweaking here doesn't help, it only
complicates gfs2_instantiate. So let me revert that.

> +
>  /**
>   * do_promote - promote as many requests as possible on the current queue
>   * @gl: The glock
> @@ -392,7 +419,6 @@ static int do_promote(struct gfs2_glock *gl)
>  __releases(&gl->gl_lockref.lock)
>  __acquires(&gl->gl_lockref.lock)
>  {
> -       const struct gfs2_glock_operations *glops = gl->gl_ops;
>         struct gfs2_holder *gh, *tmp;
>         bool lock_released;
>         int ret;
> @@ -409,19 +435,20 @@ __acquires(&gl->gl_lockref.lock)
>                         break;
>                 }
>                 if (gh->gh_list.prev == &gl->gl_holders &&
> -                   !(gh->gh_flags & GL_SKIP) && glops->go_instantiate) {
> +                   !(gh->gh_flags & GL_SKIP) && gl->gl_ops->go_instantiate) {
>                         lock_released = true;
>                         spin_unlock(&gl->gl_lockref.lock);
> -                       ret = glops->go_instantiate(gh);
> +                       ret = gfs2_instantiate(gh);
>                         spin_lock(&gl->gl_lockref.lock);
>                         if (ret) {
> -                               if (ret == 1)
> -                                       return 2;
> -                               gh->gh_error = ret;
> -                               list_del_init(&gh->gh_list);
> -                               trace_gfs2_glock_queue(gh, 0);
> -                               gfs2_holder_wake(gh);
> -                               goto restart;
> +                               if (ret == 2)
> +                                       return ret;
> +                               else {
> +                                       list_del_init(&gh->gh_list);
> +                                       trace_gfs2_glock_queue(gh, 0);
> +                                       gfs2_holder_wake(gh);
> +                                       goto restart;
> +                               }

We can replace the "else { XXX }" with "XXX" here.

>                         }
>                 }
>                 set_bit(HIF_HOLDER, &gh->gh_iflags);
> --
> 2.31.1
>

Thanks,
Andreas