The KUnit UAPI infrastructure starts userspace processes.
As it should be able to be built as a module, export the necessary symbols.

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

---
To: Kees Cook <kees@kernel.org>
To: Alexander Viro <viro@zeniv.linux.org.uk>
To: Christian Brauner <brauner@kernel.org>
To: Jan Kara <jack@suse.cz>
Cc: linux-mm@kvack.org
Cc: linux-fsdevel@vger.kernel.org

./get_maintainer.pl would have also Cc-ed all of the memory management and
scheduler maintainers. I trimmed the list to only BINFMT/EXEC and VFS.
---
 fs/exec.c        | 2 ++
 fs/file.c        | 1 +
 fs/filesystems.c | 2 ++
 fs/fs_struct.c   | 1 +
 fs/pipe.c        | 2 ++
 kernel/exit.c    | 3 +++
 kernel/fork.c    | 2 ++
 7 files changed, 13 insertions(+)

diff --git a/fs/exec.c b/fs/exec.c
index 1f5fdd2e096e392b342f122d35aba4cf035441c7..13f7f27641942eddcb179bdd93d99b799d155813 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -68,6 +68,7 @@
 #include <linux/user_events.h>
 #include <linux/rseq.h>
 #include <linux/ksm.h>
+#include <linux/export.h>
 
 #include <linux/uaccess.h>
 #include <asm/mmu_context.h>
@@ -1919,6 +1920,7 @@ int kernel_execve(const char *kernel_filename,
 	putname(filename);
 	return retval;
 }
+EXPORT_SYMBOL_GPL_FOR_MODULES(kernel_execve, "kunit-uapi");
 
 static int do_execve(struct filename *filename,
 	const char __user *const __user *__argv,
diff --git a/fs/file.c b/fs/file.c
index 3a3146664cf37115624e12f7f06826d48827e9d7..89d07feb9c328337451ce40cb0f368b6cb986c2c 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -1317,6 +1317,7 @@ int replace_fd(unsigned fd, struct file *file, unsigned flags)
 	spin_unlock(&files->file_lock);
 	return err;
 }
