Hi,
On Tue, Jul 18, 2023 at 2:07 PM Alexander Aring <aahri...@redhat.com> wrote:
>
> This patch fixes the current handling of F_CANCELLK by not just doing a
> unlock as we need to try to cancel a lock at first. A unlock makes sense
> on a non-blocking lock request but if it's a blocking lock request we
> need to cancel the request until it's not granted yet. This patch is fixing
> this behaviour by first try to cancel a lock request and if it's failed
> it's unlocking the lock which seems to be granted.
>
> Note: currently the nfs locking handling was disabled by commit
> 40595cdc93ed ("nfs: block notification on fs with its own ->lock").
> However DLM was never being updated regarding to this change. Future
> patches will try to fix lockd lock requests for DLM. This patch is
> currently assuming the upstream DLM lockd handling is correct.
>
> Signed-off-by: Alexander Aring <aahri...@redhat.com>
> ---
> fs/dlm/plock.c | 102 +++++++++++++++++++++++++++++++++-----
> fs/gfs2/file.c | 9 ++--
> fs/ocfs2/stack_user.c | 13 ++---
> include/linux/dlm_plock.h | 2 +
> 4 files changed, 97 insertions(+), 29 deletions(-)
>
> diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
> index a8ffa0760913..84510994b177 100644
> --- a/fs/dlm/plock.c
> +++ b/fs/dlm/plock.c
> @@ -42,6 +42,27 @@ static inline void set_version(struct dlm_plock_info *info)
> info->version[2] = DLM_PLOCK_VERSION_PATCH;
> }
>
> +static struct plock_op *plock_lookup_waiter(const struct dlm_plock_info
> *info)
> +{
> + struct plock_op *op = NULL, *iter;
> +
> + list_for_each_entry(iter, &recv_list, list) {
> + if (iter->info.fsid == info->fsid &&
> + iter->info.number == info->number &&
> + iter->info.owner == info->owner &&
> + iter->info.pid == info->pid &&
> + iter->info.start == info->start &&
> + iter->info.end == info->end &&
> + iter->info.ex == info->ex &&
> + iter->info.wait) {
> + op = iter;
> + break;
> + }
> + }
> +
> + return op;
> +}
> +
> static int check_version(struct dlm_plock_info *info)
> {
> if ((DLM_PLOCK_VERSION_MAJOR != info->version[0]) ||
> @@ -334,6 +355,73 @@ int dlm_posix_unlock(dlm_lockspace_t *lockspace, u64
> number, struct file *file,
> }
> EXPORT_SYMBOL_GPL(dlm_posix_unlock);
>
> +/*
> + * NOTE: This implementation can only handle async lock requests as nfs
> + * do it. It cannot handle cancellation of a pending lock request sitting
> + * in wait_event(), but for now only nfs is the only user local kernel
> + * user.
> + */
> +int dlm_posix_cancel(dlm_lockspace_t *lockspace, u64 number, struct file
> *file,
> + struct file_lock *fl)
> +{
> + struct dlm_plock_info info;
> + struct plock_op *op;
> + struct dlm_ls *ls;
> + int rv;
> +
> + /* this only works for async request for now and nfs is the only
> + * kernel user right now.
> + */
> + if (WARN_ON_ONCE(!fl->fl_lmops || !fl->fl_lmops->lm_grant))
> + return -EOPNOTSUPP;
> +
> + ls = dlm_find_lockspace_local(lockspace);
> + if (!ls)
> + return -EINVAL;
> +
> + info.pid = fl->fl_pid;
> + info.ex = (fl->fl_type == F_WRLCK);
> + info.fsid = ls->ls_global_id;
> + dlm_put_lockspace(ls);
> + info.number = number;
> + info.start = fl->fl_start;
> + info.end = fl->fl_end;
> + info.owner = (__u64)fl->fl_pid;
> +
> + rv = do_lock_cancel(&info);
> + switch (rv) {
> + case 0:
> + spin_lock(&ops_lock);
> + /* lock request to cancel must be on recv_list because
> + * do_lock_cancel() synchronizes it.
> + */
> + op = plock_lookup_waiter(&info);
> + if (WARN_ON_ONCE(!op)) {
> + rv = -ENOLCK;
> + break;
missing spin_unlock() here. I will add it to my upcoming patch series.
- Alex
