ltp.lists.linux.it archive mirror
 help / color / mirror / Atom feed
* [LTP] [PATCH 0/2] open_tree_attr syscall coverage
@ 2025-08-28 13:11 Andrea Cervesato
  2025-08-28 13:11 ` [LTP] [PATCH 1/2] Add open_tree_attr fallback Andrea Cervesato
  2025-08-28 13:11 ` [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant Andrea Cervesato
  0 siblings, 2 replies; 11+ messages in thread
From: Andrea Cervesato @ 2025-08-28 13:11 UTC (permalink / raw)
  To: ltp

open_tree_attr has been added in kernel 6.15 in the following commit:
https://lore.kernel.org/all/20250128-work-mnt_idmap-update-v2-v1-3-c25feb0d2eb3@kernel.org/

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
Andrea Cervesato (2):
      Add open_tree_attr fallback
      mount_setattr01: add open_tree_attr variant

 configure.ac                                       |  1 +
 include/lapi/fsmount.h                             |  8 +++
 .../syscalls/mount_setattr/mount_setattr01.c       | 69 ++++++++++++++++------
 3 files changed, 61 insertions(+), 17 deletions(-)
---
base-commit: 2f465c71bc999ed1796b69f68330495f2c69e131
change-id: 20250828-open_tree_attr-4b2ece576c1c

Best regards,
-- 
Andrea Cervesato <andrea.cervesato@suse.com>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 1/2] Add open_tree_attr fallback
  2025-08-28 13:11 [LTP] [PATCH 0/2] open_tree_attr syscall coverage Andrea Cervesato
