* [LTP] [PATCH 1/5] fcntl40: test for owner values on classic posix lock
2023-05-30 20:37 [LTP] [PATCH 0/5] fcntl: add more testcases Alexander Aring
@ 2023-05-30 20:37 ` Alexander Aring
2023-06-21 9:03 ` Petr Vorel
2023-05-30 20:37 ` [LTP] [PATCH 2/5] fcntl41: test for owner values on OFD posix locks Alexander Aring
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Alexander Aring @ 2023-05-30 20:37 UTC (permalink / raw)
To: ltp
This patch adds fcntl40 to test similar owner values for classical owner
locks. There was an issue been found in the gfs2 filesystem because
there can be collisions with identical owner values.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
testcases/kernel/syscalls/fcntl/.gitignore | 2 +
testcases/kernel/syscalls/fcntl/Makefile | 3 +
testcases/kernel/syscalls/fcntl/fcntl40.c | 188 +++++++++++++++++++++
3 files changed, 193 insertions(+)
create mode 100644 testcases/kernel/syscalls/fcntl/fcntl40.c
diff --git a/testcases/kernel/syscalls/fcntl/.gitignore b/testcases/kernel/syscalls/fcntl/.gitignore
index 10cb0995f..e558cfe83 100644
--- a/testcases/kernel/syscalls/fcntl/.gitignore
+++ b/testcases/kernel/syscalls/fcntl/.gitignore
@@ -74,3 +74,5 @@
/fcntl38_64
/fcntl39
/fcntl39_64
+/fcntl40
+/fcntl40_64
diff --git a/testcases/kernel/syscalls/fcntl/Makefile b/testcases/kernel/syscalls/fcntl/Makefile
index df663a50a..c3196a527 100644
--- a/testcases/kernel/syscalls/fcntl/Makefile
+++ b/testcases/kernel/syscalls/fcntl/Makefile
@@ -12,6 +12,9 @@ fcntl34_64: LDLIBS += -lpthread
fcntl36: LDLIBS += -lpthread
fcntl36_64: LDLIBS += -lpthread
+fcntl40: LDLIBS += -lpthread
+fcntl40_64: LDLIBS += -lpthread
+
include $(top_srcdir)/include/mk/testcases.mk
include $(abs_srcdir)/../utils/newer_64.mk
diff --git a/testcases/kernel/syscalls/fcntl/fcntl40.c b/testcases/kernel/syscalls/fcntl/fcntl40.c
new file mode 100644
index 000000000..829685436
--- /dev/null
+++ b/testcases/kernel/syscalls/fcntl/fcntl40.c
@@ -0,0 +1,188 @@
+/*
+ * [Description]
+ * Tests gfs2 dlm posix op queue handling in the kernel.
+ * It is recommended to run watch -n 0.1 "dlm_tool plocks $LS"
+ * aside to monitor dlm plock handling.
+ *
+ * [How to use it]
+ * Call it with TMPDIR=/mnt ./fcntl40 where TMPDIR is a gfs2 mountpoint.
+ * Try it on other filesystems to compare results.
+ *
+ * [What's it doing]
+ *
+ * The test shows that we currently have problems with the operation lookup
+ * functionality [0] when we using threads. The owner value is the same for two
+ * locks being in WAITING state. dlm_controld "dlm_tool plocks $LS" will show
+ * it correctly that the specific lock is not in waiting anymore. The issue
+ * begins matching the right iter->done in the kernel at dev_write() see [0].
+ *
+ * What this test does is (using dlm_controld interval range interpretation):
+ *
+ * parent:
+ *
+ * 1. lock[0-1]
+ *
+ * child:
+ *
+ * thread1:
+ *
+ * 2. lockw[1-1] - important 1-1 at first because the order of WAITING state
+ * locks matters
+ *
+ * thread2:
+ *
+ * 3. lockw[0-0]
+ *
+ * parent:
+ *
+ * 4. unlock[1-1] - will give a iter->done = 1 in [0] for lock at 3. and the
+ * application results in a deadlock
+ * 5. unlock[0-0]
+ *
+ * We have this issue also with SETLK, GETLK - it's easier to reproduce
+ * with SETLKW because dev_write() is more controlable by doing unlocks.
+ *
+ * OFD (open filedescriptor locks) are also affected and should be able
+ * to reproduce with fork() only and not threads. The owner value of [0]
+ * depends on "struct file *" pointer in this case.
+ *
+ * [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/dlm/plock.c?h=v6.3#n432
+ */
+
+#include <sys/wait.h>
+#include <pthread.h>
+
+#include "tst_safe_pthread.h"
+#include "tst_test.h"
+
+static int fd;
+
+static void *do_thread1(void *arg)
+{
+ const struct flock fl_0_0 = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 0L,
+ .l_len = 1L,
+ };
+ (void)arg;
+
+ tst_res(TINFO, "thread1 waits for thread2 to lock 1-1");
+ TST_CHECKPOINT_WAIT(1);
+
+ tst_res(TINFO, "thread1 lock region 0-0 - It should block");
+ SAFE_FCNTL(fd, F_SETLKW, &fl_0_0);
+ tst_res(TINFO, "lock region 0-0 acquired");
+
+ return NULL;
+}
+
+static void *do_thread2(void *arg)
+{
+ const struct flock fl_1_1 = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 1L,
+ .l_len = 1L,
+ };
+ (void)arg;
+
+ tst_res(TINFO, "thread1 lock region 1-1 - It should block");
+ SAFE_FCNTL(fd, F_SETLKW, &fl_1_1);
+ tst_res(TINFO, "lock region 1-1 acquired");
+
+ TST_CHECKPOINT_WAKE(2);
+
+ return NULL;
+}
+
+void do_child(void)
+{
+ pthread_t t1, t2;
+
+ SAFE_PTHREAD_CREATE(&t1, NULL, do_thread1, NULL);
+ SAFE_PTHREAD_CREATE(&t2, NULL, do_thread2, NULL);
+
+ SAFE_PTHREAD_JOIN(t1, NULL);
+ SAFE_PTHREAD_JOIN(t2, NULL);
+
+ tst_res(TPASS, "Child passed!");
+}
+
+void do_parent(void)
+{
+ struct flock fl = {
+ .l_whence = SEEK_SET,
+ };
+
+ /* wait for 1 seconds so thread2 lock 1-1 tries to acquires at first
+ * than thread1 lock 0-0 tries to acquired to have a specific waiting
+ * order in dlm posix handling.
+ */
+ sleep(1);
+ /* tell thread2 to call SETLKW for lock 0-0 */
+ TST_CHECKPOINT_WAKE(1);
+ /* wait 3 seconds for thread 1 and 2 being in waiting state */
+ sleep(3);
+
+ /* unlock 0-1, should be successful */
+ fl.l_type = F_UNLCK;
+ fl.l_start = 1;
+ fl.l_len = 1;
+ tst_res(TINFO, "unlock region 1-1 thread2");
+ SAFE_FCNTL(fd, F_SETLK, &fl);
+
+ /* wait until thread 2 got acquired and leave waiting */
+ TST_CHECKPOINT_WAIT(2);
+
+ fl.l_start = 0;
+ fl.l_len = 1;
+ fl.l_type = F_UNLCK;
+ tst_res(TINFO, "unlock region 0-0 thread2");
+ SAFE_FCNTL(fd, F_SETLK, &fl);
+}
+
+static void fcntl40_test(void)
+{
+ struct flock fl = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 0L,
+ .l_len = 2L,
+ };
+ pid_t pid;
+
+ tst_res(TINFO, "parent lock region 0-1 - should be successful");
+ SAFE_FCNTL(fd, F_SETLK, &fl);
+ tst_res(TINFO, "parent region 0-1 locked");
+
+ pid = SAFE_FORK();
+ if (pid == 0) {
+ do_child();
+ return;
+ }
+
+ do_parent();
+ wait(NULL);
+
+ tst_res(TPASS, "Parent passed!");
+}
+
+static void setup(void)
+{
+ fd = SAFE_OPEN("filename", O_RDWR | O_CREAT, 0700);
+}
+
+static void cleanup(void)
+{
+ if (fd > -1)
+ SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+ .forks_child = 1,
+ .needs_checkpoints = 1,
+ .test_all = fcntl40_test,
+ .setup = setup,
+ .cleanup = cleanup,
+};
--
2.31.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [LTP] [PATCH 1/5] fcntl40: test for owner values on classic posix lock
2023-05-30 20:37 ` [LTP] [PATCH 1/5] fcntl40: test for owner values on classic posix lock Alexander Aring
@ 2023-06-21 9:03 ` Petr Vorel
2023-06-30 19:59 ` Alexander Aring
0 siblings, 1 reply; 16+ messages in thread
From: Petr Vorel @ 2023-06-21 9:03 UTC (permalink / raw)
To: Alexander Aring; +Cc: ltp
Hi Alexander,
> This patch adds fcntl40 to test similar owner values for classical owner
> locks. There was an issue been found in the gfs2 filesystem because
> there can be collisions with identical owner values.
Thanks for your work!
...
> +++ b/testcases/kernel/syscalls/fcntl/fcntl40.c
> @@ -0,0 +1,188 @@
There is no SPDX and copyright, see other files:
// SPDX-License-Identifier: GPL-2.0-or-later
/*
* Copyright (c) 2023 ...
*/
> +/*
NOTE: this should be /*\
to be able to get the description in our automatically generated documentation
https://github.com/linux-test-project/ltp/releases/download/20230516/metadata.20230516.html
> + * [Description]
> + * Tests gfs2 dlm posix op queue handling in the kernel.
> + * It is recommended to run watch -n 0.1 "dlm_tool plocks $LS"
> + * aside to monitor dlm plock handling.
> + *
> + * [How to use it]
> + * Call it with TMPDIR=/mnt ./fcntl40 where TMPDIR is a gfs2 mountpoint.
I wonder if we could check for GFS2_MAGIC (we'd need to add it to
include/tst_fs.h => 0x01161970) and quit the test with tst_brk(TCONF) if TMPDIR
is not on gfs2.
ATM we don't have any helper in struct tst_test, which would do it.
> + * Try it on other filesystems to compare results.
> + *
> + * [What's it doing]
nit: I'd replace this with [Algorithm].
...
> +void do_child(void)
This should be static (in all files).
make check (or make check-fcntl40) is your friend.
> +{
> + pthread_t t1, t2;
> +
> + SAFE_PTHREAD_CREATE(&t1, NULL, do_thread1, NULL);
> + SAFE_PTHREAD_CREATE(&t2, NULL, do_thread2, NULL);
> +
> + SAFE_PTHREAD_JOIN(t1, NULL);
> + SAFE_PTHREAD_JOIN(t2, NULL);
> +
> + tst_res(TPASS, "Child passed!");
> +}
> +
> +void do_parent(void)
This should also be static.
> +{
> + struct flock fl = {
> + .l_whence = SEEK_SET,
> + };
> +
> + /* wait for 1 seconds so thread2 lock 1-1 tries to acquires at first
> + * than thread1 lock 0-0 tries to acquired to have a specific waiting
> + * order in dlm posix handling.
> + */
> + sleep(1);
I wonder if there could be some proactive check instead of sleep.
FYI we have undocumented TST_RETRY_FUNC() in C API.
> + /* tell thread2 to call SETLKW for lock 0-0 */
> + TST_CHECKPOINT_WAKE(1);
> + /* wait 3 seconds for thread 1 and 2 being in waiting state */
> + sleep(3);
> +
> + /* unlock 0-1, should be successful */
> + fl.l_type = F_UNLCK;
> + fl.l_start = 1;
> + fl.l_len = 1;
> + tst_res(TINFO, "unlock region 1-1 thread2");
> + SAFE_FCNTL(fd, F_SETLK, &fl);
> +
> + /* wait until thread 2 got acquired and leave waiting */
> + TST_CHECKPOINT_WAIT(2);
> +
> + fl.l_start = 0;
> + fl.l_len = 1;
> + fl.l_type = F_UNLCK;
> + tst_res(TINFO, "unlock region 0-0 thread2");
> + SAFE_FCNTL(fd, F_SETLK, &fl);
> +}
> +
> +static void fcntl40_test(void)
> +{
> + struct flock fl = {
> + .l_type = F_WRLCK,
> + .l_whence = SEEK_SET,
> + .l_start = 0L,
> + .l_len = 2L,
> + };
> + pid_t pid;
> +
> + tst_res(TINFO, "parent lock region 0-1 - should be successful");
> + SAFE_FCNTL(fd, F_SETLK, &fl);
> + tst_res(TINFO, "parent region 0-1 locked");
> +
> + pid = SAFE_FORK();
> + if (pid == 0) {
> + do_child();
> + return;
> + }
> +
> + do_parent();
> + wait(NULL);
waitpid() should be replaced by tst_reap_children(), see
https://github.com/linux-test-project/ltp/wiki/C-Test-API#18-doing-the-test-in-the-child-process
> +
> + tst_res(TPASS, "Parent passed!");
There is TPASS in child, does it really need to be in the parent as well?
> +}
> +
> +static void setup(void)
> +{
> + fd = SAFE_OPEN("filename", O_RDWR | O_CREAT, 0700);
> +}
> +
> +static void cleanup(void)
> +{
> + if (fd > -1)
> + SAFE_CLOSE(fd);
> +}
> +
> +static struct tst_test test = {
> + .forks_child = 1,
> + .needs_checkpoints = 1,
> + .test_all = fcntl40_test,
> + .setup = setup,
> + .cleanup = cleanup,
> +};
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [LTP] [PATCH 1/5] fcntl40: test for owner values on classic posix lock
2023-06-21 9:03 ` Petr Vorel
@ 2023-06-30 19:59 ` Alexander Aring
2023-07-02 19:18 ` Petr Vorel
2023-07-02 19:19 ` Petr Vorel
0 siblings, 2 replies; 16+ messages in thread
From: Alexander Aring @ 2023-06-30 19:59 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
Hi,
On Wed, Jun 21, 2023 at 5:10 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Alexander,
>
> > This patch adds fcntl40 to test similar owner values for classical owner
> > locks. There was an issue been found in the gfs2 filesystem because
> > there can be collisions with identical owner values.
>
> Thanks for your work!
>
> ...
> > +++ b/testcases/kernel/syscalls/fcntl/fcntl40.c
> > @@ -0,0 +1,188 @@
> There is no SPDX and copyright, see other files:
>
> // SPDX-License-Identifier: GPL-2.0-or-later
> /*
> * Copyright (c) 2023 ...
> */
>
> > +/*
> NOTE: this should be /*\
> to be able to get the description in our automatically generated documentation
>
> https://github.com/linux-test-project/ltp/releases/download/20230516/metadata.20230516.html
>
ok
> > + * [Description]
> > + * Tests gfs2 dlm posix op queue handling in the kernel.
> > + * It is recommended to run watch -n 0.1 "dlm_tool plocks $LS"
> > + * aside to monitor dlm plock handling.
> > + *
> > + * [How to use it]
> > + * Call it with TMPDIR=/mnt ./fcntl40 where TMPDIR is a gfs2 mountpoint.
> I wonder if we could check for GFS2_MAGIC (we'd need to add it to
> include/tst_fs.h => 0x01161970) and quit the test with tst_brk(TCONF) if TMPDIR
> is not on gfs2.
>
> ATM we don't have any helper in struct tst_test, which would do it.
>
I will mention that gfs2 is only an example here. It becomes
interesting when a file system implements its own .lock() callback OR
if somebody wants to test file system core, when a filesystem does not
implement its own .lock().
> > + * Try it on other filesystems to compare results.
> > + *
> > + * [What's it doing]
> nit: I'd replace this with [Algorithm].
>
ok.
> ...
> > +void do_child(void)
> This should be static (in all files).
>
> make check (or make check-fcntl40) is your friend.
>
ok. Thanks for telling me about make check.
> > +{
> > + pthread_t t1, t2;
> > +
> > + SAFE_PTHREAD_CREATE(&t1, NULL, do_thread1, NULL);
> > + SAFE_PTHREAD_CREATE(&t2, NULL, do_thread2, NULL);
> > +
> > + SAFE_PTHREAD_JOIN(t1, NULL);
> > + SAFE_PTHREAD_JOIN(t2, NULL);
> > +
> > + tst_res(TPASS, "Child passed!");
> > +}
> > +
> > +void do_parent(void)
> This should also be static.
>
ok.
> > +{
> > + struct flock fl = {
> > + .l_whence = SEEK_SET,
> > + };
> > +
> > + /* wait for 1 seconds so thread2 lock 1-1 tries to acquires at first
> > + * than thread1 lock 0-0 tries to acquired to have a specific waiting
> > + * order in dlm posix handling.
> > + */
> > + sleep(1);
>
> I wonder if there could be some proactive check instead of sleep.
> FYI we have undocumented TST_RETRY_FUNC() in C API.
>
I try to look into it.
> > + /* tell thread2 to call SETLKW for lock 0-0 */
> > + TST_CHECKPOINT_WAKE(1);
> > + /* wait 3 seconds for thread 1 and 2 being in waiting state */
> > + sleep(3);
> > +
> > + /* unlock 0-1, should be successful */
> > + fl.l_type = F_UNLCK;
> > + fl.l_start = 1;
> > + fl.l_len = 1;
> > + tst_res(TINFO, "unlock region 1-1 thread2");
> > + SAFE_FCNTL(fd, F_SETLK, &fl);
> > +
> > + /* wait until thread 2 got acquired and leave waiting */
> > + TST_CHECKPOINT_WAIT(2);
> > +
> > + fl.l_start = 0;
> > + fl.l_len = 1;
> > + fl.l_type = F_UNLCK;
> > + tst_res(TINFO, "unlock region 0-0 thread2");
> > + SAFE_FCNTL(fd, F_SETLK, &fl);
> > +}
> > +
> > +static void fcntl40_test(void)
> > +{
> > + struct flock fl = {
> > + .l_type = F_WRLCK,
> > + .l_whence = SEEK_SET,
> > + .l_start = 0L,
> > + .l_len = 2L,
> > + };
> > + pid_t pid;
> > +
> > + tst_res(TINFO, "parent lock region 0-1 - should be successful");
> > + SAFE_FCNTL(fd, F_SETLK, &fl);
> > + tst_res(TINFO, "parent region 0-1 locked");
> > +
> > + pid = SAFE_FORK();
> > + if (pid == 0) {
> > + do_child();
> > + return;
> > + }
> > +
> > + do_parent();
> > + wait(NULL);
>
> waitpid() should be replaced by tst_reap_children(), see
> https://github.com/linux-test-project/ltp/wiki/C-Test-API#18-doing-the-test-in-the-child-process
>
ok. Thanks.
> > +
> > + tst_res(TPASS, "Parent passed!");
> There is TPASS in child, does it really need to be in the parent as well?
no.
- Alex
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [LTP] [PATCH 1/5] fcntl40: test for owner values on classic posix lock
2023-06-30 19:59 ` Alexander Aring
@ 2023-07-02 19:18 ` Petr Vorel
2023-07-05 13:23 ` Alexander Aring
2023-07-02 19:19 ` Petr Vorel
1 sibling, 1 reply; 16+ messages in thread
From: Petr Vorel @ 2023-07-02 19:18 UTC (permalink / raw)
To: Alexander Aring; +Cc: ltp
Hi Alex,
...
> > > + * [Description]
> > > + * Tests gfs2 dlm posix op queue handling in the kernel.
> > > + * It is recommended to run watch -n 0.1 "dlm_tool plocks $LS"
> > > + * aside to monitor dlm plock handling.
> > > + *
> > > + * [How to use it]
> > > + * Call it with TMPDIR=/mnt ./fcntl40 where TMPDIR is a gfs2 mountpoint.
> > I wonder if we could check for GFS2_MAGIC (we'd need to add it to
> > include/tst_fs.h => 0x01161970) and quit the test with tst_brk(TCONF) if TMPDIR
> > is not on gfs2.
> > ATM we don't have any helper in struct tst_test, which would do it.
> I will mention that gfs2 is only an example here. It becomes
> interesting when a file system implements its own .lock() callback OR
> if somebody wants to test file system core, when a filesystem does not
> implement its own .lock().
I see .lock is implemented in 9p, afs, ceph, cifs, ocfs2, orangefs, even NFS.
"file system core": do you mean VFS? Because that would be more usable than the
filesystems above (which are quite exotic).
...
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [LTP] [PATCH 1/5] fcntl40: test for owner values on classic posix lock
2023-07-02 19:18 ` Petr Vorel
@ 2023-07-05 13:23 ` Alexander Aring
2023-07-07 8:14 ` Petr Vorel
0 siblings, 1 reply; 16+ messages in thread
From: Alexander Aring @ 2023-07-05 13:23 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
Hi,
On Sun, Jul 2, 2023 at 3:18 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Alex,
>
> ...
> > > > + * [Description]
> > > > + * Tests gfs2 dlm posix op queue handling in the kernel.
> > > > + * It is recommended to run watch -n 0.1 "dlm_tool plocks $LS"
> > > > + * aside to monitor dlm plock handling.
> > > > + *
> > > > + * [How to use it]
> > > > + * Call it with TMPDIR=/mnt ./fcntl40 where TMPDIR is a gfs2 mountpoint.
> > > I wonder if we could check for GFS2_MAGIC (we'd need to add it to
> > > include/tst_fs.h => 0x01161970) and quit the test with tst_brk(TCONF) if TMPDIR
> > > is not on gfs2.
>
> > > ATM we don't have any helper in struct tst_test, which would do it.
>
>
> > I will mention that gfs2 is only an example here. It becomes
> > interesting when a file system implements its own .lock() callback OR
> > if somebody wants to test file system core, when a filesystem does not
> > implement its own .lock().
>
> I see .lock is implemented in 9p, afs, ceph, cifs, ocfs2, orangefs, even NFS.
> "file system core": do you mean VFS? Because that would be more usable than the
> filesystems above (which are quite exotic).
>
It depends here what they are doing in .lock() - If they just call
locks_lock_file_wait() or similar helpers depending on the call, then
they don't do much different than what vfs is doing.
In case of gfs2/ocfs it is very special, it redirects their calls to
DLM and DLM has its own whole posix implementation behind it. We only
call locks_lock_file_wait() to keep the Linux bookkeeping in
/proc/locks.
What I am doing here is mostly trusting the Linux implementation and
comparing results from e.g. tmpfs with GFS2 and I found issues.
For now I trust the Linux implementation and check if we have a
different result here. I need to be very careful here because
GFS2/OCFS2 are cluster filesystems and the fcntl() interface was never
made for cluster filesystems. However I currently only test "one node"
locking and this should deliver the same results as tmpfs, etc.
- Alex
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [LTP] [PATCH 1/5] fcntl40: test for owner values on classic posix lock
2023-07-05 13:23 ` Alexander Aring
@ 2023-07-07 8:14 ` Petr Vorel
2023-07-07 12:50 ` Alexander Aring
0 siblings, 1 reply; 16+ messages in thread
From: Petr Vorel @ 2023-07-07 8:14 UTC (permalink / raw)
To: Alexander Aring; +Cc: ltp
> Hi,
> On Sun, Jul 2, 2023 at 3:18 PM Petr Vorel <pvorel@suse.cz> wrote:
> > Hi Alex,
> > ...
> > > > > + * [Description]
> > > > > + * Tests gfs2 dlm posix op queue handling in the kernel.
> > > > > + * It is recommended to run watch -n 0.1 "dlm_tool plocks $LS"
> > > > > + * aside to monitor dlm plock handling.
> > > > > + *
> > > > > + * [How to use it]
> > > > > + * Call it with TMPDIR=/mnt ./fcntl40 where TMPDIR is a gfs2 mountpoint.
> > > > I wonder if we could check for GFS2_MAGIC (we'd need to add it to
> > > > include/tst_fs.h => 0x01161970) and quit the test with tst_brk(TCONF) if TMPDIR
> > > > is not on gfs2.
> > > > ATM we don't have any helper in struct tst_test, which would do it.
> > > I will mention that gfs2 is only an example here. It becomes
> > > interesting when a file system implements its own .lock() callback OR
> > > if somebody wants to test file system core, when a filesystem does not
> > > implement its own .lock().
> > I see .lock is implemented in 9p, afs, ceph, cifs, ocfs2, orangefs, even NFS.
> > "file system core": do you mean VFS? Because that would be more usable than the
> > filesystems above (which are quite exotic).
> It depends here what they are doing in .lock() - If they just call
> locks_lock_file_wait() or similar helpers depending on the call, then
> they don't do much different than what vfs is doing.
> In case of gfs2/ocfs it is very special, it redirects their calls to
> DLM and DLM has its own whole posix implementation behind it. We only
> call locks_lock_file_wait() to keep the Linux bookkeeping in
> /proc/locks.
> What I am doing here is mostly trusting the Linux implementation and
> comparing results from e.g. tmpfs with GFS2 and I found issues.
> For now I trust the Linux implementation and check if we have a
> different result here. I need to be very careful here because
> GFS2/OCFS2 are cluster filesystems and the fcntl() interface was never
> made for cluster filesystems. However I currently only test "one node"
> locking and this should deliver the same results as tmpfs, etc.
Thanks for info. I'm still not sure if this is useful only for gfs2/ocfs
and we should prepare block device with gfs2 or ocfs and test filesystem on it.
Or if it would be useful to test other filesystem implementation. In this latter
case we usually use .all_filesystems (we don't have proper docs for
.all_filesystems, the closest is [1]) and test is then run on various common
filesystems (see fs_type_whitelist[] in lib/tst_supported_fs_types.c), but in
that case gfs2 would be skipped (it's not a common filesystem).
In case preparing block device with gfs2 something like this might work:
static struct tst_test test = {
...
.dev_fs_type = "gfs2",
.format_device = 1,
.dev_fs_opts = (const char *const []){ "-t", "mycluster:mygfs2", "-p", "lock_dlm", "-j" , "2" , NULL},
Kind regards,
Petr
[1] https://github.com/linux-test-project/ltp/wiki/C-Test-API#113-filesystem-type-detection-and-skiplist
> - Alex
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [LTP] [PATCH 1/5] fcntl40: test for owner values on classic posix lock
2023-07-07 8:14 ` Petr Vorel
@ 2023-07-07 12:50 ` Alexander Aring
2023-07-07 13:17 ` Petr Vorel
0 siblings, 1 reply; 16+ messages in thread
From: Alexander Aring @ 2023-07-07 12:50 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
Hi,
On Fri, Jul 7, 2023 at 4:14 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> > Hi,
>
> > On Sun, Jul 2, 2023 at 3:18 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > > Hi Alex,
>
> > > ...
> > > > > > + * [Description]
> > > > > > + * Tests gfs2 dlm posix op queue handling in the kernel.
> > > > > > + * It is recommended to run watch -n 0.1 "dlm_tool plocks $LS"
> > > > > > + * aside to monitor dlm plock handling.
> > > > > > + *
> > > > > > + * [How to use it]
> > > > > > + * Call it with TMPDIR=/mnt ./fcntl40 where TMPDIR is a gfs2 mountpoint.
> > > > > I wonder if we could check for GFS2_MAGIC (we'd need to add it to
> > > > > include/tst_fs.h => 0x01161970) and quit the test with tst_brk(TCONF) if TMPDIR
> > > > > is not on gfs2.
>
> > > > > ATM we don't have any helper in struct tst_test, which would do it.
>
>
> > > > I will mention that gfs2 is only an example here. It becomes
> > > > interesting when a file system implements its own .lock() callback OR
> > > > if somebody wants to test file system core, when a filesystem does not
> > > > implement its own .lock().
>
> > > I see .lock is implemented in 9p, afs, ceph, cifs, ocfs2, orangefs, even NFS.
> > > "file system core": do you mean VFS? Because that would be more usable than the
> > > filesystems above (which are quite exotic).
>
>
> > It depends here what they are doing in .lock() - If they just call
> > locks_lock_file_wait() or similar helpers depending on the call, then
> > they don't do much different than what vfs is doing.
> > In case of gfs2/ocfs it is very special, it redirects their calls to
> > DLM and DLM has its own whole posix implementation behind it. We only
> > call locks_lock_file_wait() to keep the Linux bookkeeping in
> > /proc/locks.
>
> > What I am doing here is mostly trusting the Linux implementation and
> > comparing results from e.g. tmpfs with GFS2 and I found issues.
>
> > For now I trust the Linux implementation and check if we have a
> > different result here. I need to be very careful here because
> > GFS2/OCFS2 are cluster filesystems and the fcntl() interface was never
> > made for cluster filesystems. However I currently only test "one node"
> > locking and this should deliver the same results as tmpfs, etc.
>
> Thanks for info. I'm still not sure if this is useful only for gfs2/ocfs
> and we should prepare block device with gfs2 or ocfs and test filesystem on it.
> Or if it would be useful to test other filesystem implementation. In this latter
> case we usually use .all_filesystems (we don't have proper docs for
> .all_filesystems, the closest is [1]) and test is then run on various common
> filesystems (see fs_type_whitelist[] in lib/tst_supported_fs_types.c), but in
> that case gfs2 would be skipped (it's not a common filesystem).
>
> In case preparing block device with gfs2 something like this might work:
>
> static struct tst_test test = {
> ...
> .dev_fs_type = "gfs2",
> .format_device = 1,
> .dev_fs_opts = (const char *const []){ "-t", "mycluster:mygfs2", "-p", "lock_dlm", "-j" , "2" , NULL},
>
Can I override this setting by some ENV because I actually want to run
it on a different filesystem which is using VFS posix lock
implementation, because as I said I want to compare the results.
- Alex
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [LTP] [PATCH 1/5] fcntl40: test for owner values on classic posix lock
2023-07-07 12:50 ` Alexander Aring
@ 2023-07-07 13:17 ` Petr Vorel
0 siblings, 0 replies; 16+ messages in thread
From: Petr Vorel @ 2023-07-07 13:17 UTC (permalink / raw)
To: Alexander Aring; +Cc: ltp
> Hi,
> On Fri, Jul 7, 2023 at 4:14 AM Petr Vorel <pvorel@suse.cz> wrote:
> > > Hi,
> > > On Sun, Jul 2, 2023 at 3:18 PM Petr Vorel <pvorel@suse.cz> wrote:
> > > > Hi Alex,
> > > > ...
> > > > > > > + * [Description]
> > > > > > > + * Tests gfs2 dlm posix op queue handling in the kernel.
> > > > > > > + * It is recommended to run watch -n 0.1 "dlm_tool plocks $LS"
> > > > > > > + * aside to monitor dlm plock handling.
> > > > > > > + *
> > > > > > > + * [How to use it]
> > > > > > > + * Call it with TMPDIR=/mnt ./fcntl40 where TMPDIR is a gfs2 mountpoint.
> > > > > > I wonder if we could check for GFS2_MAGIC (we'd need to add it to
> > > > > > include/tst_fs.h => 0x01161970) and quit the test with tst_brk(TCONF) if TMPDIR
> > > > > > is not on gfs2.
> > > > > > ATM we don't have any helper in struct tst_test, which would do it.
> > > > > I will mention that gfs2 is only an example here. It becomes
> > > > > interesting when a file system implements its own .lock() callback OR
> > > > > if somebody wants to test file system core, when a filesystem does not
> > > > > implement its own .lock().
> > > > I see .lock is implemented in 9p, afs, ceph, cifs, ocfs2, orangefs, even NFS.
> > > > "file system core": do you mean VFS? Because that would be more usable than the
> > > > filesystems above (which are quite exotic).
> > > It depends here what they are doing in .lock() - If they just call
> > > locks_lock_file_wait() or similar helpers depending on the call, then
> > > they don't do much different than what vfs is doing.
> > > In case of gfs2/ocfs it is very special, it redirects their calls to
> > > DLM and DLM has its own whole posix implementation behind it. We only
> > > call locks_lock_file_wait() to keep the Linux bookkeeping in
> > > /proc/locks.
> > > What I am doing here is mostly trusting the Linux implementation and
> > > comparing results from e.g. tmpfs with GFS2 and I found issues.
> > > For now I trust the Linux implementation and check if we have a
> > > different result here. I need to be very careful here because
> > > GFS2/OCFS2 are cluster filesystems and the fcntl() interface was never
> > > made for cluster filesystems. However I currently only test "one node"
> > > locking and this should deliver the same results as tmpfs, etc.
> > Thanks for info. I'm still not sure if this is useful only for gfs2/ocfs
> > and we should prepare block device with gfs2 or ocfs and test filesystem on it.
> > Or if it would be useful to test other filesystem implementation. In this latter
> > case we usually use .all_filesystems (we don't have proper docs for
> > .all_filesystems, the closest is [1]) and test is then run on various common
> > filesystems (see fs_type_whitelist[] in lib/tst_supported_fs_types.c), but in
> > that case gfs2 would be skipped (it's not a common filesystem).
> > In case preparing block device with gfs2 something like this might work:
> > static struct tst_test test = {
> > ...
> > .dev_fs_type = "gfs2",
> > .format_device = 1,
> > .dev_fs_opts = (const char *const []){ "-t", "mycluster:mygfs2", "-p", "lock_dlm", "-j" , "2" , NULL},
> Can I override this setting by some ENV because I actually want to run
> it on a different filesystem which is using VFS posix lock
> implementation, because as I said I want to compare the results.
Sure, there is LTP_DEV_FS_TYPE. But the point is to write test which will be
useful for the default scenario.
Kind regards,
Petr
> - Alex
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [LTP] [PATCH 1/5] fcntl40: test for owner values on classic posix lock
2023-06-30 19:59 ` Alexander Aring
2023-07-02 19:18 ` Petr Vorel
@ 2023-07-02 19:19 ` Petr Vorel
1 sibling, 0 replies; 16+ messages in thread
From: Petr Vorel @ 2023-07-02 19:19 UTC (permalink / raw)
To: Alexander Aring; +Cc: ltp
Hi Alex,
...
> > > + tst_res(TPASS, "Parent passed!");
> > There is TPASS in child, does it really need to be in the parent as well?
> no.
Thanks for info, let's skip it then in the parent.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 2/5] fcntl41: test for owner values on OFD posix locks
2023-05-30 20:37 [LTP] [PATCH 0/5] fcntl: add more testcases Alexander Aring
2023-05-30 20:37 ` [LTP] [PATCH 1/5] fcntl40: test for owner values on classic posix lock Alexander Aring
@ 2023-05-30 20:37 ` Alexander Aring
2023-06-21 9:38 ` Petr Vorel
2023-05-30 20:37 ` [LTP] [PATCH 3/5] fcntl42: test for F_SETLKW interruption case Alexander Aring
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Alexander Aring @ 2023-05-30 20:37 UTC (permalink / raw)
To: ltp
This patch adds fcntl41 to test similar owner values for OFD locks.
There was an issue been found in the gfs2 filesystem because there
can be collisions with identical owner values.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
testcases/kernel/syscalls/fcntl/.gitignore | 2 +
testcases/kernel/syscalls/fcntl/fcntl41.c | 178 +++++++++++++++++++++
2 files changed, 180 insertions(+)
create mode 100644 testcases/kernel/syscalls/fcntl/fcntl41.c
diff --git a/testcases/kernel/syscalls/fcntl/.gitignore b/testcases/kernel/syscalls/fcntl/.gitignore
index e558cfe83..4bdae1a22 100644
--- a/testcases/kernel/syscalls/fcntl/.gitignore
+++ b/testcases/kernel/syscalls/fcntl/.gitignore
@@ -76,3 +76,5 @@
/fcntl39_64
/fcntl40
/fcntl40_64
+/fcntl41
+/fcntl41_64
diff --git a/testcases/kernel/syscalls/fcntl/fcntl41.c b/testcases/kernel/syscalls/fcntl/fcntl41.c
new file mode 100644
index 000000000..40d14ff02
--- /dev/null
+++ b/testcases/kernel/syscalls/fcntl/fcntl41.c
@@ -0,0 +1,178 @@
+/*
+ * [Description]
+ * Tests gfs2 dlm posix op queue handling in the kernel.
+ * It is recommended to run watch -n 0.1 "dlm_tool plocks $LS"
+ * aside to monitor dlm plock handling.
+ *
+ * [How to use it]
+ * Call it with TMPDIR=/mnt ./fcntl41 where TMPDIR is a gfs2 mountpoint.
+ * Try it on other filesystems to compare results.
+ *
+ * [What's it doing]
+ *
+ * The test shows that we currently have problems with the operation lookup
+ * functionality [0] when we using threads. The owner value is the same for two
+ * locks being in WAITING state. dlm_controld "dlm_tool plocks $LS" will show
+ * it correctly that the specific lock is not in waiting anymore. The issue
+ * begins matching the right iter->done in the kernel at dev_write() see [0].
+ *
+ * What this test does is (using dlm_controld interval range interpretation):
+ *
+ * parent:
+ *
+ * 1. lock[0-1]
+ *
+ * child:
+ *
+ * thread1:
+ *
+ * 2. lockw[1-1] - important 1-1 at first because the order of WAITING state
+ * locks matters
+ *
+ * thread2:
+ *
+ * 3. lockw[0-0]
+ *
+ * parent:
+ *
+ * 4. unlock[1-1] - will give a iter->done = 1 in [0] for lock at 3. and the
+ * application results in a deadlock
+ * 5. unlock[0-0]
+ *
+ * We have this issue also with SETLK, GETLK - it's easier to reproduce
+ * with SETLKW because dev_write() is more controlable by doing unlocks.
+ *
+ * OFD (open filedescriptor locks) are also affected and should be able
+ * to reproduce with fork() only and not threads. The owner value of [0]
+ * depends on "struct file *" pointer in this case.
+ *
+ * [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/dlm/plock.c?h=v6.3#n432
+ */
+
+#include <sys/wait.h>
+
+#include "tst_test.h"
+
+static int fd, fd2;
+
+void do_child1(void)
+{
+ const struct flock fl_0_0 = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 0L,
+ .l_len = 1L,
+ };
+
+ tst_res(TINFO, "thread1 waits for thread2 to lock 1-1");
+ TST_CHECKPOINT_WAIT(1);
+
+ tst_res(TINFO, "thread1 lock region 0-0 - It should block");
+ SAFE_FCNTL(fd2, F_OFD_SETLKW, &fl_0_0);
+ tst_res(TINFO, "lock region 0-0 acquired");
+
+ tst_res(TPASS, "Child1 passed!");
+}
+
+void do_child2(void)
+{
+ const struct flock fl_1_1 = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 1L,
+ .l_len = 1L,
+ };
+
+ tst_res(TINFO, "thread1 lock region 1-1 - It should block");
+ SAFE_FCNTL(fd2, F_OFD_SETLKW, &fl_1_1);
+ tst_res(TINFO, "lock region 1-1 acquired");
+
+ TST_CHECKPOINT_WAKE(2);
+ tst_res(TPASS, "Child2 passed!");
+}
+
+void do_parent(void)
+{
+ struct flock fl = {
+ .l_whence = SEEK_SET,
+ };
+
+ /* wait for 1 seconds so thread2 lock 1-1 tries to acquires at first
+ * than thread1 lock 0-0 tries to acquired to have a specific waiting
+ * order in dlm posix handling.
+ */
+ sleep(1);
+ /* tell thread2 to call SETLKW for lock 0-0 */
+ TST_CHECKPOINT_WAKE(1);
+ /* wait 3 seconds for thread 1 and 2 being in waiting state */
+ sleep(3);
+
+ /* unlock 0-1, should be successful */
+ fl.l_type = F_UNLCK;
+ fl.l_start = 1;
+ fl.l_len = 1;
+ tst_res(TINFO, "unlock region 1-1 thread2");
+ SAFE_FCNTL(fd, F_OFD_SETLK, &fl);
+
+ /* wait until thread 2 got acquired and leave waiting */
+ TST_CHECKPOINT_WAIT(2);
+
+ fl.l_start = 0;
+ fl.l_len = 1;
+ fl.l_type = F_UNLCK;
+ tst_res(TINFO, "unlock region 0-0 thread2");
+ SAFE_FCNTL(fd, F_OFD_SETLK, &fl);
+}
+
+static void fcntl40_test(void)
+{
+ struct flock fl = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 0L,
+ .l_len = 2L,
+ };
+ pid_t pid;
+
+ tst_res(TINFO, "parent lock region 0-1 - should be successful");
+ SAFE_FCNTL(fd, F_OFD_SETLK, &fl);
+ tst_res(TINFO, "parent region 0-1 locked");
+
+ pid = SAFE_FORK();
+ if (pid == 0) {
+ do_child1();
+ return;
+ }
+
+ pid = SAFE_FORK();
+ if (pid == 0) {
+ do_child2();
+ return;
+ }
+
+ do_parent();
+ wait(NULL);
+ wait(NULL);
+
+ tst_res(TPASS, "Parent passed!");
+}
+
+static void setup(void)
+{
+ fd = SAFE_OPEN("filename", O_RDWR | O_CREAT, 0700);
+ fd2 = SAFE_OPEN("filename", O_RDWR | O_CREAT, 0700);
+}
+
+static void cleanup(void)
+{
+ if (fd > -1)
+ SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+ .forks_child = 1,
+ .needs_checkpoints = 1,
+ .test_all = fcntl40_test,
+ .setup = setup,
+ .cleanup = cleanup,
+};
--
2.31.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [LTP] [PATCH 2/5] fcntl41: test for owner values on OFD posix locks
2023-05-30 20:37 ` [LTP] [PATCH 2/5] fcntl41: test for owner values on OFD posix locks Alexander Aring
@ 2023-06-21 9:38 ` Petr Vorel
2023-06-30 20:00 ` Alexander Aring
0 siblings, 1 reply; 16+ messages in thread
From: Petr Vorel @ 2023-06-21 9:38 UTC (permalink / raw)
To: Alexander Aring; +Cc: ltp
Hi Alexander,
> diff --git a/testcases/kernel/syscalls/fcntl/fcntl41.c b/testcases/kernel/syscalls/fcntl/fcntl41.c
> new file mode 100644
> index 000000000..40d14ff02
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fcntl/fcntl41.c
...
> +#include <sys/wait.h>
> +
> +#include "tst_test.h"
> +
> +static int fd, fd2;
> +
> +void do_child1(void)
> +{
> + const struct flock fl_0_0 = {
> + .l_type = F_WRLCK,
> + .l_whence = SEEK_SET,
> + .l_start = 0L,
> + .l_len = 1L,
> + };
> +
> + tst_res(TINFO, "thread1 waits for thread2 to lock 1-1");
> + TST_CHECKPOINT_WAIT(1);
> +
> + tst_res(TINFO, "thread1 lock region 0-0 - It should block");
> + SAFE_FCNTL(fd2, F_OFD_SETLKW, &fl_0_0);
F_OFD_SETLKW is undefined on old Cent0S 7, we still support:
https://github.com/pevik/ltp/actions/runs/5331934790/jobs/9660442246
You need to use our fallback to avoid this.
#include "lapi/fcntl.h"
We also have fcntl_common.h, which is used for F_OFD_* (fcntl_compat(),
you may need to use it. It also includes lapi/fcntl.h.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [LTP] [PATCH 2/5] fcntl41: test for owner values on OFD posix locks
2023-06-21 9:38 ` Petr Vorel
@ 2023-06-30 20:00 ` Alexander Aring
0 siblings, 0 replies; 16+ messages in thread
From: Alexander Aring @ 2023-06-30 20:00 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
Hi,
On Wed, Jun 21, 2023 at 5:38 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Alexander,
>
> > diff --git a/testcases/kernel/syscalls/fcntl/fcntl41.c b/testcases/kernel/syscalls/fcntl/fcntl41.c
> > new file mode 100644
> > index 000000000..40d14ff02
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/fcntl/fcntl41.c
> ...
> > +#include <sys/wait.h>
> > +
> > +#include "tst_test.h"
> > +
> > +static int fd, fd2;
> > +
> > +void do_child1(void)
> > +{
> > + const struct flock fl_0_0 = {
> > + .l_type = F_WRLCK,
> > + .l_whence = SEEK_SET,
> > + .l_start = 0L,
> > + .l_len = 1L,
> > + };
> > +
> > + tst_res(TINFO, "thread1 waits for thread2 to lock 1-1");
> > + TST_CHECKPOINT_WAIT(1);
> > +
> > + tst_res(TINFO, "thread1 lock region 0-0 - It should block");
> > + SAFE_FCNTL(fd2, F_OFD_SETLKW, &fl_0_0);
> F_OFD_SETLKW is undefined on old Cent0S 7, we still support:
> https://github.com/pevik/ltp/actions/runs/5331934790/jobs/9660442246
>
> You need to use our fallback to avoid this.
>
> #include "lapi/fcntl.h"
>
> We also have fcntl_common.h, which is used for F_OFD_* (fcntl_compat(),
> you may need to use it. It also includes lapi/fcntl.h.
>
ok.
- Alex
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 16+ messages in thread
* [LTP] [PATCH 3/5] fcntl42: test for F_SETLKW interruption case
2023-05-30 20:37 [LTP] [PATCH 0/5] fcntl: add more testcases Alexander Aring
2023-05-30 20:37 ` [LTP] [PATCH 1/5] fcntl40: test for owner values on classic posix lock Alexander Aring
2023-05-30 20:37 ` [LTP] [PATCH 2/5] fcntl41: test for owner values on OFD posix locks Alexander Aring
@ 2023-05-30 20:37 ` Alexander Aring
2023-05-30 20:37 ` [LTP] [PATCH 4/5] fcntl43: test for identical F_SETLKW lock requests Alexander Aring
2023-05-30 20:37 ` [LTP] [PATCH 5/5] fcntl44: test for kill child while others waiting Alexander Aring
4 siblings, 0 replies; 16+ messages in thread
From: Alexander Aring @ 2023-05-30 20:37 UTC (permalink / raw)
To: ltp
This patch adds fcntl42 testcase to test on side effects, e.g. unlock
all acquired locks, when a lock request was interrupted by a signal.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
testcases/kernel/syscalls/fcntl/.gitignore | 2 +
testcases/kernel/syscalls/fcntl/fcntl42.c | 153 +++++++++++++++++++++
2 files changed, 155 insertions(+)
create mode 100644 testcases/kernel/syscalls/fcntl/fcntl42.c
diff --git a/testcases/kernel/syscalls/fcntl/.gitignore b/testcases/kernel/syscalls/fcntl/.gitignore
index 4bdae1a22..abffa2967 100644
--- a/testcases/kernel/syscalls/fcntl/.gitignore
+++ b/testcases/kernel/syscalls/fcntl/.gitignore
@@ -78,3 +78,5 @@
/fcntl40_64
/fcntl41
/fcntl41_64
+/fcntl42
+/fcntl42_64
diff --git a/testcases/kernel/syscalls/fcntl/fcntl42.c b/testcases/kernel/syscalls/fcntl/fcntl42.c
new file mode 100644
index 000000000..4d66568fd
--- /dev/null
+++ b/testcases/kernel/syscalls/fcntl/fcntl42.c
@@ -0,0 +1,153 @@
+/*
+ * [Description]
+ * This test confirms that DLM posix locking has problems when a posix lock
+ * got interrupted every lock gets unlocked.
+ *
+ * man fcntl:
+ *
+ * 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)).
+ *
+ * The above quote of the man page of fcntl() is what this testcase tests.
+ * particulary if it has side-effects of previously acquired locks.
+ *
+ * [How to use it]
+ * Call it with TMPDIR=/mnt ./fcntl42 where TMPDIR is a gfs2 mountpoint.
+ * Try it on other filesystems to compare results.
+ *
+ * [What's it doing]
+ *
+ * What this test does is (using dlm_controld interval range interpretation):
+ *
+ * parent:
+ *
+ * 1. lock[0-0]
+ *
+ * child:
+ *
+ * 2. lock[1-1]
+ * 3. lockw[0-0] - should block (see 1. parent), but we get interrupted by SIGALRM
+ *
+ * parent:
+ *
+ * 4. trylock[1-1] - should return -1 and errno -EAGAIN because the child
+ * should still have lock[1-1] acuired and this is what
+ * the child thinks to have. If it's successful the child
+ * wrongly assumes it has the lock[1-1] still acquired and
+ * the child process is still alive.
+ *
+ */
+
+#include <sys/wait.h>
+
+#include "tst_test.h"
+
+static int fd;
+
+static void catch_alarm(int num)
+{
+ (void)num;
+
+ tst_res(TINFO, "catch alarm");
+}
+
+void do_child1(void)
+{
+ struct flock fl = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 1L,
+ .l_len = 1L,
+ };
+ struct sigaction act;
+ int rv;
+
+ SAFE_FCNTL(fd, F_SETLK, &fl);
+ tst_res(TINFO, "child locked lock 1-1");
+
+ fl.l_type = F_WRLCK;
+ fl.l_start = 0;
+ fl.l_len = 1;
+
+ memset(&act, 0, sizeof(act));
+ act.sa_handler = catch_alarm;
+ sigemptyset(&act.sa_mask);
+ sigaddset(&act.sa_mask, SIGALRM);
+ sigaction(SIGALRM, &act, NULL);
+
+ /* interrupt SETLKW by signal in 3 secs */
+ alarm(3);
+ rv = fcntl(fd, F_SETLKW, &fl);
+ if (rv == -1 && errno == EINTR)
+ tst_res(TPASS, "Child1 interrupted 0-0");
+
+ TST_CHECKPOINT_WAKE(1);
+ /* keep child alive until parent check region */
+ TST_CHECKPOINT_WAIT(2);
+}
+
+static void fcntl40_test(void)
+{
+ struct flock fl = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 0L,
+ .l_len = 1L,
+ };
+ pid_t pid;
+ int rv;
+
+ SAFE_FCNTL(fd, F_SETLK, &fl);
+ tst_res(TINFO, "parent lock 0-0");
+
+ pid = SAFE_FORK();
+ if (pid == 0) {
+ do_child1();
+ tst_res(TINFO, "childs exits");
+ return;
+ }
+
+ TST_CHECKPOINT_WAIT(1);
+
+ fl.l_type = F_WRLCK;
+ fl.l_start = 1;
+ fl.l_len = 1;
+ rv = fcntl(fd, F_SETLK, &fl);
+ /* parent testing childs region, the child will think
+ * it has region 1-1 locked because it was interrupted
+ * by region 0-0. Due bugs the interruption also unlocked
+ * region 1-1.
+ */
+ if (rv == -1 && errno == EAGAIN)
+ tst_res(TPASS, "region 1-1 locked");
+ else
+ tst_res(TFAIL, "region 1-1 unlocked");
+
+ TST_CHECKPOINT_WAKE(2);
+
+ wait(NULL);
+ tst_res(TINFO, "parent exits");
+}
+
+static void setup(void)
+{
+ fd = SAFE_OPEN("filename", O_RDWR | O_CREAT, 0700);
+}
+
+static void cleanup(void)
+{
+ if (fd > -1)
+ SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+ .forks_child = 1,
+ .needs_checkpoints = 1,
+ .test_all = fcntl40_test,
+ .setup = setup,
+ .cleanup = cleanup,
+};
--
2.31.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 16+ messages in thread* [LTP] [PATCH 4/5] fcntl43: test for identical F_SETLKW lock requests
2023-05-30 20:37 [LTP] [PATCH 0/5] fcntl: add more testcases Alexander Aring
` (2 preceding siblings ...)
2023-05-30 20:37 ` [LTP] [PATCH 3/5] fcntl42: test for F_SETLKW interruption case Alexander Aring
@ 2023-05-30 20:37 ` Alexander Aring
2023-05-30 20:37 ` [LTP] [PATCH 5/5] fcntl44: test for kill child while others waiting Alexander Aring
4 siblings, 0 replies; 16+ messages in thread
From: Alexander Aring @ 2023-05-30 20:37 UTC (permalink / raw)
To: ltp
This patch adds fcntl43 to test two identical lock requests at the same
time. They are identical as they will be granted at the same time.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
testcases/kernel/syscalls/fcntl/.gitignore | 2 +
testcases/kernel/syscalls/fcntl/Makefile | 3 +
testcases/kernel/syscalls/fcntl/fcntl43.c | 140 +++++++++++++++++++++
3 files changed, 145 insertions(+)
create mode 100644 testcases/kernel/syscalls/fcntl/fcntl43.c
diff --git a/testcases/kernel/syscalls/fcntl/.gitignore b/testcases/kernel/syscalls/fcntl/.gitignore
index abffa2967..6622aedbc 100644
--- a/testcases/kernel/syscalls/fcntl/.gitignore
+++ b/testcases/kernel/syscalls/fcntl/.gitignore
@@ -80,3 +80,5 @@
/fcntl41_64
/fcntl42
/fcntl42_64
+/fcntl43
+/fcntl43_64
diff --git a/testcases/kernel/syscalls/fcntl/Makefile b/testcases/kernel/syscalls/fcntl/Makefile
index c3196a527..7478b87ec 100644
--- a/testcases/kernel/syscalls/fcntl/Makefile
+++ b/testcases/kernel/syscalls/fcntl/Makefile
@@ -15,6 +15,9 @@ fcntl36_64: LDLIBS += -lpthread
fcntl40: LDLIBS += -lpthread
fcntl40_64: LDLIBS += -lpthread
+fcntl43: LDLIBS += -lpthread
+fcntl43_64: LDLIBS += -lpthread
+
include $(top_srcdir)/include/mk/testcases.mk
include $(abs_srcdir)/../utils/newer_64.mk
diff --git a/testcases/kernel/syscalls/fcntl/fcntl43.c b/testcases/kernel/syscalls/fcntl/fcntl43.c
new file mode 100644
index 000000000..0a69396a4
--- /dev/null
+++ b/testcases/kernel/syscalls/fcntl/fcntl43.c
@@ -0,0 +1,140 @@
+/*
+ * [Description]
+ * Tests gfs2 dlm posix op queue handling in the kernel.
+ * It is recommended to run watch -n 0.1 "dlm_tool plocks $LS"
+ * aside to monitor dlm plock handling.
+ *
+ * This testcase creates two lock requests which looks identical and should
+ * be granted at the same time.
+ *
+ * [How to use it]
+ * Call it with TMPDIR=/mnt ./fcntl43 where TMPDIR is a gfs2 mountpoint.
+ * Try it on other filesystems to compare results.
+ */
+
+#include <sys/wait.h>
+#include <pthread.h>
+
+#include "tst_safe_pthread.h"
+#include "tst_test.h"
+
+static int fd;
+
+static void *do_thread1(void *arg)
+{
+ const struct flock fl = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 0L,
+ .l_len = 1L,
+ };
+ (void)arg;
+
+ tst_res(TINFO, "thread1 waits for thread2 to lock 0-0");
+ TST_CHECKPOINT_WAIT(1);
+
+ tst_res(TINFO, "thread1 lock region 0-0 - It should block");
+ SAFE_FCNTL(fd, F_SETLKW, &fl);
+ tst_res(TINFO, "lock region 0-0 acquired");
+
+ return NULL;
+}
+
+static void *do_thread2(void *arg)
+{
+ const struct flock fl = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 0L,
+ .l_len = 1L,
+ };
+ (void)arg;
+
+ tst_res(TINFO, "thread1 lock region 0-0 - It should block");
+ SAFE_FCNTL(fd, F_SETLKW, &fl);
+ tst_res(TINFO, "lock region 0-0 acquired");
+
+ return NULL;
+}
+
+void do_child(void)
+{
+ pthread_t t1, t2;
+
+ SAFE_PTHREAD_CREATE(&t1, NULL, do_thread1, NULL);
+ SAFE_PTHREAD_CREATE(&t2, NULL, do_thread2, NULL);
+
+ SAFE_PTHREAD_JOIN(t1, NULL);
+ SAFE_PTHREAD_JOIN(t2, NULL);
+
+ tst_res(TPASS, "Child passed!");
+}
+
+void do_parent(void)
+{
+ struct flock fl = {
+ .l_whence = SEEK_SET,
+ };
+
+ /* wait for 1 seconds so thread2 lock 0-0 tries to acquires at first
+ * than thread1 lock 0-0 tries to acquired to have a specific waiting
+ * order in dlm posix handling.
+ */
+ sleep(1);
+ /* tell thread2 to call SETLKW for lock 0-0 */
+ TST_CHECKPOINT_WAKE(1);
+ /* wait 3 seconds for thread 1 and 2 being in waiting state */
+ sleep(3);
+
+ /* unlock 0-0, should be successful */
+ fl.l_type = F_UNLCK;
+ fl.l_start = 0;
+ fl.l_len = 1;
+ tst_res(TINFO, "unlock region 0-0 thread2");
+ SAFE_FCNTL(fd, F_SETLK, &fl);
+}
+
+static void fcntl40_test(void)
+{
+ struct flock fl = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 0L,
+ .l_len = 2L,
+ };
+ pid_t pid;
+
+ tst_res(TINFO, "parent lock region 0-1 - should be successful");
+ SAFE_FCNTL(fd, F_SETLK, &fl);
+ tst_res(TINFO, "parent region 0-1 locked");
+
+ pid = SAFE_FORK();
+ if (pid == 0) {
+ do_child();
+ return;
+ }
+
+ do_parent();
+ wait(NULL);
+
+ tst_res(TPASS, "Parent passed!");
+}
+
+static void setup(void)
+{
+ fd = SAFE_OPEN("filename", O_RDWR | O_CREAT, 0700);
+}
+
+static void cleanup(void)
+{
+ if (fd > -1)
+ SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+ .forks_child = 1,
+ .needs_checkpoints = 1,
+ .test_all = fcntl40_test,
+ .setup = setup,
+ .cleanup = cleanup,
+};
--
2.31.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 16+ messages in thread* [LTP] [PATCH 5/5] fcntl44: test for kill child while others waiting
2023-05-30 20:37 [LTP] [PATCH 0/5] fcntl: add more testcases Alexander Aring
` (3 preceding siblings ...)
2023-05-30 20:37 ` [LTP] [PATCH 4/5] fcntl43: test for identical F_SETLKW lock requests Alexander Aring
@ 2023-05-30 20:37 ` Alexander Aring
4 siblings, 0 replies; 16+ messages in thread
From: Alexander Aring @ 2023-05-30 20:37 UTC (permalink / raw)
To: ltp
This patch adds fcntl44 to kill a forked child blocking for get a lock
granted and another child waiting for the same lock context. This test
checks on cleanup issues when the forked child gets killed.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
testcases/kernel/syscalls/fcntl/.gitignore | 2 +
testcases/kernel/syscalls/fcntl/fcntl44.c | 128 +++++++++++++++++++++
2 files changed, 130 insertions(+)
create mode 100644 testcases/kernel/syscalls/fcntl/fcntl44.c
diff --git a/testcases/kernel/syscalls/fcntl/.gitignore b/testcases/kernel/syscalls/fcntl/.gitignore
index 6622aedbc..0ee00ce1f 100644
--- a/testcases/kernel/syscalls/fcntl/.gitignore
+++ b/testcases/kernel/syscalls/fcntl/.gitignore
@@ -82,3 +82,5 @@
/fcntl42_64
/fcntl43
/fcntl43_64
+/fcntl44_64
+/fcntl44
diff --git a/testcases/kernel/syscalls/fcntl/fcntl44.c b/testcases/kernel/syscalls/fcntl/fcntl44.c
new file mode 100644
index 000000000..364916634
--- /dev/null
+++ b/testcases/kernel/syscalls/fcntl/fcntl44.c
@@ -0,0 +1,128 @@
+/*
+ * [Description]
+ * Tests killing a child while several other processes using different OFD
+ * lock contexts are blocking. When the child is killed there is a cleanup
+ * routine going on. The parent will check if there were any unexpected
+ * side-effects, e.g. unlock all previous acquired locks, happened.
+ *
+ * It is recommended to run watch -n 0.1 "dlm_tool plocks $LS"
+ * aside to monitor dlm plock handling.
+ *
+ * [How to use it]
+ * Call it with TMPDIR=/mnt ./fcntl44 where TMPDIR is a gfs2 mountpoint.
+ * Try it on other filesystems to compare results.
+ *
+ * [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/dlm/plock.c?h=v6.3#n432
+ */
+
+#include <sys/wait.h>
+
+#include "tst_test.h"
+
+static int fd, fd2;
+
+void do_child1(void)
+{
+ const struct flock fl = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 0L,
+ .l_len = 1L,
+ };
+
+ SAFE_FCNTL(fd2, F_OFD_SETLKW, &fl);
+ tst_res(TINFO, "child1 lock region 0-0 acquired");
+
+ TST_CHECKPOINT_WAIT(1);
+
+ tst_res(TPASS, "Child1 passed!");
+}
+
+void do_child2(void)
+{
+ const struct flock fl = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 1L,
+ .l_len = 1L,
+ };
+
+ SAFE_FCNTL(fd2, F_OFD_SETLKW, &fl);
+}
+
+static void fcntl40_test(void)
+{
+ struct flock fl = {
+ .l_type = F_WRLCK,
+ .l_whence = SEEK_SET,
+ .l_start = 0L,
+ .l_len = 2L,
+ };
+ pid_t pid, pid2;
+ int rv;
+
+ SAFE_FCNTL(fd, F_SETLK, &fl);
+ tst_res(TINFO, "parent lock 0-1");
+
+ pid = SAFE_FORK();
+ if (pid == 0) {
+ do_child1();
+ return;
+ }
+
+ pid2 = SAFE_FORK();
+ if (pid2 == 0) {
+ do_child2();
+ return;
+ }
+
+ /* wait until childs wait to acquire */
+ sleep(2);
+
+ kill(pid2, SIGKILL);
+ /* wait until linux killed the process */
+ sleep(3);
+ tst_res(TPASS, "Child2 killed!");
+
+ fl.l_type = F_UNLCK;
+ SAFE_FCNTL(fd, F_SETLK, &fl);
+
+ /* let child1 acquire 0-0 */
+ sleep(2);
+
+ fl.l_type = F_WRLCK;
+ fl.l_start = 0;
+ fl.l_len = 1;
+ rv = fcntl(fd, F_OFD_SETLK, &fl);
+ if (rv == -1 && errno == EAGAIN)
+ tst_res(TPASS, "region 1-1 was locked");
+ else
+ tst_res(TFAIL, "region 1-1 was unlocked");
+
+ TST_CHECKPOINT_WAKE(1);
+
+ /* due bug child1 does not return because child2 killing removed waiters */
+ wait(NULL);
+
+ tst_res(TPASS, "Parent passed!");
+}
+
+static void setup(void)
+{
+ fd = SAFE_OPEN("filename", O_RDWR | O_CREAT, 0700);
+ fd2 = SAFE_OPEN("filename", O_RDWR | O_CREAT, 0700);
+}
+
+static void cleanup(void)
+{
+ if (fd > -1)
+ SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+ .forks_child = 1,
+ .needs_checkpoints = 1,
+ .test_all = fcntl40_test,
+ .setup = setup,
+ .cleanup = cleanup,
+};
--
2.31.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 16+ messages in thread