All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] selftests/mount_setattr: fix redefine struct mount_attr build error
@ 2023-02-13 18:31 Shuah Khan
  2023-02-13 23:50 ` Seth Forshee
  0 siblings, 1 reply; 6+ messages in thread
From: Shuah Khan @ 2023-02-13 18:31 UTC (permalink / raw)
  To: shuah, brauner, sforshee
  Cc: Shuah Khan, linux-fsdevel, linux-kselftest, linux-kernel

Fix the following build error due to redefining struct mount_attr by
removing duplicate define from mount_setattr_test.c

gcc -g -isystem .../tools/testing/selftests/../../../usr/include -Wall -O2 -pthread     mount_setattr_test.c  -o .../tools/testing/selftests/mount_setattr/mount_setattr_test
mount_setattr_test.c:107:8: error: redefinition of ‘struct mount_attr’
  107 | struct mount_attr {
      |        ^~~~~~~~~~
In file included from /usr/include/x86_64-linux-gnu/sys/mount.h:32,
                 from mount_setattr_test.c:10:
.../usr/include/linux/mount.h:129:8: note: originally defined here
  129 | struct mount_attr {
      |        ^~~~~~~~~~
make: *** [../lib.mk:145: .../tools/testing/selftests/mount_setattr/mount_setattr_test] Error 1

Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
 tools/testing/selftests/mount_setattr/mount_setattr_test.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/tools/testing/selftests/mount_setattr/mount_setattr_test.c b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
index 8c5fea68ae67..582669ca38e9 100644
--- a/tools/testing/selftests/mount_setattr/mount_setattr_test.c
+++ b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
@@ -103,13 +103,6 @@
 	#else
 		#define __NR_mount_setattr 442
 	#endif
-
-struct mount_attr {
-	__u64 attr_set;
-	__u64 attr_clr;
-	__u64 propagation;
-	__u64 userns_fd;
-};
 #endif
 
 #ifndef __NR_open_tree
-- 
2.37.2


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

* Re: [PATCH] selftests/mount_setattr: fix redefine struct mount_attr build error
  2023-02-13 18:31 [PATCH] selftests/mount_setattr: fix redefine struct mount_attr build error Shuah Khan
@ 2023-02-13 23:50 ` Seth Forshee
  2023-02-14 17:10   ` Shuah Khan
  0 siblings, 1 reply; 6+ messages in thread
From: Seth Forshee @ 2023-02-13 23:50 UTC (permalink / raw)
  To: Shuah Khan; +Cc: shuah, brauner, linux-fsdevel, linux-kselftest, linux-kernel

On Mon, Feb 13, 2023 at 11:31:49AM -0700, Shuah Khan wrote:
> Fix the following build error due to redefining struct mount_attr by
> removing duplicate define from mount_setattr_test.c
> 
> gcc -g -isystem .../tools/testing/selftests/../../../usr/include -Wall -O2 -pthread     mount_setattr_test.c  -o .../tools/testing/selftests/mount_setattr/mount_setattr_test
> mount_setattr_test.c:107:8: error: redefinition of ‘struct mount_attr’
>   107 | struct mount_attr {
>       |        ^~~~~~~~~~
> In file included from /usr/include/x86_64-linux-gnu/sys/mount.h:32,
>                  from mount_setattr_test.c:10:
> .../usr/include/linux/mount.h:129:8: note: originally defined here
>   129 | struct mount_attr {
>       |        ^~~~~~~~~~
> make: *** [../lib.mk:145: .../tools/testing/selftests/mount_setattr/mount_setattr_test] Error 1
> 
> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> ---
>  tools/testing/selftests/mount_setattr/mount_setattr_test.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/mount_setattr/mount_setattr_test.c b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
> index 8c5fea68ae67..582669ca38e9 100644
> --- a/tools/testing/selftests/mount_setattr/mount_setattr_test.c
> +++ b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
> @@ -103,13 +103,6 @@
>  	#else
>  		#define __NR_mount_setattr 442
>  	#endif
> -
> -struct mount_attr {
> -	__u64 attr_set;
> -	__u64 attr_clr;
> -	__u64 propagation;
> -	__u64 userns_fd;
> -};
>  #endif

The difficulty with this is that whether or not you need this definition
depends on your system headers. My laptop doesn't have definitions for
either __NR_mount_setattr or struct mount_attr, so for me the build
works without this patch but fails with it.

I suppose we could fix this universally by using a different name for
the struct in the test, e.g.:

diff --git a/tools/testing/selftests/mount_setattr/mount_setattr_test.c b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
index 8c5fea68ae67..0658129d526e 100644
--- a/tools/testing/selftests/mount_setattr/mount_setattr_test.c
+++ b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
@@ -103,13 +103,6 @@
 	#else
 		#define __NR_mount_setattr 442
 	#endif
-
-struct mount_attr {
-	__u64 attr_set;
-	__u64 attr_clr;
-	__u64 propagation;
-	__u64 userns_fd;
-};
 #endif
 
 #ifndef __NR_open_tree
@@ -132,6 +125,13 @@ struct mount_attr {
 	#endif
 #endif
 
+struct __mount_attr {
+	__u64 attr_set;
+	__u64 attr_clr;
+	__u64 propagation;
+	__u64 userns_fd;
+};
+
 #ifndef MOUNT_ATTR_IDMAP
 #define MOUNT_ATTR_IDMAP 0x00100000
 #endif
@@ -141,7 +141,7 @@ struct mount_attr {
 #endif
 
 static inline int sys_mount_setattr(int dfd, const char *path, unsigned int flags,
-				    struct mount_attr *attr, size_t size)
+				    struct __mount_attr *attr, size_t size)
 {
 	return syscall(__NR_mount_setattr, dfd, path, flags, attr, size);
 }
@@ -347,7 +347,7 @@ static bool is_shared_mount(const char *path)
 
 static void *mount_setattr_thread(void *data)
 {
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set	= MOUNT_ATTR_RDONLY | MOUNT_ATTR_NOSUID,
 		.attr_clr	= 0,
 		.propagation	= MS_SHARED,
@@ -445,7 +445,7 @@ FIXTURE_TEARDOWN(mount_setattr)
 
 TEST_F(mount_setattr, invalid_attributes)
 {
-	struct mount_attr invalid_attr = {
+	struct __mount_attr invalid_attr = {
 		.attr_set = (1U << 31),
 	};
 
@@ -479,11 +479,11 @@ TEST_F(mount_setattr, extensibility)
 {
 	unsigned int old_flags = 0, new_flags = 0, expected_flags = 0;
 	char *s = "dummy";
-	struct mount_attr invalid_attr = {};
+	struct __mount_attr invalid_attr = {};
 	struct mount_attr_large {
-		struct mount_attr attr1;
-		struct mount_attr attr2;
-		struct mount_attr attr3;
+		struct __mount_attr attr1;
+		struct __mount_attr attr2;
+		struct __mount_attr attr3;
 	} large_attr = {};
 
 	if (!mount_setattr_supported())
@@ -542,7 +542,7 @@ TEST_F(mount_setattr, extensibility)
 TEST_F(mount_setattr, basic)
 {
 	unsigned int old_flags = 0, new_flags = 0, expected_flags = 0;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set	= MOUNT_ATTR_RDONLY | MOUNT_ATTR_NOEXEC | MOUNT_ATTR_RELATIME,
 		.attr_clr	= MOUNT_ATTR__ATIME,
 	};
@@ -578,7 +578,7 @@ TEST_F(mount_setattr, basic_recursive)
 {
 	int fd;
 	unsigned int old_flags = 0, new_flags = 0, expected_flags = 0;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set	= MOUNT_ATTR_RDONLY | MOUNT_ATTR_NOEXEC | MOUNT_ATTR_RELATIME,
 		.attr_clr	= MOUNT_ATTR__ATIME,
 	};
@@ -672,7 +672,7 @@ TEST_F(mount_setattr, mount_has_writers)
 {
 	int fd, dfd;
 	unsigned int old_flags = 0, new_flags = 0;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set	= MOUNT_ATTR_RDONLY | MOUNT_ATTR_NOEXEC | MOUNT_ATTR_RELATIME,
 		.attr_clr	= MOUNT_ATTR__ATIME,
 		.propagation	= MS_SHARED,
@@ -729,7 +729,7 @@ TEST_F(mount_setattr, mount_has_writers)
 TEST_F(mount_setattr, mixed_mount_options)
 {
 	unsigned int old_flags1 = 0, old_flags2 = 0, new_flags = 0, expected_flags = 0;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_clr = MOUNT_ATTR_RDONLY | MOUNT_ATTR_NOSUID | MOUNT_ATTR_NOEXEC | MOUNT_ATTR__ATIME,
 		.attr_set = MOUNT_ATTR_RELATIME,
 	};
@@ -763,7 +763,7 @@ TEST_F(mount_setattr, mixed_mount_options)
 TEST_F(mount_setattr, time_changes)
 {
 	unsigned int old_flags = 0, new_flags = 0, expected_flags = 0;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set	= MOUNT_ATTR_NODIRATIME | MOUNT_ATTR_NOATIME,
 	};
 
@@ -967,7 +967,7 @@ TEST_F(mount_setattr, multi_threaded)
 TEST_F(mount_setattr, wrong_user_namespace)
 {
 	int ret;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set = MOUNT_ATTR_RDONLY,
 	};
 
@@ -983,7 +983,7 @@ TEST_F(mount_setattr, wrong_user_namespace)
 TEST_F(mount_setattr, wrong_mount_namespace)
 {
 	int fd, ret;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set = MOUNT_ATTR_RDONLY,
 	};
 
@@ -1074,7 +1074,7 @@ FIXTURE_TEARDOWN(mount_setattr_idmapped)
  */
 TEST_F(mount_setattr_idmapped, invalid_fd_negative)
 {
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set	= MOUNT_ATTR_IDMAP,
 		.userns_fd	= -EBADF,
 	};
@@ -1092,7 +1092,7 @@ TEST_F(mount_setattr_idmapped, invalid_fd_negative)
  */
 TEST_F(mount_setattr_idmapped, invalid_fd_large)
 {
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set	= MOUNT_ATTR_IDMAP,
 		.userns_fd	= INT64_MAX,
 	};
@@ -1111,7 +1111,7 @@ TEST_F(mount_setattr_idmapped, invalid_fd_large)
 TEST_F(mount_setattr_idmapped, invalid_fd_closed)
 {
 	int fd;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set = MOUNT_ATTR_IDMAP,
 	};
 
@@ -1134,7 +1134,7 @@ TEST_F(mount_setattr_idmapped, invalid_fd_closed)
 TEST_F(mount_setattr_idmapped, invalid_fd_initial_userns)
 {
 	int open_tree_fd = -EBADF;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set = MOUNT_ATTR_IDMAP,
 	};
 
@@ -1243,7 +1243,7 @@ static int get_userns_fd(unsigned long nsid, unsigned long hostid, unsigned long
 TEST_F(mount_setattr_idmapped, attached_mount_inside_current_mount_namespace)
 {
 	int open_tree_fd = -EBADF;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set = MOUNT_ATTR_IDMAP,
 	};
 
@@ -1273,7 +1273,7 @@ TEST_F(mount_setattr_idmapped, attached_mount_inside_current_mount_namespace)
 TEST_F(mount_setattr_idmapped, attached_mount_outside_current_mount_namespace)
 {
 	int open_tree_fd = -EBADF;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set = MOUNT_ATTR_IDMAP,
 	};
 
@@ -1303,7 +1303,7 @@ TEST_F(mount_setattr_idmapped, attached_mount_outside_current_mount_namespace)
 TEST_F(mount_setattr_idmapped, detached_mount_inside_current_mount_namespace)
 {
 	int open_tree_fd = -EBADF;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set = MOUNT_ATTR_IDMAP,
 	};
 
@@ -1333,7 +1333,7 @@ TEST_F(mount_setattr_idmapped, detached_mount_inside_current_mount_namespace)
 TEST_F(mount_setattr_idmapped, detached_mount_outside_current_mount_namespace)
 {
 	int open_tree_fd = -EBADF;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set = MOUNT_ATTR_IDMAP,
 	};
 
@@ -1365,7 +1365,7 @@ TEST_F(mount_setattr_idmapped, detached_mount_outside_current_mount_namespace)
 TEST_F(mount_setattr_idmapped, change_idmapping)
 {
 	int open_tree_fd = -EBADF;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set = MOUNT_ATTR_IDMAP,
 	};
 
@@ -1410,7 +1410,7 @@ static bool expected_uid_gid(int dfd, const char *path, int flags,
 TEST_F(mount_setattr_idmapped, idmap_mount_tree_invalid)
 {
 	int open_tree_fd = -EBADF;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set = MOUNT_ATTR_IDMAP,
 	};
 
@@ -1445,7 +1445,7 @@ TEST_F(mount_setattr, mount_attr_nosymfollow)
 {
 	int fd;
 	unsigned int old_flags = 0, new_flags = 0, expected_flags = 0;
-	struct mount_attr attr = {
+	struct __mount_attr attr = {
 		.attr_set	= MOUNT_ATTR_NOSYMFOLLOW,
 	};

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

* Re: [PATCH] selftests/mount_setattr: fix redefine struct mount_attr build error
  2023-02-13 23:50 ` Seth Forshee
