cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: Alexander Aring <aahringo@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH v6.4-rc2 3/5] fs: dlm: switch posix lock to killable only
Date: Fri, 19 May 2023 11:21:26 -0400	[thread overview]
Message-ID: <20230519152128.65272-3-aahringo@redhat.com> (raw)
In-Reply-To: <20230519152128.65272-1-aahringo@redhat.com>

This patch will revert commit a6b1533e9a57 ("dlm: make posix locks
interruptible"). It was probably introduced to reach the fcntl()
F_SETLKW requirement to make fcntl() calls interruptible, see:

"F_SETLKW (struct flock *):

 As for F_SETLK, but if a conflicting lock is held on the file,
 then wait for that lock to be released. If a signal is caught
 while waiting, then the call is interrupted and (after the signal
 handler has returned) returns immediately (with return value -1
 and errno set to EINTR; see signal(7))."

This requires to interrupt the current plock operation in question
sitting in the wait_event_interruptible() waiting for op->done becomes
true. This isn't currently the case as do_unlock_close() will act like
the process was killed as it unlocks all previously acquired locks which
can occur into data corruption because the process still thinks it has
the previously acquired locks still acquired.

To test this behaviour a ltp testcase was created [0]. After this patch
the process can't be interrupted anymore which is against the API
but considered currently as a limitation of DLM. However it will stop to
unlock all previously acquired locks and the user process isn't aware of
it.

It requires more work in DLM to support such feature as intended. It
requires some lock request cancellation request which does not yet
exists for dlm plock user space communication. As this feature never
worked as intended and have side effects as mentioned aboe this patch moves
the wait to killable again.

[0] https://gitlab.com/netcoder/ltp/-/blob/dlm_fcntl_owner_testcase/testcases/kernel/syscalls/fcntl/fcntl42.c

Cc: stable at vger.kernel.org
Fixes: a6b1533e9a57 ("dlm: make posix locks interruptible")
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/plock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dlm/plock.c b/fs/dlm/plock.c
index fea2157fac5b..31bc601ee3d8 100644
--- a/fs/dlm/plock.c
+++ b/fs/dlm/plock.c
@@ -155,7 +155,7 @@ int dlm_posix_lock(dlm_lockspace_t *lockspace, u64 number, struct file *file,
 
 	send_op(op);
 
-	rv = wait_event_interruptible(recv_wq, (op->done != 0));
+	rv = wait_event_killable(recv_wq, (op->done != 0));
 	if (rv == -ERESTARTSYS) {
 		spin_lock(&ops_lock);
 		/* recheck under ops_lock if we got a done != 0,
-- 
2.31.1


  parent reply	other threads:[~2023-05-19 15:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-19 15:21 [Cluster-devel] [PATCH v6.4-rc2 1/5] fs: dlm: change local pids to be positive pids Alexander Aring
2023-05-19 15:21 ` [Cluster-devel] [PATCH v6.4-rc2 2/5] fs: dlm: fix cleanup pending ops when interrupted Alexander Aring
2023-05-19 15:21 ` Alexander Aring [this message]
2023-05-19 15:21 ` [Cluster-devel] [PATCH v6.4-rc2 4/5] fs: dlm: make F_SETLK not killable Alexander Aring
2023-05-19 15:21 ` [Cluster-devel] [PATCH v6.4-rc2 5/5] fs: dlm: avoid F_SETLKW plock op lookup collisions Alexander Aring
2023-05-19 21:20   ` Alexander Aring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230519152128.65272-3-aahringo@redhat.com \
    --to=aahringo@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).