On Wed, 2023-08-23 at 17:33 -0400, Alexander Aring wrote:
> This patch returns nlm_lck_blocked in nlmsvc_lock() when an asynchronous
> lock request is pending. During testing I ran into the case with the
> side-effects that lockd is waiting for only one lm_grant() callback
> because it's already part of the nlm_blocked list. If another
> asynchronous for the same nlm_block is triggered two lm_grant()
> callbacks will occur but lockd was only waiting for one.
> 
> To avoid any change of existing users this handling will only being made
> when export_op_support_safe_async_lock() returns true.
> 
> Signed-off-by: Alexander Aring <aahri...@redhat.com>
> ---
>  fs/lockd/svclock.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
> index 6e3b230e8317..aa4174fbaf5b 100644
> --- a/fs/lockd/svclock.c
> +++ b/fs/lockd/svclock.c
> @@ -531,6 +531,23 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file 
> *file,
>               goto out;
>       }
>  
> +     spin_lock(&nlm_blocked_lock);
> +     /*
> +      * If this is a lock request for an already pending
> +      * lock request we return nlm_lck_blocked without calling
> +      * vfs_lock_file() again. Otherwise we have two pending
> +      * requests on the underlaying ->lock() implementation but
> +      * only one nlm_block to being granted by lm_grant().
> +      */
> +     if (export_op_support_safe_async_lock(inode->i_sb->s_export_op,
> +                                           nlmsvc_file_file(file)->f_op) &&
> +         !list_empty(&block->b_list)) {
> +             spin_unlock(&nlm_blocked_lock);
> +             ret = nlm_lck_blocked;
> +             goto out;
> +     }

Looks reasonable. The block->b_list check is subtle, but the comment
helps.

> +     spin_unlock(&nlm_blocked_lock);
> +
>       if (!wait)
>               lock->fl.fl_flags &= ~FL_SLEEP;
>       mode = lock_to_openmode(&lock->fl);
> @@ -543,13 +560,6 @@ nlmsvc_lock(struct svc_rqst *rqstp, struct nlm_file 
> *file,
>                       ret = nlm_granted;
>                       goto out;
>               case -EAGAIN:
> -                     /*
> -                      * If this is a blocking request for an
> -                      * already pending lock request then we need
> -                      * to put it back on lockd's block list
> -                      */
> -                     if (wait)
> -                             break;
>                       ret = async_block ? nlm_lck_blocked : nlm_lck_denied;
>                       goto out;
>               case FILE_LOCK_DEFERRED:


Reviewed-by: Jeff Layton <jlay...@kernel.org>

Reply via email to