@ 2023-02-14 17:10   ` Shuah Khan
  2023-02-14 20:45     ` Seth Forshee
  0 siblings, 1 reply; 6+ messages in thread
From: Shuah Khan @ 2023-02-14 17:10 UTC (permalink / raw)
  To: Seth Forshee
  Cc: shuah, brauner, linux-fsdevel, linux-kselftest, linux-kernel, Shuah Khan

On 2/13/23 16:50, Seth Forshee wrote:
> On Mon, Feb 13, 2023 at 11:31:49AM -0700, Shuah Khan wrote:
>> Fix the following build error due to redefining struct mount_attr by
>> removing duplicate define from mount_setattr_test.c
>>
>> gcc -g -isystem .../tools/testing/selftests/../../../usr/include -Wall -O2 -pthread     mount_setattr_test.c  -o .../tools/testing/selftests/mount_setattr/mount_setattr_test
>> mount_setattr_test.c:107:8: error: redefinition of ‘struct mount_attr’
>>    107 | struct mount_attr {
>>        |        ^~~~~~~~~~
>> In file included from /usr/include/x86_64-linux-gnu/sys/mount.h:32,
>>                   from mount_setattr_test.c:10:
>> .../usr/include/linux/mount.h:129:8: note: originally defined here
>>    129 | struct mount_attr {
>>        |        ^~~~~~~~~~
>> make: *** [../lib.mk:145: .../tools/testing/selftests/mount_setattr/mount_setattr_test] Error 1
>>
>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
>> ---
>>   tools/testing/selftests/mount_setattr/mount_setattr_test.c | 7 -------
>>   1 file changed, 7 deletions(-)
>>
>> diff --git a/tools/testing/selftests/mount_setattr/mount_setattr_test.c b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
>> index 8c5fea68ae67..582669ca38e9 100644
>> --- a/tools/testing/selftests/mount_setattr/mount_setattr_test.c
>> +++ b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
>> @@ -103,13 +103,6 @@
>>   	#else
>>   		#define __NR_mount_setattr 442
>>   	#endif
>> -
>> -struct mount_attr {
>> -	__u64 attr_set;
>> -	__u64 attr_clr;
>> -	__u64 propagation;
>> -	__u64 userns_fd;
>> -};
>>   #endif
> 
> The difficulty with this is that whether or not you need this definition
> depends on your system headers. My laptop doesn't have definitions for
> either __NR_mount_setattr or struct mount_attr, so for me the build
> works without this patch but fails with it.
> 

The header search looks up system headers followed by installed headers in
the repo (both in-tree and out-of-tree builds). kselftest builds do depend
on headers_install. Did you building after running headers_install?

> I suppose we could fix this universally by using a different name for
> the struct in the test, e.g.:
> 

This is not a good way to for a couple of reasons. This masks any problems
due to incompatibility between these defines.

thanks,
-- Shuah


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

* Re: [PATCH] selftests/mount_setattr: fix redefine struct mount_attr build error
  2023-02-14 17:10   ` Shuah Khan
@ 2023-02-14 20:45     ` Seth Forshee
  2023-02-14 23:37       ` Shuah Khan
  0 siblings, 1 reply; 6+ messages in thread
From: Seth Forshee @ 2023-02-14 20:45 UTC (permalink / raw)
  To: Shuah Khan; +Cc: shuah, brauner, linux-fsdevel, linux-kselftest, linux-kernel

On Tue, Feb 14, 2023 at 10:10:00AM -0700, Shuah Khan wrote:
> On 2/13/23 16:50, Seth Forshee wrote:
> > On Mon, Feb 13, 2023 at 11:31:49AM -0700, Shuah Khan wrote:
> > > Fix the following build error due to redefining struct mount_attr by
> > > removing duplicate define from mount_setattr_test.c
> > > 
> > > gcc -g -isystem .../tools/testing/selftests/../../../usr/include -Wall -O2 -pthread     mount_setattr_test.c  -o .../tools/testing/selftests/mount_setattr/mount_setattr_test
> > > mount_setattr_test.c:107:8: error: redefinition of ‘struct mount_attr’
> > >    107 | struct mount_attr {
> > >        |        ^~~~~~~~~~
> > > In file included from /usr/include/x86_64-linux-gnu/sys/mount.h:32,
> > >                   from mount_setattr_test.c:10:
> > > .../usr/include/linux/mount.h:129:8: note: originally defined here
> > >    129 | struct mount_attr {
> > >        |        ^~~~~~~~~~
> > > make: *** [../lib.mk:145: .../tools/testing/selftests/mount_setattr/mount_setattr_test] Error 1
> > > 
> > > Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> > > ---
> > >   tools/testing/selftests/mount_setattr/mount_setattr_test.c | 7 -------
> > >   1 file changed, 7 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/mount_setattr/mount_setattr_test.c b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
> > > index 8c5fea68ae67..582669ca38e9 100644
> > > --- a/tools/testing/selftests/mount_setattr/mount_setattr_test.c
> > > +++ b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
> > > @@ -103,13 +103,6 @@
> > >   	#else
> > >   		#define __NR_mount_setattr 442
> > >   	#endif
> > > -
> > > -struct mount_attr {
> > > -	__u64 attr_set;
> > > -	__u64 attr_clr;
> > > -	__u64 propagation;
> > > -	__u64 userns_fd;
> > > -};
> > >   #endif
> > 
> > The difficulty with this is that whether or not you need this definition
> > depends on your system headers. My laptop doesn't have definitions for
> > either __NR_mount_setattr or struct mount_attr, so for me the build
> > works without this patch but fails with it.
> > 
> 
> The header search looks up system headers followed by installed headers in
> the repo (both in-tree and out-of-tree builds). kselftest builds do depend
> on headers_install. Did you building after running headers_install?

I wasn't aware they depend on headers_install. Why doesn't
Documentation/dev-tools/kselftest.rst mention this in the section that
describes how to run tests?

It seems what I really need to fix the build is to include
linux/mount.h, which works for me with or without headers_install,
because I have the struct in /usr/include/linux/mount.h. And I suppose
the makefile should use KHDR_INCLUDES. So maybe the changes below should
also be included.

However I know Christian has said that there are challenges with
including the mount headers. He wrote this test, so I'd like to hear his
thoughts about adding the include. He's on vacation this week though.

> > I suppose we could fix this universally by using a different name for
> > the struct in the test, e.g.:
> > 
> 
> This is not a good way to for a couple of reasons. This masks any problems
> due to incompatibility between these defines.

Agreed that this isn't ideal.

Thanks,
Seth


diff --git a/tools/testing/selftests/mount_setattr/Makefile b/tools/testing/selftests/mount_setattr/Makefile
index 2250f7dcb81e..fde72df01b11 100644
--- a/tools/testing/selftests/mount_setattr/Makefile
+++ b/tools/testing/selftests/mount_setattr/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for mount selftests.
-CFLAGS = -g -I../../../../usr/include/ -Wall -O2 -pthread
+CFLAGS = -g $(KHDR_INCLUDES) -Wall -O2 -pthread
 
 TEST_GEN_FILES += mount_setattr_test
 
diff --git a/tools/testing/selftests/mount_setattr/mount_setattr_test.c b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
index 8c5fea68ae67..969647228817 100644
--- a/tools/testing/selftests/mount_setattr/mount_setattr_test.c
+++ b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
@@ -18,6 +18,7 @@
 #include <grp.h>
 #include <stdbool.h>
 #include <stdarg.h>
+#include <linux/mount.h>
 
 #include "../kselftest_harness.h"
 


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

* Re: [PATCH] selftests/mount_setattr: fix redefine struct mount_attr build error
  2023-02-14 20:45     ` Seth Forshee
@ 2023-02-14 23:37       ` Shuah Khan
  2023-03-13  9:47         ` Christian Brauner
  0 siblings, 1 reply; 6+ messages in thread
From: Shuah Khan @ 2023-02-14 23:37 UTC (permalink / raw)
  To: Seth Forshee
  Cc: shuah, brauner, linux-fsdevel, linux-kselftest, linux-kernel, Shuah Khan

On 2/14/23 13:45, Seth Forshee wrote:
> On Tue, Feb 14, 2023 at 10:10:00AM -0700, Shuah Khan wrote:

>>>
>>
>> The header search looks up system headers followed by installed headers in
>> the repo (both in-tree and out-of-tree builds). kselftest builds do depend
>> on headers_install. Did you building after running headers_install?
> 
> I wasn't aware they depend on headers_install. Why doesn't
> Documentation/dev-tools/kselftest.rst mention this in the section that
> describes how to run tests?
> 

It ahs always been a dependency. If you were to compile from the
main (root) Makefile as below - headers_install get done before
test compile:

make kselftest-all TARGETS=mount_setattr

> It seems what I really need to fix the build is to include
> linux/mount.h, which works for me with or without headers_install,
> because I have the struct in /usr/include/linux/mount.h. And I suppose
> the makefile should use KHDR_INCLUDES. So maybe the changes below should
> also be included.
> 

Yes. Makefile change to use KHDR_INCLUDES is already done. Please
take a look at linux-kselftest next - this was done as part of a
tree-wide change.

If including linux/mount.h is thr correct solution, please send me
the patch on top of linux-kselftest next and I will pull it in.
  
> However I know Christian has said that there are challenges with
> including the mount headers. He wrote this test, so I'd like to hear his
> thoughts about adding the include. He's on vacation this week though.
> 

Sounds good.

thanks,
-- Shuah


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

* Re: [PATCH] selftests/mount_setattr: fix redefine struct mount_attr build error
  2023-02-14 23:37       ` Shuah Khan
@ 2023-03-13  9:47         ` Christian Brauner
  0 siblings, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2023-03-13  9:47 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Seth Forshee, shuah, linux-fsdevel, linux-kselftest, linux-kernel

On Tue, Feb 14, 2023 at 04:37:05PM -0700, Shuah Khan wrote:
> On 2/14/23 13:45, Seth Forshee wrote:
> > On Tue, Feb 14, 2023 at 10:10:00AM -0700, Shuah Khan wrote:
> 
> > > > 
> > > 
> > > The header search looks up system headers followed by installed headers in
> > > the repo (both in-tree and out-of-tree builds). kselftest builds do depend
> > > on headers_install. Did you building after running headers_install?
> > 
> > I wasn't aware they depend on headers_install. Why doesn't
> > Documentation/dev-tools/kselftest.rst mention this in the section that
> > describes how to run tests?
> > 
> 
> It ahs always been a dependency. If you were to compile from the
> main (root) Makefile as below - headers_install get done before
> test compile:
> 
> make kselftest-all TARGETS=mount_setattr
> 
> > It seems what I really need to fix the build is to include
> > linux/mount.h, which works for me with or without headers_install,
> > because I have the struct in /usr/include/linux/mount.h. And I suppose
> > the makefile should use KHDR_INCLUDES. So maybe the changes below should
> > also be included.
> > 
> 
> Yes. Makefile change to use KHDR_INCLUDES is already done. Please
> take a look at linux-kselftest next - this was done as part of a
> tree-wide change.
> 
> If including linux/mount.h is thr correct solution, please send me
> the patch on top of linux-kselftest next and I will pull it in.
> > However I know Christian has said that there are challenges with
> > including the mount headers. He wrote this test, so I'd like to hear his
> > thoughts about adding the include. He's on vacation this week though.

The problem is that the linux/mount.h and sys/mount.h headers may
conflict depending on the libc version used. So if linux/mount.h is
included care needs to be taken that no headers are included that
implicitly pull in sys/mount.h and vica versa. But if that isn't a
problem in this test and it solves the issue we can just include it.

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

end of thread, other threads:[~2023-03-13  9:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-13 18:31 [PATCH] selftests/mount_setattr: fix redefine struct mount_attr build error Shuah Khan
2023-02-13 23:50 ` Seth Forshee
2023-02-14 17:10   ` Shuah Khan
2023-02-14 20:45     ` Seth Forshee
2023-02-14 23:37       ` Shuah Khan
2023-03-13  9:47         ` Christian Brauner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.