Hi,
On Thu, Jul 13, 2023 at 10:49 AM Greg KH gregkh@linuxfoundation.org wrote:
On Thu, Jul 13, 2023 at 10:40:28AM -0400, Alexander Aring wrote:
This patch introduces a new flag DLM_PLOCK_FL_NO_REPLY in case an dlm plock operation should not send a reply back. Currently this is kind of being handled in DLM_PLOCK_FL_CLOSE, but DLM_PLOCK_FL_CLOSE has more meanings that it will remove all waiters for a specific nodeid/owner values in by doing a unlock operation. In case of an error in dlm user space software e.g. dlm_controld we get an reply with an error back. This cannot be matched because there is no op to match in recv_list. We filter now on DLM_PLOCK_FL_NO_REPLY in case we had an error back as reply. In newer dlm_controld version it will never send a result back when DLM_PLOCK_FL_NO_REPLY is set. This filter is a workaround to handle older dlm_controld versions.
Fixes: 901025d2f319 ("dlm: make plock operation killable") Cc: stable@vger.kernel.org Signed-off-by: Alexander Aring aahringo@redhat.com
Why is adding a new uapi a stable patch?
because the user space is just to copy the flags back to the kernel. I thought it would work. :)
fs/dlm/plock.c | 23 +++++++++++++++++++---- include/uapi/linux/dlm_plock.h | 1 + 2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c index 70a4752ed913..7fe9f4b922d3 100644 --- a/fs/dlm/plock.c +++ b/fs/dlm/plock.c @@ -96,7 +96,7 @@ static void do_unlock_close(const struct dlm_plock_info *info) op->info.end = OFFSET_MAX; op->info.owner = info->owner;
op->info.flags |= DLM_PLOCK_FL_CLOSE;
op->info.flags |= (DLM_PLOCK_FL_CLOSE | DLM_PLOCK_FL_NO_REPLY); send_op(op);}
@@ -293,7 +293,7 @@ int dlm_posix_unlock(dlm_lockspace_t *lockspace, u64 number, struct file *file, op->info.owner = (__u64)(long) fl->fl_owner;
if (fl->fl_flags & FL_CLOSE) {
op->info.flags |= DLM_PLOCK_FL_CLOSE;
op->info.flags |= (DLM_PLOCK_FL_CLOSE | DLM_PLOCK_FL_NO_REPLY); send_op(op); rv = 0; goto out;@@ -392,7 +392,7 @@ static ssize_t dev_read(struct file *file, char __user *u, size_t count, spin_lock(&ops_lock); if (!list_empty(&send_list)) { op = list_first_entry(&send_list, struct plock_op, list);
if (op->info.flags & DLM_PLOCK_FL_CLOSE)
if (op->info.flags & DLM_PLOCK_FL_NO_REPLY) list_del(&op->list); else list_move_tail(&op->list, &recv_list);@@ -407,7 +407,7 @@ static ssize_t dev_read(struct file *file, char __user *u, size_t count, that were generated by the vfs cleaning up for a close (the process did not make an unlock call). */
if (op->info.flags & DLM_PLOCK_FL_CLOSE)
if (op->info.flags & DLM_PLOCK_FL_NO_REPLY) dlm_release_plock_op(op); if (copy_to_user(u, &info, sizeof(info)))@@ -433,6 +433,21 @@ static ssize_t dev_write(struct file *file, const char __user *u, size_t count, if (check_version(&info)) return -EINVAL;
/* Some old dlm user space software will send replies back,* even if DLM_PLOCK_FL_NO_REPLY is set (because the flag is* new) e.g. if a error occur. We can't match them in recv_list* because they were never be part of it. We filter it here,* new dlm user space software will filter it in user space.** In future this handling can be removed.*/if (info.flags & DLM_PLOCK_FL_NO_REPLY) {pr_info("Received unexpected reply from op %d, ""please update DLM user space software!\n",info.optype);Never allow userspace to spam the kernel log. And this is not going to work, you need to handle the error and at most, report this to userspace once.
I will ignore handling this issue for older kernels because it would probably be fine that the user space never gets an invalid value handled.
Also, don't wrap your strings, checkpatch should have told you this.
That is correct and I was ignoring it as the implementation has another wrapped string somewhere else. It is a warning not an error.
Will send a v2 to not wrap the string around and drop Fixes and cc stable.
- Alex