Re: [PATCH v4 06/15] fs,fork,exit: export symbols necessary for KUnit UAPI support

[Thomas Weißschuh]

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