@ 2025-08-28 13:11 ` Andrea Cervesato
  2025-08-28 13:11 ` [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant Andrea Cervesato
  1 sibling, 0 replies; 11+ messages in thread
From: Andrea Cervesato @ 2025-08-28 13:11 UTC (permalink / raw)
  To: ltp

From: Andrea Cervesato <andrea.cervesato@suse.com>

open_tree_attr syscall has been introduced in kernel 6.15 and it's
now defined in lapi/fsmount.h.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 configure.ac           | 1 +
 include/lapi/fsmount.h | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/configure.ac b/configure.ac
index 62ae27d494474fe342b1cceeac7f4c21c0b1028c..2d4d527bcaa675dae5d712a96d266d17184b8a0d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -137,6 +137,7 @@ AC_CHECK_FUNCS_ONCE([ \
     move_mount \
     name_to_handle_at \
     open_tree \
+    open_tree_attr \
     openat \
     openat2 \
     pidfd_getfd \
diff --git a/include/lapi/fsmount.h b/include/lapi/fsmount.h
index 1783272a00a1a049031cacbb417239123450a0e4..451987ae637d040e08970b7ffcf015bae07a6f11 100644
--- a/include/lapi/fsmount.h
+++ b/include/lapi/fsmount.h
@@ -105,6 +105,14 @@ static inline int open_tree(int dirfd, const char *pathname, unsigned int flags)
 }
 #endif /* HAVE_OPEN_TREE */
 
+#ifndef HAVE_OPEN_TREE_ATTR
+static inline int open_tree_attr(int dirfd, const char *pathname, unsigned int flags,
+				 struct mount_attr *attr, size_t size)
+{
+	return tst_syscall(__NR_open_tree_attr, dirfd, pathname, flags, attr, size);
+}
+#endif /* HAVE_OPEN_TREE_ATTR */
+
 #ifndef HAVE_MOUNT_SETATTR
 static inline int mount_setattr(int dirfd, const char *from_pathname, unsigned int flags,
 				struct mount_attr *attr, size_t size)

-- 
2.51.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant
  2025-08-28 13:11 [LTP] [PATCH 0/2] open_tree_attr syscall coverage Andrea Cervesato
  2025-08-28 13:11 ` [LTP] [PATCH 1/2] Add open_tree_attr fallback Andrea Cervesato
@ 2025-08-28 13:11 ` Andrea Cervesato
  2025-08-28 23:08   ` Wei Gao via ltp
  2025-08-29 12:58   ` Cyril Hrubis
  1 sibling, 2 replies; 11+ messages in thread
From: Andrea Cervesato @ 2025-08-28 13:11 UTC (permalink / raw)
  To: ltp

From: Andrea Cervesato <andrea.cervesato@suse.com>

This patch simply introduces a new variant to the mount_setattr01
test, in order to verify that open_tree_attr() works correctly.
The open_tree_attr() syscall has been introduced in the kernel v6.15 by
commit "c4a16820d901 - fs: add open_tree_attr()".

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 .../syscalls/mount_setattr/mount_setattr01.c       | 69 ++++++++++++++++------
 1 file changed, 52 insertions(+), 17 deletions(-)

diff --git a/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c b/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
index eb32cd91a7fcb4cb303e184dd40d5e10a280874c..bede26318a6985339b5b4bbb2db4a91744110eda 100644
--- a/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
+++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
@@ -3,10 +3,11 @@
  * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
  * Author: Dai Shili <daisl.fnst@fujitsu.com>
  * Author: Chen Hanxiao <chenhx.fnst@fujitsu.com>
+ * Copyright (C) 2025 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
  */
 
 /*\
- * Basic mount_setattr() test.
+ * Basic mount_setattr()/open_tree_attr() test.
  * Test whether the basic mount attributes are set correctly.
  *
  * Verify some MOUNT_SETATTR(2) attributes:
@@ -22,7 +23,8 @@
  * - MOUNT_ATTR_NODIRATIME - prevents updating access time for
  *   directories on this mount
  *
- * The functionality was added in v5.12.
+ * The mount_setattr functionality was added in v5.12, while the open_tree_attr
+ * functionality was added in v6.15.
  */
 
 #define _GNU_SOURCE
@@ -41,6 +43,7 @@
 	}
 
 static int mount_flag, otfd = -1;
+struct mount_attr *attr;
 
 static struct tcase {
 	char *name;
@@ -66,35 +69,59 @@ static void cleanup(void)
 static void setup(void)
 {
 	fsopen_supported_by_kernel();
-	struct stat st = {0};
 
-	if (stat(OT_MNTPOINT, &st) == -1)
+	if (access(OT_MNTPOINT, F_OK) != 0)
 		SAFE_MKDIR(OT_MNTPOINT, 0777);
 }
 
+static int open_tree_variant1(struct mount_attr *attr)
+{
+	tst_res(TINFO, "Variant using open_tree() + mount_setattr()");
+
+	otfd = TST_EXP_FD(open_tree(AT_FDCWD, MNTPOINT,
+			AT_EMPTY_PATH | OPEN_TREE_CLONE));
+	if (otfd == -1)
+		return -1;
+
+	TST_EXP_PASS(mount_setattr(otfd, "", AT_EMPTY_PATH,
+			attr, sizeof(*attr)));
+	if (TST_RET == -1)
+		return -1;
+
+	return otfd;
+}
+
+static int open_tree_variant2(struct mount_attr *attr)
+{
+	tst_res(TINFO, "Variant using open_tree_attr()");
+
+	otfd = TST_EXP_FD(open_tree_attr(AT_FDCWD, MNTPOINT,
+			AT_EMPTY_PATH | OPEN_TREE_CLONE,
+			attr, sizeof(*attr)));
+
+	return otfd;
+}
+
 static void run(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
-	struct mount_attr attr = {
-		.attr_set = tc->mount_attrs,
-	};
 	struct statvfs buf;
 
-	TST_EXP_FD_SILENT(open_tree(AT_FDCWD, MNTPOINT, AT_EMPTY_PATH |
-		AT_SYMLINK_NOFOLLOW | OPEN_TREE_CLOEXEC | OPEN_TREE_CLONE));
-	if (!TST_PASS)
-		return;
+	memset(attr, 0, sizeof(*attr));
+	attr->attr_set = tc->mount_attrs;
 
-	otfd = (int)TST_RET;
+	if (tst_variant)
+		otfd = open_tree_variant1(attr);
+	else
+		otfd = open_tree_variant2(attr);
 
-	TST_EXP_PASS_SILENT(mount_setattr(otfd, "", AT_EMPTY_PATH, &attr, sizeof(attr)),
-		"%s set", tc->name);
-	if (!TST_PASS)
-		goto out1;
+	if (otfd == -1)
+		goto out2;
 
 	TST_EXP_PASS_SILENT(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH));
 	if (!TST_PASS)
 		goto out1;
+
 	mount_flag = 1;
 	SAFE_CLOSE(otfd);
 
@@ -123,9 +150,17 @@ static struct tst_test test = {
 	.test = run,
 	.setup = setup,
 	.cleanup = cleanup,
+	.test_variants = 2,
 	.needs_root = 1,
 	.mount_device = 1,
 	.mntpoint = MNTPOINT,
 	.all_filesystems = 1,
-	.skip_filesystems = (const char *const []){"fuse", NULL},
+	.skip_filesystems = (const char *const []) {
+		"fuse",
+		NULL
+	},
+	.bufs = (struct tst_buffers []) {
+		{&attr, .size = sizeof(struct mount_attr)},
+		{}
+	}
 };

-- 
2.51.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant
  2025-08-28 13:11 ` [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant Andrea Cervesato
@ 2025-08-28 23:08   ` Wei Gao via ltp
  2025-08-29  9:31     ` Cyril Hrubis
  2025-08-29 12:06     ` Andrea Cervesato via ltp
  2025-08-29 12:58   ` Cyril Hrubis
  1 sibling, 2 replies; 11+ messages in thread
From: Wei Gao via ltp @ 2025-08-28 23:08 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

On Thu, Aug 28, 2025 at 03:11:25PM +0200, Andrea Cervesato wrote:
> From: Andrea Cervesato <andrea.cervesato@suse.com>
> 
> This patch simply introduces a new variant to the mount_setattr01
> test, in order to verify that open_tree_attr() works correctly.
> The open_tree_attr() syscall has been introduced in the kernel v6.15 by
> commit "c4a16820d901 - fs: add open_tree_attr()".
> 
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  .../syscalls/mount_setattr/mount_setattr01.c       | 69 ++++++++++++++++------
>  1 file changed, 52 insertions(+), 17 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c b/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
> index eb32cd91a7fcb4cb303e184dd40d5e10a280874c..bede26318a6985339b5b4bbb2db4a91744110eda 100644
> --- a/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
> +++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
> @@ -3,10 +3,11 @@
>   * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
>   * Author: Dai Shili <daisl.fnst@fujitsu.com>
>   * Author: Chen Hanxiao <chenhx.fnst@fujitsu.com>
> + * Copyright (C) 2025 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>   */
>  
>  /*\
> - * Basic mount_setattr() test.
> + * Basic mount_setattr()/open_tree_attr() test.
>   * Test whether the basic mount attributes are set correctly.
>   *
>   * Verify some MOUNT_SETATTR(2) attributes:
> @@ -22,7 +23,8 @@
>   * - MOUNT_ATTR_NODIRATIME - prevents updating access time for
>   *   directories on this mount
>   *
> - * The functionality was added in v5.12.
> + * The mount_setattr functionality was added in v5.12, while the open_tree_attr
> + * functionality was added in v6.15.
>   */
>  
>  #define _GNU_SOURCE
> @@ -41,6 +43,7 @@
>  	}
>  
>  static int mount_flag, otfd = -1;
> +struct mount_attr *attr;
>  
>  static struct tcase {
>  	char *name;
> @@ -66,35 +69,59 @@ static void cleanup(void)
>  static void setup(void)
>  {
>  	fsopen_supported_by_kernel();
> -	struct stat st = {0};
>  
> -	if (stat(OT_MNTPOINT, &st) == -1)
> +	if (access(OT_MNTPOINT, F_OK) != 0)
>  		SAFE_MKDIR(OT_MNTPOINT, 0777);
>  }
>  
> +static int open_tree_variant1(struct mount_attr *attr)
> +{
> +	tst_res(TINFO, "Variant using open_tree() + mount_setattr()");
> +
> +	otfd = TST_EXP_FD(open_tree(AT_FDCWD, MNTPOINT,
> +			AT_EMPTY_PATH | OPEN_TREE_CLONE));
> +	if (otfd == -1)
> +		return -1;
> +
> +	TST_EXP_PASS(mount_setattr(otfd, "", AT_EMPTY_PATH,
> +			attr, sizeof(*attr)));
Duplicate mount_setattr? There is another mount_setattr after this
called.
> +	if (TST_RET == -1)
> +		return -1;
> +
> +	return otfd;
> +}
> +
> +static int open_tree_variant2(struct mount_attr *attr)
> +{
> +	tst_res(TINFO, "Variant using open_tree_attr()");
> +
> +	otfd = TST_EXP_FD(open_tree_attr(AT_FDCWD, MNTPOINT,
> +			AT_EMPTY_PATH | OPEN_TREE_CLONE,
> +			attr, sizeof(*attr)));
> +
> +	return otfd;
> +}
> +
>  static void run(unsigned int n)
>  {
>  	struct tcase *tc = &tcases[n];
> -	struct mount_attr attr = {
> -		.attr_set = tc->mount_attrs,
> -	};
>  	struct statvfs buf;
>  
> -	TST_EXP_FD_SILENT(open_tree(AT_FDCWD, MNTPOINT, AT_EMPTY_PATH |
> -		AT_SYMLINK_NOFOLLOW | OPEN_TREE_CLOEXEC | OPEN_TREE_CLONE));
> -	if (!TST_PASS)
> -		return;
> +	memset(attr, 0, sizeof(*attr));
> +	attr->attr_set = tc->mount_attrs;
>  
> -	otfd = (int)TST_RET;
> +	if (tst_variant)
> +		otfd = open_tree_variant1(attr);
> +	else
> +		otfd = open_tree_variant2(attr);
I am not sure this is perfect way loop the function, add function point into struct tcase
is better in my opinion.
>  
> -	TST_EXP_PASS_SILENT(mount_setattr(otfd, "", AT_EMPTY_PATH, &attr, sizeof(attr)),
> -		"%s set", tc->name);
> -	if (!TST_PASS)
> -		goto out1;
> +	if (otfd == -1)
> +		goto out2;
>  
>  	TST_EXP_PASS_SILENT(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH));
I suppose we can also check mounts option take effect or not like:
https://patchwork.ozlabs.org/project/ltp/patch/20250828225157.982-1-wegao@suse.com/
>  	if (!TST_PASS)
>  		goto out1;
> +
>  	mount_flag = 1;
>  	SAFE_CLOSE(otfd);
>  
> @@ -123,9 +150,17 @@ static struct tst_test test = {
>  	.test = run,
>  	.setup = setup,
>  	.cleanup = cleanup,
> +	.test_variants = 2,
>  	.needs_root = 1,
>  	.mount_device = 1,
>  	.mntpoint = MNTPOINT,
>  	.all_filesystems = 1,
> -	.skip_filesystems = (const char *const []){"fuse", NULL},
> +	.skip_filesystems = (const char *const []) {
> +		"fuse",
> +		NULL
> +	},
> +	.bufs = (struct tst_buffers []) {
> +		{&attr, .size = sizeof(struct mount_attr)},
> +		{}
> +	}
>  };
> 
> -- 
> 2.51.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant
  2025-08-28 23:08   ` Wei Gao via ltp
@ 2025-08-29  9:31     ` Cyril Hrubis
  2025-08-29 12:06       ` Andrea Cervesato via ltp
  2025-08-29 12:06     ` Andrea Cervesato via ltp
  1 sibling, 1 reply; 11+ messages in thread
From: Cyril Hrubis @ 2025-08-29  9:31 UTC (permalink / raw)
  To: Wei Gao; +Cc: ltp

Hi!
> > +static int open_tree_variant1(struct mount_attr *attr)
> > +{
> > +	tst_res(TINFO, "Variant using open_tree() + mount_setattr()");
> > +
> > +	otfd = TST_EXP_FD(open_tree(AT_FDCWD, MNTPOINT,
> > +			AT_EMPTY_PATH | OPEN_TREE_CLONE));
> > +	if (otfd == -1)
> > +		return -1;
> > +
> > +	TST_EXP_PASS(mount_setattr(otfd, "", AT_EMPTY_PATH,
> > +			attr, sizeof(*attr)));
> Duplicate mount_setattr? There is another mount_setattr after this
> called.

As far as I can tell the call that was done later has been removed in
this patch. That is because both of the variants do open_tree and
mount_setattr in the open_tree_variant() functions.

> > +	if (TST_RET == -1)
> > +		return -1;
> > +
> > +	return otfd;
> > +}
> > +
> > +static int open_tree_variant2(struct mount_attr *attr)
> > +{
> > +	tst_res(TINFO, "Variant using open_tree_attr()");
> > +
> > +	otfd = TST_EXP_FD(open_tree_attr(AT_FDCWD, MNTPOINT,
> > +			AT_EMPTY_PATH | OPEN_TREE_CLONE,
> > +			attr, sizeof(*attr)));
> > +
> > +	return otfd;
> > +}
> > +
> >  static void run(unsigned int n)
> >  {
> >  	struct tcase *tc = &tcases[n];
> > -	struct mount_attr attr = {
> > -		.attr_set = tc->mount_attrs,
> > -	};
> >  	struct statvfs buf;
> >  
> > -	TST_EXP_FD_SILENT(open_tree(AT_FDCWD, MNTPOINT, AT_EMPTY_PATH |
> > -		AT_SYMLINK_NOFOLLOW | OPEN_TREE_CLOEXEC | OPEN_TREE_CLONE));
> > -	if (!TST_PASS)
> > -		return;
> > +	memset(attr, 0, sizeof(*attr));
> > +	attr->attr_set = tc->mount_attrs;
> >  
> > -	otfd = (int)TST_RET;
> > +	if (tst_variant)
> > +		otfd = open_tree_variant1(attr);
> > +	else
> > +		otfd = open_tree_variant2(attr);
> I am not sure this is perfect way loop the function, add function point into struct tcase
> is better in my opinion.

The tst_variant was implemented exactly for the case where we have
different syscalls doing the same job. In this case a shortcut was added
that does open_tree + mount_setattr in a single syscall, so using
variants is very reasonable choice.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant
  2025-08-28 23:08   ` Wei Gao via ltp
  2025-08-29  9:31     ` Cyril Hrubis
@ 2025-08-29 12:06     ` Andrea Cervesato via ltp
  1 sibling, 0 replies; 11+ messages in thread
From: Andrea Cervesato via ltp @ 2025-08-29 12:06 UTC (permalink / raw)
  To: Wei Gao, Andrea Cervesato; +Cc: ltp

Hi!

On 8/29/25 1:08 AM, Wei Gao wrote:
>> -	otfd = (int)TST_RET;
>> +	if (tst_variant)
>> +		otfd = open_tree_variant1(attr);
>> +	else
>> +		otfd = open_tree_variant2(attr);
> I am not sure this is perfect way loop the function, add function point into struct tcase
> is better in my opinion.
That's actually an overkill, considering we already have one variable 
deciding what variant we are going to execute. To add a pointer in tcase 
it means to define a type, declare a pointer, initialize it and to use it.
>>   
>> -	TST_EXP_PASS_SILENT(mount_setattr(otfd, "", AT_EMPTY_PATH, &attr, sizeof(attr)),
>> -		"%s set", tc->name);
>> -	if (!TST_PASS)
>> -		goto out1;
>> +	if (otfd == -1)
>> +		goto out2;
>>   
>>   	TST_EXP_PASS_SILENT(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH));
> I suppose we can also check mounts option take effect or not like:
> https://patchwork.ozlabs.org/project/ltp/patch/20250828225157.982-1-wegao@suse.com/

Also this would require a map between mount attrs and strings which are 
going to be converted back again. It would make sense if we were testing 
move_mount, but we already have the attr information via tcase and statvfs.

- Andrea

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant
  2025-08-29  9:31     ` Cyril Hrubis
@ 2025-08-29 12:06       ` Andrea Cervesato via ltp
  2025-08-29 12:55         ` Cyril Hrubis
  0 siblings, 1 reply; 11+ messages in thread
From: Andrea Cervesato via ltp @ 2025-08-29 12:06 UTC (permalink / raw)
  To: Cyril Hrubis, Wei Gao; +Cc: ltp

Hi!

On 8/29/25 11:31 AM, Cyril Hrubis wrote:
> The tst_variant was implemented exactly for the case where we have
> different syscalls doing the same job. In this case a shortcut was added
> that does open_tree + mount_setattr in a single syscall, so using
> variants is very reasonable choice.

Is it a free for all to merge the patch?

- Andrea


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant
  2025-08-29 12:06       ` Andrea Cervesato via ltp
@ 2025-08-29 12:55         ` Cyril Hrubis
  2025-09-01 10:28           ` Andrea Cervesato via ltp
  0 siblings, 1 reply; 11+ messages in thread
From: Cyril Hrubis @ 2025-08-29 12:55 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
> > The tst_variant was implemented exactly for the case where we have
> > different syscalls doing the same job. In this case a shortcut was added
> > that does open_tree + mount_setattr in a single syscall, so using
> > variants is very reasonable choice.
> 
> Is it a free for all to merge the patch?

And does it work correctly on kernels older than 6.15? I do not see any
checks for the open_tree_attr() syscall availability.

Apart from that there are a few minor things to fix, I will do a proper
review.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant
  2025-08-28 13:11 ` [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant Andrea Cervesato
  2025-08-28 23:08   ` Wei Gao via ltp
@ 2025-08-29 12:58   ` Cyril Hrubis
  1 sibling, 0 replies; 11+ messages in thread
From: Cyril Hrubis @ 2025-08-29 12:58 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
>  /*\
> - * Basic mount_setattr() test.
> + * Basic mount_setattr()/open_tree_attr() test.
>   * Test whether the basic mount attributes are set correctly.
>   *
>   * Verify some MOUNT_SETATTR(2) attributes:
> @@ -22,7 +23,8 @@
>   * - MOUNT_ATTR_NODIRATIME - prevents updating access time for
>   *   directories on this mount
>   *
> - * The functionality was added in v5.12.
> + * The mount_setattr functionality was added in v5.12, while the open_tree_attr
> + * functionality was added in v6.15.
>   */
>  
>  #define _GNU_SOURCE
> @@ -41,6 +43,7 @@
>  	}
>  
>  static int mount_flag, otfd = -1;
> +struct mount_attr *attr;
>  
>  static struct tcase {
>  	char *name;
> @@ -66,35 +69,59 @@ static void cleanup(void)
>  static void setup(void)
>  {
>  	fsopen_supported_by_kernel();
> -	struct stat st = {0};
>  
> -	if (stat(OT_MNTPOINT, &st) == -1)
> +	if (access(OT_MNTPOINT, F_OK) != 0)
>  		SAFE_MKDIR(OT_MNTPOINT, 0777);
>  }
>  
> +static int open_tree_variant1(struct mount_attr *attr)
> +{
> +	tst_res(TINFO, "Variant using open_tree() + mount_setattr()");
> +
> +	otfd = TST_EXP_FD(open_tree(AT_FDCWD, MNTPOINT,
> +			AT_EMPTY_PATH | OPEN_TREE_CLONE));
> +	if (otfd == -1)
> +		return -1;
> +
> +	TST_EXP_PASS(mount_setattr(otfd, "", AT_EMPTY_PATH,
> +			attr, sizeof(*attr)));
> +	if (TST_RET == -1)
> +		return -1;

close the otfd here?

> +	return otfd;
> +}
> +
> +static int open_tree_variant2(struct mount_attr *attr)
> +{
> +	tst_res(TINFO, "Variant using open_tree_attr()");
> +
> +	otfd = TST_EXP_FD(open_tree_attr(AT_FDCWD, MNTPOINT,
> +			AT_EMPTY_PATH | OPEN_TREE_CLONE,
> +			attr, sizeof(*attr)));
> +
> +	return otfd;
> +}

In the original test the TST_EXP_* were _SILENT variants

>  static void run(unsigned int n)
>  {
>  	struct tcase *tc = &tcases[n];
> -	struct mount_attr attr = {
> -		.attr_set = tc->mount_attrs,
> -	};
>  	struct statvfs buf;
>  
> -	TST_EXP_FD_SILENT(open_tree(AT_FDCWD, MNTPOINT, AT_EMPTY_PATH |
> -		AT_SYMLINK_NOFOLLOW | OPEN_TREE_CLOEXEC | OPEN_TREE_CLONE));
> -	if (!TST_PASS)
> -		return;
> +	memset(attr, 0, sizeof(*attr));
> +	attr->attr_set = tc->mount_attrs;
>  
> -	otfd = (int)TST_RET;
> +	if (tst_variant)
> +		otfd = open_tree_variant1(attr);
> +	else
> +		otfd = open_tree_variant2(attr);
>  
> -	TST_EXP_PASS_SILENT(mount_setattr(otfd, "", AT_EMPTY_PATH, &attr, sizeof(attr)),
> -		"%s set", tc->name);
> -	if (!TST_PASS)
> -		goto out1;
> +	if (otfd == -1)
> +		goto out2;
>  
>  	TST_EXP_PASS_SILENT(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH));
>  	if (!TST_PASS)
>  		goto out1;
> +
>  	mount_flag = 1;
>  	SAFE_CLOSE(otfd);
>  
> @@ -123,9 +150,17 @@ static struct tst_test test = {
>  	.test = run,
>  	.setup = setup,
>  	.cleanup = cleanup,
> +	.test_variants = 2,
>  	.needs_root = 1,
>  	.mount_device = 1,
>  	.mntpoint = MNTPOINT,
>  	.all_filesystems = 1,
> -	.skip_filesystems = (const char *const []){"fuse", NULL},
> +	.skip_filesystems = (const char *const []) {
> +		"fuse",
> +		NULL
> +	},

Such cleanups should ideally be done in a separate patch.

> +	.bufs = (struct tst_buffers []) {
> +		{&attr, .size = sizeof(struct mount_attr)},
> +		{}
> +	}
>  };
> 
> -- 
> 2.51.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant
  2025-08-29 12:55         ` Cyril Hrubis
@ 2025-09-01 10:28           ` Andrea Cervesato via ltp
  2025-09-01 15:48             ` Cyril Hrubis
  0 siblings, 1 reply; 11+ messages in thread
From: Andrea Cervesato via ltp @ 2025-09-01 10:28 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

Hi!

On 8/29/25 2:55 PM, Cyril Hrubis wrote:
> And does it work correctly on kernels older than 6.15? I do not see any
> checks for the open_tree_attr() syscall availability.

We discussed time ago that we prefer to have a tst_syscall() TCONF 
caused by missing syscall, rather than checking that syscall is present 
all the times via autoconf.

- Andrea


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant
  2025-09-01 10:28           ` Andrea Cervesato via ltp
@ 2025-09-01 15:48             ` Cyril Hrubis
  0 siblings, 0 replies; 11+ messages in thread
From: Cyril Hrubis @ 2025-09-01 15:48 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp

Hi!
> > And does it work correctly on kernels older than 6.15? I do not see any
> > checks for the open_tree_attr() syscall availability.
> 
> We discussed time ago that we prefer to have a tst_syscall() TCONF 
> caused by missing syscall, rather than checking that syscall is present 
> all the times via autoconf.

I wasn't speaking about autoconf, that never works for kernel support.

I was afraid that tst_brk() from tst_syscall() would exit the whole
test, but it looks like the test library is smart enough and it does not
set the abort_flag for TCONF and the test will continue running
even if one of the variants called tst_brk(TCONF, ...). So I suppose
that the test actually runs correctly on kernel that does not support
open_tree_attr() even with -i 10.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-09-01 15:47 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-28 13:11 [LTP] [PATCH 0/2] open_tree_attr syscall coverage Andrea Cervesato
2025-08-28 13:11 ` [LTP] [PATCH 1/2] Add open_tree_attr fallback Andrea Cervesato
2025-08-28 13:11 ` [LTP] [PATCH 2/2] mount_setattr01: add open_tree_attr variant Andrea Cervesato
2025-08-28 23:08   ` Wei Gao via ltp
2025-08-29  9:31     ` Cyril Hrubis
2025-08-29 12:06       ` Andrea Cervesato via ltp
2025-08-29 12:55         ` Cyril Hrubis
2025-09-01 10:28           ` Andrea Cervesato via ltp
2025-09-01 15:48             ` Cyril Hrubis
2025-08-29 12:06     ` Andrea Cervesato via ltp
2025-08-29 12:58   ` Cyril Hrubis

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).