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 <aahringo@xxxxxxxxxx>
> ---
> 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 <jlayton@xxxxxxxxxx>