+EXPORT_SYMBOL_GPL_FOR_MODULES(replace_fd, "kunit-uapi");
 
 /**
  * receive_fd() - Install received file into file descriptor table
diff --git a/fs/filesystems.c b/fs/filesystems.c
index 95e5256821a53494d88f496193305a2e50e04444..a3a588f387bbd8268246d1026389deaadf265d0b 100644
--- a/fs/filesystems.c
+++ b/fs/filesystems.c
@@ -17,6 +17,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/fs_parser.h>
+#include <linux/export.h>
 
 /*
  * Handling of filesystem drivers list.
@@ -45,6 +46,7 @@ void put_filesystem(struct file_system_type *fs)
 {
 	module_put(fs->owner);
 }
+EXPORT_SYMBOL_GPL_FOR_MODULES(put_filesystem, "kunit-uapi");
 
 static struct file_system_type **find_filesystem(const char *name, unsigned len)
 {
diff --git a/fs/fs_struct.c b/fs/fs_struct.c
index 64c2d0814ed6889cc12603410e6e9dc44089586f..26340d225deba3f2ec30252293fdf417235a6a4a 100644
--- a/fs/fs_struct.c
+++ b/fs/fs_struct.c
@@ -46,6 +46,7 @@ void set_fs_pwd(struct fs_struct *fs, const struct path *path)
 	if (old_pwd.dentry)
 		path_put(&old_pwd);
 }
+EXPORT_SYMBOL_GPL_FOR_MODULES(set_fs_pwd, "kunit-uapi");
 
 static inline int replace_path(struct path *p, const struct path *old, const struct path *new)
 {
diff --git a/fs/pipe.c b/fs/pipe.c
index 45077c37bad154ef146b047834d35d489fcc4d8d..d6cb743d2cfc041f08b498a5a764e9a96dc34069 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -27,6 +27,7 @@
 #include <linux/watch_queue.h>
 #include <linux/sysctl.h>
 #include <linux/sort.h>
+#include <linux/export.h>
 
 #include <linux/uaccess.h>
 #include <asm/ioctls.h>
@@ -971,6 +972,7 @@ int create_pipe_files(struct file **res, int flags)
 	file_set_fsnotify_mode(res[1], FMODE_NONOTIFY_PERM);
 	return 0;
 }
+EXPORT_SYMBOL_GPL_FOR_MODULES(create_pipe_files, "kunit-uapi");
 
 static int __do_pipe_flags(int *fd, struct file **files, int flags)
 {
diff --git a/kernel/exit.c b/kernel/exit.c
index bd743900354ca5fc6c550f80e30393a632eb9a4e..610dffb1276ac60b475708587ca053f315fea9c3 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -70,6 +70,7 @@
 #include <linux/user_events.h>
 #include <linux/uaccess.h>
 #include <linux/pidfs.h>
+#include <linux/export.h>
 
 #include <uapi/linux/wait.h>
 
@@ -1005,6 +1006,7 @@ void __noreturn do_exit(long code)
 	lockdep_free_task(tsk);
 	do_task_dead();
 }
+EXPORT_SYMBOL_GPL_FOR_MODULES(do_exit, "kunit-uapi");
 
 void __noreturn make_task_dead(int signr)
 {
@@ -1887,6 +1889,7 @@ int kernel_wait(pid_t pid, int *stat)
 	put_pid(wo.wo_pid);
 	return ret;
 }
+EXPORT_SYMBOL_GPL_FOR_MODULES(kernel_wait, "kunit-uapi");
 
 SYSCALL_DEFINE4(wait4, pid_t, upid, int __user *, stat_addr,
 		int, options, struct rusage __user *, ru)
diff --git a/kernel/fork.c b/kernel/fork.c
index 1ee8eb11f38bae1d2eb6de9494aea94b7a19e6c3..5de7a9bc005ade6dcfbdfe1a63cadbef8782658c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -105,6 +105,7 @@
 #include <uapi/linux/pidfd.h>
 #include <linux/pidfs.h>
 #include <linux/tick.h>
+#include <linux/export.h>
 
 #include <asm/pgalloc.h>
 #include <linux/uaccess.h>
@@ -2676,6 +2677,7 @@ pid_t user_mode_thread(int (*fn)(void *), void *arg, unsigned long flags)
 
 	return kernel_clone(&args);
 }
+EXPORT_SYMBOL_GPL_FOR_MODULES(user_mode_thread, "kunit-uapi");
 
 #ifdef __ARCH_WANT_SYS_FORK
 SYSCALL_DEFINE0(fork)

-- 
2.50.0

Hi Kees, Al, Christian and Honza,

On Thu, Jun 26, 2025 at 08:10:14AM +0200, Thomas Weißschuh wrote:
> The KUnit UAPI infrastructure starts userspace processes.
> As it should be able to be built as a module, export the necessary symbols.

could you take a look at these new symbol exports?

Thanks,
Thomas

> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> 
> ---
> To: Kees Cook <kees@kernel.org>
> To: Alexander Viro <viro@zeniv.linux.org.uk>
> To: Christian Brauner <brauner@kernel.org>
> To: Jan Kara <jack@suse.cz>
> Cc: linux-mm@kvack.org
> Cc: linux-fsdevel@vger.kernel.org
> 
> ./get_maintainer.pl would have also Cc-ed all of the memory management and
> scheduler maintainers. I trimmed the list to only BINFMT/EXEC and VFS.
> ---
>  fs/exec.c        | 2 ++
>  fs/file.c        | 1 +
>  fs/filesystems.c | 2 ++
>  fs/fs_struct.c   | 1 +
>  fs/pipe.c        | 2 ++
>  kernel/exit.c    | 3 +++
>  kernel/fork.c    | 2 ++
>  7 files changed, 13 insertions(+)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index 1f5fdd2e096e392b342f122d35aba4cf035441c7..13f7f27641942eddcb179bdd93d99b799d155813 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -68,6 +68,7 @@
>  #include <linux/user_events.h>
>  #include <linux/rseq.h>
>  #include <linux/ksm.h>
> +#include <linux/export.h>
>  
>  #include <linux/uaccess.h>
>  #include <asm/mmu_context.h>
> @@ -1919,6 +1920,7 @@ int kernel_execve(const char *kernel_filename,
>  	putname(filename);
>  	return retval;
>  }
> +EXPORT_SYMBOL_GPL_FOR_MODULES(kernel_execve, "kunit-uapi");
>  
>  static int do_execve(struct filename *filename,
>  	const char __user *const __user *__argv,
> diff --git a/fs/file.c b/fs/file.c
> index 3a3146664cf37115624e12f7f06826d48827e9d7..89d07feb9c328337451ce40cb0f368b6cb986c2c 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -1317,6 +1317,7 @@ int replace_fd(unsigned fd, struct file *file, unsigned flags)
>  	spin_unlock(&files->file_lock);
>  	return err;
>  }
> +EXPORT_SYMBOL_GPL_FOR_MODULES(replace_fd, "kunit-uapi");
>  
>  /**
>   * receive_fd() - Install received file into file descriptor table
> diff --git a/fs/filesystems.c b/fs/filesystems.c
> index 95e5256821a53494d88f496193305a2e50e04444..a3a588f387bbd8268246d1026389deaadf265d0b 100644
> --- a/fs/filesystems.c
> +++ b/fs/filesystems.c
> @@ -17,6 +17,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/fs_parser.h>
> +#include <linux/export.h>
>  
>  /*
>   * Handling of filesystem drivers list.
> @@ -45,6 +46,7 @@ void put_filesystem(struct file_system_type *fs)
>  {
>  	module_put(fs->owner);
>  }
> +EXPORT_SYMBOL_GPL_FOR_MODULES(put_filesystem, "kunit-uapi");
>  
>  static struct file_system_type **find_filesystem(const char *name, unsigned len)
>  {
> diff --git a/fs/fs_struct.c b/fs/fs_struct.c
> index 64c2d0814ed6889cc12603410e6e9dc44089586f..26340d225deba3f2ec30252293fdf417235a6a4a 100644
> --- a/fs/fs_struct.c
> +++ b/fs/fs_struct.c
> @@ -46,6 +46,7 @@ void set_fs_pwd(struct fs_struct *fs, const struct path *path)
>  	if (old_pwd.dentry)
>  		path_put(&old_pwd);
>  }
> +EXPORT_SYMBOL_GPL_FOR_MODULES(set_fs_pwd, "kunit-uapi");
>  
>  static inline int replace_path(struct path *p, const struct path *old, const struct path *new)
>  {
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 45077c37bad154ef146b047834d35d489fcc4d8d..d6cb743d2cfc041f08b498a5a764e9a96dc34069 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -27,6 +27,7 @@
>  #include <linux/watch_queue.h>
>  #include <linux/sysctl.h>
>  #include <linux/sort.h>
> +#include <linux/export.h>
>  
>  #include <linux/uaccess.h>
>  #include <asm/ioctls.h>
> @@ -971,6 +972,7 @@ int create_pipe_files(struct file **res, int flags)
>  	file_set_fsnotify_mode(res[1], FMODE_NONOTIFY_PERM);
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL_FOR_MODULES(create_pipe_files, "kunit-uapi");
>  
>  static int __do_pipe_flags(int *fd, struct file **files, int flags)
>  {
> diff --git a/kernel/exit.c b/kernel/exit.c
> index bd743900354ca5fc6c550f80e30393a632eb9a4e..610dffb1276ac60b475708587ca053f315fea9c3 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -70,6 +70,7 @@
>  #include <linux/user_events.h>
>  #include <linux/uaccess.h>
>  #include <linux/pidfs.h>
> +#include <linux/export.h>
>  
>  #include <uapi/linux/wait.h>
>  
> @@ -1005,6 +1006,7 @@ void __noreturn do_exit(long code)
>  	lockdep_free_task(tsk);
>  	do_task_dead();
>  }
> +EXPORT_SYMBOL_GPL_FOR_MODULES(do_exit, "kunit-uapi");
>  
>  void __noreturn make_task_dead(int signr)
>  {
> @@ -1887,6 +1889,7 @@ int kernel_wait(pid_t pid, int *stat)
>  	put_pid(wo.wo_pid);
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL_FOR_MODULES(kernel_wait, "kunit-uapi");
>  
>  SYSCALL_DEFINE4(wait4, pid_t, upid, int __user *, stat_addr,
>  		int, options, struct rusage __user *, ru)
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 1ee8eb11f38bae1d2eb6de9494aea94b7a19e6c3..5de7a9bc005ade6dcfbdfe1a63cadbef8782658c 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -105,6 +105,7 @@
>  #include <uapi/linux/pidfd.h>
>  #include <linux/pidfs.h>
>  #include <linux/tick.h>
> +#include <linux/export.h>
>  
>  #include <asm/pgalloc.h>
>  #include <linux/uaccess.h>
> @@ -2676,6 +2677,7 @@ pid_t user_mode_thread(int (*fn)(void *), void *arg, unsigned long flags)
>  
>  	return kernel_clone(&args);
>  }
> +EXPORT_SYMBOL_GPL_FOR_MODULES(user_mode_thread, "kunit-uapi");
>  
>  #ifdef __ARCH_WANT_SYS_FORK
>  SYSCALL_DEFINE0(fork)
> 
> -- 
> 2.50.0
> 

On Fri, Jul 11, 2025 at 12:35:59PM +0200, Thomas Weißschuh wrote:
> Hi Kees, Al, Christian and Honza,
> 
> On Thu, Jun 26, 2025 at 08:10:14AM +0200, Thomas Weißschuh wrote:
> > The KUnit UAPI infrastructure starts userspace processes.
> > As it should be able to be built as a module, export the necessary symbols.

What's wrong with kernel/umh.c?

> could you take a look at these new symbol exports?

> > +EXPORT_SYMBOL_GPL_FOR_MODULES(put_filesystem, "kunit-uapi");

What's that one for???

(+Luis for the usermode helper discussion)

On Fri, Jul 11, 2025 at 04:44:23PM +0100, Al Viro wrote:
> On Fri, Jul 11, 2025 at 12:35:59PM +0200, Thomas Weißschuh wrote:
> > Hi Kees, Al, Christian and Honza,
> > 
> > On Thu, Jun 26, 2025 at 08:10:14AM +0200, Thomas Weißschuh wrote:
> > > The KUnit UAPI infrastructure starts userspace processes.
> > > As it should be able to be built as a module, export the necessary symbols.
> 
> What's wrong with kernel/umh.c?

It gets neutered by CONFIG_STATIC_USERMODEHELPER_PATH. That could be worked
around be overriding sub_info->path, but it would be a hack.
It does not allow to implement a custom wait routine to forward the process
output to KUnit as implemented in kunit_uapi_forward_to_printk() [0].
That may be solved by adding another thread, but that would also be hacky.

It would probably be possible to extend kernel/umh.c for my usecase but I
didn't want bloat the core kernel code for my test-only functionality.

> > could you take a look at these new symbol exports?
> 
> > > +EXPORT_SYMBOL_GPL_FOR_MODULES(put_filesystem, "kunit-uapi");
> 
> What's that one for???

What are you referring to?

The macro EXPORT_SYMBOL_GPL_FOR_MODULES() will only export the symbol for one
specific module. Personally I'm also fine with EXPORT_SYMBOL_GPL().

"kunit-uapi" is a new module I am implementing in this patchset. It allows to
run userspace executables as part of KUnit.
Some more information in the cover-letter [1] of the series and the code using
these symbols[0]. Both should also be in your inbox.
There is also an article on LWN [2].

[0] https://lore.kernel.org/lkml/20250626-kunit-kselftests-v4-12-48760534fef5@linutronix.de/
[1] https://lore.kernel.org/lkml/20250626-kunit-kselftests-v4-0-48760534fef5@linutronix.de/
[2] https://lwn.net/SubscriberLink/1029077/fa55c3b2d238a6bb/


Thomas

On Mon, Jul 14, 2025 at 07:52:28AM +0200, Thomas Weißschuh wrote:
> On Fri, Jul 11, 2025 at 04:44:23PM +0100, Al Viro wrote:

(...)

> > On Fri, Jul 11, 2025 at 12:35:59PM +0200, Thomas Weißschuh wrote:
> > > could you take a look at these new symbol exports?
> > 
> > > > +EXPORT_SYMBOL_GPL_FOR_MODULES(put_filesystem, "kunit-uapi");
> > 
> > What's that one for???
> 
> What are you referring to?

Reading this again you probably asked why put_filesystem() is exported.

As I see it, that should be called after being done with the return value of
get_fs_type(). Not that it does anything for the always built-in ramfs.
The alternatives I see are a commented-out variant with an explanation or
making put_filesystem() into an inline function.


Thomas

On Wed, Jul 16, 2025 at 07:30:46AM +0200, Thomas Weißschuh wrote:
> On Mon, Jul 14, 2025 at 07:52:28AM +0200, Thomas Weißschuh wrote:
> > On Fri, Jul 11, 2025 at 04:44:23PM +0100, Al Viro wrote:
> 
> (...)
> 
> > > On Fri, Jul 11, 2025 at 12:35:59PM +0200, Thomas Weißschuh wrote:
> > > > could you take a look at these new symbol exports?
> > > 
> > > > > +EXPORT_SYMBOL_GPL_FOR_MODULES(put_filesystem, "kunit-uapi");
> > > 
> > > What's that one for???
> > 
> > What are you referring to?
> 
> Reading this again you probably asked why put_filesystem() is exported.
> 
> As I see it, that should be called after being done with the return value of
> get_fs_type(). Not that it does anything for the always built-in ramfs.
> The alternatives I see are a commented-out variant with an explanation or
> making put_filesystem() into an inline function.

The right answer is to rework your code to not need all those exports.
Nothing modular, and especially not something testing only should need
all these low-level bits.

On Tue, Jul 15, 2025 at 11:21:43PM -0700, Christoph Hellwig wrote:
> On Wed, Jul 16, 2025 at 07:30:46AM +0200, Thomas Weißschuh wrote:
> > On Mon, Jul 14, 2025 at 07:52:28AM +0200, Thomas Weißschuh wrote:
> > > On Fri, Jul 11, 2025 at 04:44:23PM +0100, Al Viro wrote:
> > 
> > (...)
> > 
> > > > On Fri, Jul 11, 2025 at 12:35:59PM +0200, Thomas Weißschuh wrote:
> > > > > could you take a look at these new symbol exports?
> > > > 
> > > > > > +EXPORT_SYMBOL_GPL_FOR_MODULES(put_filesystem, "kunit-uapi");
> > > > 
> > > > What's that one for???
> > > 
> > > What are you referring to?
> > 
> > Reading this again you probably asked why put_filesystem() is exported.
> > 
> > As I see it, that should be called after being done with the return value of
> > get_fs_type(). Not that it does anything for the always built-in ramfs.
> > The alternatives I see are a commented-out variant with an explanation or
> > making put_filesystem() into an inline function.
> 
> The right answer is to rework your code to not need all those exports.

If I saw a way, I surely would do that and I certainly tried before.
For the first revisions of this series I didn't even try to make this code
modular to avoid these discussions.

> Nothing modular, and especially not something testing only should need
> all these low-level bits.

Let's take kernel_execve() as example, there is no way around using this
function in one way or another. It only has two existing callers.
init/main.c: It is completely unsuitable for this usecase.
kernel/umh.c: It is also what Al suggested and I am all for it.
Unfortunately it is missing features. Citation from my response to Al:

> It gets neutered by CONFIG_STATIC_USERMODEHELPER_PATH. That could be worked
> around be overriding sub_info->path, but it would be a hack.
> It does not allow to implement a custom wait routine to forward the process
> output to KUnit as implemented in kunit_uapi_forward_to_printk() [0].
> That may be solved by adding another thread, but that would also be hacky.

So I can either hack around the official API of umh.c, or modify it only to
cater to "something testing".

Then we have put_filesystem(), it is the counterpart to get_fs_type().
But while get_fs_type() is EXPORT_SYMBOL(), put_filesystem() is private.
I can leave out the call to put_filesystem(), the result would be the same
but it is still a hack.

create_pipe_files() and replace_fd() are used together with umh.c. But while
the umh.c API is EXPORT_SYMBOL_GPL(), they are not.

In general KUnit is already fairly integrated into the core kernel.
When built as a module it contains some built-in components and even has its
own member in 'struct task_struct'.
Having a built-in helper for my framework that wraps the calls to the
non-exported symbols into a helper function would work but again be hacky.

Or the code becomes non-modular again and suddenly nobody cares anymore...


Thomas

On Wed, Jul 16, 2025 at 10:39:57AM +0200, Thomas Weißschuh wrote:
> Let's take kernel_execve() as example, there is no way around using this
> function in one way or another. It only has two existing callers.
> init/main.c: It is completely unsuitable for this usecase.
> kernel/umh.c: It is also what Al suggested and I am all for it.
> Unfortunately it is missing features. Citation from my response to Al:

But why does the code that calls it need to be modular?  I get why
the actual test cases should be modular, but the core test runner is
small and needs a lot of kernel internals.  Just require it to be
built-in and all this mess goes away.

That being said some of this stuff, like get_fs_type / put_filesystem
or replace_fd seem like the wrong level of abstractions for something
running tests anyway.

On Wed, Jul 16, 2025 at 04:11:04AM -0700, Christoph Hellwig wrote:
> On Wed, Jul 16, 2025 at 10:39:57AM +0200, Thomas Weißschuh wrote:
> > Let's take kernel_execve() as example, there is no way around using this
> > function in one way or another. It only has two existing callers.
> > init/main.c: It is completely unsuitable for this usecase.
> > kernel/umh.c: It is also what Al suggested and I am all for it.
> > Unfortunately it is missing features. Citation from my response to Al:
> 
> But why does the code that calls it need to be modular?  I get why
> the actual test cases should be modular, but the core test runner is
> small and needs a lot of kernel internals.  Just require it to be
> built-in and all this mess goes away.

KUnit UAPI calls into KUnit proper which itself is modular.
As such it needs to be modular, too.
This also makes a small always-builtin component annoying as it will need
to call into KUnit through indirect calls.

But again:
I see that it makes sense to restrict random out-of-tree modules
from accessing kernel internals. But here the symbols are only exported for
one single, in-tree user. What is the advantage of forcing this user to be
non-modular to get access?

> That being said some of this stuff, like get_fs_type / put_filesystem
> or replace_fd seem like the wrong level of abstractions for something
> running tests anyway.

This was modelled after usermode helper and usermode driver.
To me it makes sense, and I don't see an obvious way to get rid of these.

Or do you mean to introduce a new in-core helper to abstract this away?
Then everybody would need to pay the cost for this helper even if it is only
used from some modular code.


Thomas

On Wed, Jul 16, 2025 at 01:33:05PM +0200, Thomas Weißschuh wrote:
> On Wed, Jul 16, 2025 at 04:11:04AM -0700, Christoph Hellwig wrote:
> > On Wed, Jul 16, 2025 at 10:39:57AM +0200, Thomas Weißschuh wrote:
> > > Let's take kernel_execve() as example, there is no way around using this
> > > function in one way or another. It only has two existing callers.
> > > init/main.c: It is completely unsuitable for this usecase.
> > > kernel/umh.c: It is also what Al suggested and I am all for it.
> > > Unfortunately it is missing features. Citation from my response to Al:
> > 
> > But why does the code that calls it need to be modular?  I get why
> > the actual test cases should be modular, but the core test runner is
> > small and needs a lot of kernel internals.  Just require it to be
> > built-in and all this mess goes away.
> 
> KUnit UAPI calls into KUnit proper which itself is modular.
> As such it needs to be modular, too.

Not if you depend on KUNIT=y.

> > That being said some of this stuff, like get_fs_type / put_filesystem
> > or replace_fd seem like the wrong level of abstractions for something
> > running tests anyway.
> 
> This was modelled after usermode helper and usermode driver.
> To me it makes sense, and I don't see an obvious way to get rid of these.
> 
> Or do you mean to introduce a new in-core helper to abstract this away?
> Then everybody would need to pay the cost for this helper even if it is only
> used from some modular code.

I have no idea what you are doing as you only Cc'ed the exports patch
but not the actual work to the mailing lists, so I have no way of
helping you with the actual code.  I can just tell you my gut feeling
based on the symbols, and they are something that doesn't feel outside
of very core code.

On Wed, Jul 16, 2025 at 04:36:38AM -0700, Christoph Hellwig wrote:
> On Wed, Jul 16, 2025 at 01:33:05PM +0200, Thomas Weißschuh wrote:
> > On Wed, Jul 16, 2025 at 04:11:04AM -0700, Christoph Hellwig wrote:
> > > On Wed, Jul 16, 2025 at 10:39:57AM +0200, Thomas Weißschuh wrote:
> > > > Let's take kernel_execve() as example, there is no way around using this
> > > > function in one way or another. It only has two existing callers.
> > > > init/main.c: It is completely unsuitable for this usecase.
> > > > kernel/umh.c: It is also what Al suggested and I am all for it.
> > > > Unfortunately it is missing features. Citation from my response to Al:
> > > 
> > > But why does the code that calls it need to be modular?  I get why
> > > the actual test cases should be modular, but the core test runner is
> > > small and needs a lot of kernel internals.  Just require it to be
> > > built-in and all this mess goes away.
> > 
> > KUnit UAPI calls into KUnit proper which itself is modular.
> > As such it needs to be modular, too.
> 
> Not if you depend on KUNIT=y.

This is exactly what I did in the beginning. Then I got told about the distros
using KUNIT=m [0] and decided that it does make sense to support.
We'd have this discussion sooner or later. But I'm still not sure what
difference an in-tree-module-specific export should make.

> > > That being said some of this stuff, like get_fs_type / put_filesystem
> > > or replace_fd seem like the wrong level of abstractions for something
> > > running tests anyway.
> > 
> > This was modelled after usermode helper and usermode driver.
> > To me it makes sense, and I don't see an obvious way to get rid of these.
> > 
> > Or do you mean to introduce a new in-core helper to abstract this away?
> > Then everybody would need to pay the cost for this helper even if it is only
> > used from some modular code.
> 
> I have no idea what you are doing as you only Cc'ed the exports patch
> but not the actual work to the mailing lists, so I have no way of
> helping you with the actual code.  I can just tell you my gut feeling
> based on the symbols, and they are something that doesn't feel outside
> of very core code.

The actual code using these exports [1] was Cc'ed to both linux-fsdevel and
linux-mm. In addition to the cover-letter and the exports patch.
The rest of the series does not interact with the exports at all.

[0] https://lore.kernel.org/all/CABVgOSmdcOZ0+-k=SM4LibOVMKtcbF27p6N40kuDX_axTPZ=QQ@mail.gmail.com/
[1] https://lore.kernel.org/lkml/20250626-kunit-kselftests-v4-12-48760534fef5@linutronix.de/


Thomas

On Wed, Jul 16, 2025 at 02:47:08PM +0200, Thomas Weißschuh wrote:
> This is exactly what I did in the beginning. Then I got told about the distros
> using KUNIT=m [0] and decided that it does make sense to support.

Well, not if you need to export tons of stuff for it.

> The actual code using these exports [1] was Cc'ed to both linux-fsdevel and
> linux-mm. In addition to the cover-letter and the exports patch.
> The rest of the series does not interact with the exports at all.


Incomplete series are not reviewable.  Don't even start arguing about
that, if you don't send series to all list they deserve to be ignored
in general.

On Mon, Jul 14, 2025 at 07:52:27AM +0200, Thomas Weißschuh wrote:
> (+Luis for the usermode helper discussion)
> 
> On Fri, Jul 11, 2025 at 04:44:23PM +0100, Al Viro wrote:
> > On Fri, Jul 11, 2025 at 12:35:59PM +0200, Thomas Weißschuh wrote:
> > > Hi Kees, Al, Christian and Honza,
> > > 
> > > On Thu, Jun 26, 2025 at 08:10:14AM +0200, Thomas Weißschuh wrote:
> > > > The KUnit UAPI infrastructure starts userspace processes.
> > > > As it should be able to be built as a module, export the necessary symbols.
> > 
> > What's wrong with kernel/umh.c?
> 
> It gets neutered by CONFIG_STATIC_USERMODEHELPER_PATH. That could be worked
> around be overriding sub_info->path, but it would be a hack.
> It does not allow to implement a custom wait routine to forward the process
> output to KUnit as implemented in kunit_uapi_forward_to_printk() [0].
> That may be solved by adding another thread, but that would also be hacky.
> 
> It would probably be possible to extend kernel/umh.c for my usecase but I
> didn't want bloat the core kernel code for my test-only functionality.
> 
> > > could you take a look at these new symbol exports?
> > 
> > > > +EXPORT_SYMBOL_GPL_FOR_MODULES(put_filesystem, "kunit-uapi");
> > 
> > What's that one for???
> 
> What are you referring to?
> 
> The macro EXPORT_SYMBOL_GPL_FOR_MODULES() will only export the symbol for one
> specific module. Personally I'm also fine with EXPORT_SYMBOL_GPL().

No, we're going to use the new restricted macros going forward that
limit exports to very specific modules. This is a good example. Though
it will be renamed to EXPORT_SYMBOL_FOR_MODULES() anyway.