Test the basic locking stuff on 2 fds: multiple read locks,
conflicts between read and write locks, use of len==0 for queries.
Also tests for F_UNLCK F_OFD_GETLK extension.
Signed-off-by: Stas Sergeev <[email protected]>
CC: Jeff Layton <[email protected]>
CC: Chuck Lever <[email protected]>
CC: Alexander Viro <[email protected]>
CC: Christian Brauner <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Shuah Khan <[email protected]>
CC: [email protected]
CC: [email protected]
---
tools/testing/selftests/locking/Makefile | 2 +
tools/testing/selftests/locking/ofdlocks.c | 132 +++++++++++++++++++++
2 files changed, 134 insertions(+)
create mode 100644 tools/testing/selftests/locking/ofdlocks.c
diff --git a/tools/testing/selftests/locking/Makefile b/tools/testing/selftests/locking/Makefile
index 6e7761ab3536..a83ced1626de 100644
--- a/tools/testing/selftests/locking/Makefile
+++ b/tools/testing/selftests/locking/Makefile
@@ -7,4 +7,6 @@ all:
TEST_PROGS := ww_mutex.sh
+TEST_GEN_PROGS := ofdlocks
+
include ../lib.mk
diff --git a/tools/testing/selftests/locking/ofdlocks.c b/tools/testing/selftests/locking/ofdlocks.c
new file mode 100644
index 000000000000..1ccb2b9b5ead
--- /dev/null
+++ b/tools/testing/selftests/locking/ofdlocks.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define _GNU_SOURCE
+#include <fcntl.h>
+#include <assert.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include "../kselftest.h"
+
+static int lock_set(int fd, struct flock *fl)
+{
+ int ret;
+
+ fl->l_pid = 0; // needed for OFD locks
+ fl->l_whence = SEEK_SET;
+ ret = fcntl(fd, F_OFD_SETLK, fl);
+ if (ret)
+ perror("fcntl()");
+ return ret;
+}
+
+static int lock_get(int fd, struct flock *fl)
+{
+ int ret;
+
+ fl->l_pid = 0; // needed for OFD locks
+ fl->l_whence = SEEK_SET;
+ ret = fcntl(fd, F_OFD_GETLK, fl);
+ if (ret)
+ perror("fcntl()");
+ return ret;
+}
+
+int main(void)
+{
+ int rc;
+ struct flock fl, fl2;
+ int fd = open("/tmp/aa", O_RDWR | O_CREAT | O_EXCL, 0600);
+ int fd2 = open("/tmp/aa", O_RDONLY);
+
+ unlink("aa");
+ assert(fd != -1);
+ assert(fd2 != -1);
+ ksft_print_msg("[INFO] opened fds %i %i\n", fd, fd2);
+
+ /* Set some read lock */
+ fl.l_type = F_RDLCK;
+ fl.l_start = 5;
+ fl.l_len = 3;
+ rc = lock_set(fd, &fl);
+ if (rc == 0) {
+ ksft_print_msg
+ ("[SUCCESS] set OFD read lock on first fd\n");
+ } else {
+ ksft_print_msg("[FAIL] to set OFD read lock on first fd\n");
+ return -1;
+ }
+ /* Make sure read locks do not conflict on different fds. */
+ fl.l_type = F_RDLCK;
+ fl.l_start = 5;
+ fl.l_len = 1;
+ rc = lock_get(fd2, &fl);
+ if (rc != 0)
+ return -1;
+ if (fl.l_type != F_UNLCK) {
+ ksft_print_msg("[FAIL] read locks conflicted\n");
+ return -1;
+ }
+ /* Make sure read/write locks do conflict on different fds. */
+ fl.l_type = F_WRLCK;
+ fl.l_start = 5;
+ fl.l_len = 1;
+ rc = lock_get(fd2, &fl);
+ if (rc != 0)
+ return -1;
+ if (fl.l_type != F_UNLCK) {
+ ksft_print_msg
+ ("[SUCCESS] read and write locks conflicted\n");
+ } else {
+ ksft_print_msg
+ ("[SUCCESS] read and write locks not conflicted\n");
+ return -1;
+ }
+ /* Get info about the lock on first fd. */
+ fl.l_type = F_UNLCK;
+ fl.l_start = 5;
+ fl.l_len = 1;
+ rc = lock_get(fd, &fl);
+ if (rc != 0) {
+ ksft_print_msg
+ ("[FAIL] F_OFD_GETLK with F_UNLCK not supported\n");
+ return -1;
+ }
+ if (fl.l_type != F_UNLCK) {
+ ksft_print_msg
+ ("[SUCCESS] F_UNLCK test returns: locked, type %i pid %i len %zi\n",
+ fl.l_type, fl.l_pid, fl.l_len);
+ } else {
+ ksft_print_msg
+ ("[FAIL] F_OFD_GETLK with F_UNLCK did not return lock info\n");
+ return -1;
+ }
+ /* Try the same but by locking everything by len==0. */
+ fl2.l_type = F_UNLCK;
+ fl2.l_start = 0;
+ fl2.l_len = 0;
+ rc = lock_get(fd, &fl2);
+ if (rc != 0) {
+ ksft_print_msg
+ ("[FAIL] F_OFD_GETLK with F_UNLCK not supported\n");
+ return -1;
+ }
+ if (memcmp(&fl, &fl2, sizeof(fl))) {
+ ksft_print_msg
+ ("[FAIL] F_UNLCK test returns: locked, type %i pid %i len %zi\n",
+ fl.l_type, fl.l_pid, fl.l_len);
+ return -1;
+ }
+ ksft_print_msg("[SUCCESS] F_UNLCK with len==0 returned the same\n");
+ /* Get info about the lock on second fd - no locks on it. */
+ fl.l_type = F_UNLCK;
+ fl.l_start = 0;
+ fl.l_len = 0;
+ lock_get(fd2, &fl);
+ if (fl.l_type != F_UNLCK) {
+ ksft_print_msg
+ ("[FAIL] F_OFD_GETLK with F_UNLCK return lock info from another fd\n");
+ return -1;
+ }
+ return 0;
+}
--
2.39.2
On Wed, 2023-06-21 at 20:22 +0500, Stas Sergeev wrote:
> Test the basic locking stuff on 2 fds: multiple read locks,
> conflicts between read and write locks, use of len==0 for queries.
> Also tests for F_UNLCK F_OFD_GETLK extension.
>
> Signed-off-by: Stas Sergeev <[email protected]>
>
> CC: Jeff Layton <[email protected]>
> CC: Chuck Lever <[email protected]>
> CC: Alexander Viro <[email protected]>
> CC: Christian Brauner <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: Shuah Khan <[email protected]>
> CC: [email protected]
> CC: [email protected]
>
> ---
> tools/testing/selftests/locking/Makefile | 2 +
> tools/testing/selftests/locking/ofdlocks.c | 132 +++++++++++++++++++++
> 2 files changed, 134 insertions(+)
> create mode 100644 tools/testing/selftests/locking/ofdlocks.c
>
> diff --git a/tools/testing/selftests/locking/Makefile b/tools/testing/selftests/locking/Makefile
> index 6e7761ab3536..a83ced1626de 100644
> --- a/tools/testing/selftests/locking/Makefile
> +++ b/tools/testing/selftests/locking/Makefile
> @@ -7,4 +7,6 @@ all:
>
> TEST_PROGS := ww_mutex.sh
>
> +TEST_GEN_PROGS := ofdlocks
> +
> include ../lib.mk
I'm not sure this really belongs in the "locking" directory. Given that
there is only the ww_mutex test in there, that's more for internal
synchronization mechanisms, I think.
Can you create a new "filelock" directory and drop this into there
instead?
> diff --git a/tools/testing/selftests/locking/ofdlocks.c b/tools/testing/selftests/locking/ofdlocks.c
> new file mode 100644
> index 000000000000..1ccb2b9b5ead
> --- /dev/null
> +++ b/tools/testing/selftests/locking/ofdlocks.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define _GNU_SOURCE
> +#include <fcntl.h>
> +#include <assert.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include "../kselftest.h"
> +
> +static int lock_set(int fd, struct flock *fl)
> +{
> + int ret;
> +
> + fl->l_pid = 0; // needed for OFD locks
> + fl->l_whence = SEEK_SET;
> + ret = fcntl(fd, F_OFD_SETLK, fl);
> + if (ret)
> + perror("fcntl()");
> + return ret;
> +}
> +
> +static int lock_get(int fd, struct flock *fl)
> +{
> + int ret;
> +
> + fl->l_pid = 0; // needed for OFD locks
> + fl->l_whence = SEEK_SET;
> + ret = fcntl(fd, F_OFD_GETLK, fl);
> + if (ret)
> + perror("fcntl()");
> + return ret;
> +}
> +
> +int main(void)
> +{
> + int rc;
> + struct flock fl, fl2;
> + int fd = open("/tmp/aa", O_RDWR | O_CREAT | O_EXCL, 0600);
> + int fd2 = open("/tmp/aa", O_RDONLY);
> +
> + unlink("aa");
> + assert(fd != -1);
> + assert(fd2 != -1);
> + ksft_print_msg("[INFO] opened fds %i %i\n", fd, fd2);
> +
> + /* Set some read lock */
> + fl.l_type = F_RDLCK;
> + fl.l_start = 5;
> + fl.l_len = 3;
> + rc = lock_set(fd, &fl);
> + if (rc == 0) {
> + ksft_print_msg
> + ("[SUCCESS] set OFD read lock on first fd\n");
> + } else {
> + ksft_print_msg("[FAIL] to set OFD read lock on first fd\n");
> + return -1;
> + }
> + /* Make sure read locks do not conflict on different fds. */
> + fl.l_type = F_RDLCK;
> + fl.l_start = 5;
> + fl.l_len = 1;
> + rc = lock_get(fd2, &fl);
> + if (rc != 0)
> + return -1;
> + if (fl.l_type != F_UNLCK) {
> + ksft_print_msg("[FAIL] read locks conflicted\n");
> + return -1;
> + }
> + /* Make sure read/write locks do conflict on different fds. */
> + fl.l_type = F_WRLCK;
> + fl.l_start = 5;
> + fl.l_len = 1;
> + rc = lock_get(fd2, &fl);
> + if (rc != 0)
> + return -1;
> + if (fl.l_type != F_UNLCK) {
> + ksft_print_msg
> + ("[SUCCESS] read and write locks conflicted\n");
> + } else {
> + ksft_print_msg
> + ("[SUCCESS] read and write locks not conflicted\n");
> + return -1;
> + }
> + /* Get info about the lock on first fd. */
> + fl.l_type = F_UNLCK;
> + fl.l_start = 5;
> + fl.l_len = 1;
> + rc = lock_get(fd, &fl);
> + if (rc != 0) {
> + ksft_print_msg
> + ("[FAIL] F_OFD_GETLK with F_UNLCK not supported\n");
> + return -1;
> + }
> + if (fl.l_type != F_UNLCK) {
> + ksft_print_msg
> + ("[SUCCESS] F_UNLCK test returns: locked, type %i pid %i len %zi\n",
> + fl.l_type, fl.l_pid, fl.l_len);
> + } else {
> + ksft_print_msg
> + ("[FAIL] F_OFD_GETLK with F_UNLCK did not return lock info\n");
> + return -1;
> + }
> + /* Try the same but by locking everything by len==0. */
> + fl2.l_type = F_UNLCK;
> + fl2.l_start = 0;
> + fl2.l_len = 0;
> + rc = lock_get(fd, &fl2);
> + if (rc != 0) {
> + ksft_print_msg
> + ("[FAIL] F_OFD_GETLK with F_UNLCK not supported\n");
> + return -1;
> + }
> + if (memcmp(&fl, &fl2, sizeof(fl))) {
> + ksft_print_msg
> + ("[FAIL] F_UNLCK test returns: locked, type %i pid %i len %zi\n",
> + fl.l_type, fl.l_pid, fl.l_len);
> + return -1;
> + }
> + ksft_print_msg("[SUCCESS] F_UNLCK with len==0 returned the same\n");
> + /* Get info about the lock on second fd - no locks on it. */
> + fl.l_type = F_UNLCK;
> + fl.l_start = 0;
> + fl.l_len = 0;
> + lock_get(fd2, &fl);
> + if (fl.l_type != F_UNLCK) {
> + ksft_print_msg
> + ("[FAIL] F_OFD_GETLK with F_UNLCK return lock info from another fd\n");
> + return -1;
> + }
> + return 0;
> +}
I'm not opposed to adding a selftest here, but most filesystem testing
is done via xfstests:
https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/
It would be better to add this test to the existing generic/478 test
that tests OFD locks. Can you patch that to add a test for the new
functionality?
Thanks,
--
Jeff Layton <[email protected]>
22.06.2023 16:48, Jeff Layton пишет:
> I'm not opposed to adding a selftest here, but most filesystem testing
> is done via xfstests:
>
> https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/
>
> It would be better to add this test to the existing generic/478 test
> that tests OFD locks. Can you patch that to add a test for the new
> functionality?
I don't actually think its possible.
It seems like their script creates 2
t_ofd_locks processes, one for creating
the lock, one for testing it.
This is not our case.
To make it work our way, we'd probably
need to hack that test directly into
t_ofd_locks.c, so that it can set and test
from the same fd. And I don't know how
to even run these tests. :)
So I am really inclined to limit myself
with a kernel selftest.
22.06.2023 16:48, Jeff Layton пишет:
> I'm not sure this really belongs in the "locking" directory. Given that
> there is only the ww_mutex test in there, that's more for internal
> synchronization mechanisms, I think.
>
> Can you create a new "filelock" directory and drop this into there
> instead?
Done and sent the v3.
> On Jun 22, 2023, at 12:40 PM, stsp <[email protected]> wrote:
>
> 22.06.2023 16:48, Jeff Layton пишет:
>> I'm not opposed to adding a selftest here, but most filesystem testing
>> is done via xfstests:
>>
>> https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/
>>
>> It would be better to add this test to the existing generic/478 test
>> that tests OFD locks. Can you patch that to add a test for the new
>> functionality?
I agree with Jeff, this test needs to go in xfstests.
> I don't actually think its possible.
> It seems like their script creates 2
> t_ofd_locks processes, one for creating
> the lock, one for testing it.
> This is not our case.
> To make it work our way, we'd probably
> need to hack that test directly into
> t_ofd_locks.c, so that it can set and test
> from the same fd.
Or you could add a new test program.
> And I don't know how
> to even run these tests. :)
> So I am really inclined to limit myself
> with a kernel selftest.
IMO that's not a reason not to do this properly.
You should work with Jeff and the maintainer of
xfstests to make it happen.
--
Chuck Lever
22.06.2023 21:58, Chuck Lever III пишет:
> IMO that's not a reason not to do this properly.
>
> You should work with Jeff and the maintainer of
> xfstests to make it happen.
But its not going to be in this patch-set
anyway, as its a different source tree...
So I should prepare it when this is merged,
or?
> On Jun 22, 2023, at 1:05 PM, stsp <[email protected]> wrote:
>
>
> 22.06.2023 21:58, Chuck Lever III пишет:
>> IMO that's not a reason not to do this properly.
>>
>> You should work with Jeff and the maintainer of
>> xfstests to make it happen.
> But its not going to be in this patch-set
> anyway, as its a different source tree...
If others agree with me, then please drop the selftests
patch from this series. There is a considerably higher
probability that the new test will be run frequently
by CI if it's in xfstests.
> So I should prepare it when this is merged,
> or?
I don't have a strong preference. A good choice is to
push the test before the kernel changes are merged.
--
Chuck Lever
22.06.2023 22:12, Chuck Lever III пишет:
> I don't have a strong preference. A good choice is to
> push the test before the kernel changes are merged.
It will fail though w/o kernel changes.
So what exactly is the policy?
> > On Jun 22, 2023, at 1:05 PM, stsp <[email protected]> wrote:
> >
> >
> > 22.06.2023 21:58, Chuck Lever III пишет:
> >> IMO that's not a reason not to do this properly.
> >>
> >> You should work with Jeff and the maintainer of xfstests to make it
> >> happen.
> > But its not going to be in this patch-set anyway, as its a different
> > source tree...
>
> If others agree with me, then please drop the selftests patch from this series.
> There is a considerably higher probability that the new test will be run frequently
> by CI if it's in xfstests.
>
>
> > So I should prepare it when this is merged, or?
>
> I don't have a strong preference. A good choice is to push the test before the
> kernel changes are merged.
As an aside, an additional testing option for OFD locks is the multilock test tool that is in the nfs-ganesha project. In preparation to use OFD locks in Ganesha, I added them to multilock to check them out, and that incidentally also allows testing how the NFS client and server work when OFD locks are taken on a file from an NFS mount.
Frank
On Thu, Jun 22, 2023 at 10:31:06PM +0500, stsp wrote:
>
> 22.06.2023 22:12, Chuck Lever III пишет:
> > I don't have a strong preference. A good choice is to
> > push the test before the kernel changes are merged.
> It will fail though w/o kernel changes.
> So what exactly is the policy?
filesystem unit test functionality needs to be pushed into fstests
and/or ltp. The preference is the former, because just about every
filesystem developer and distro QA team is running this as part of
their every-day testing workflow.
fstests is written to probe whether the kernel supports a given
feature or not before testing it. It will _not_run() a test that
doesn't have the required kernel/fs/device support, and this is not
considered a test failure.
Yes, it means you have to also write the userspace feature probing
code, but that should be trivial to do because userspace already has
to be able to safely discover that this extension exists, right?
-Dave.
--
Dave Chinner
[email protected]
Hi,
22.06.2023 21:58, Chuck Lever III пишет:
> IMO that's not a reason not to do this properly.
>
> You should work with Jeff and the maintainer of
> xfstests to make it happen.
If only I knew who is that maintainer
and why he ignores all patches...
Can someone else please take a look at this:
https://www.spinics.net/lists/fstests/msg23115.html
and make a review/ack?