[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250716101207-c4201cef-abbe-481d-bca5-c2b27f324506@linutronix.de>
Date: Wed, 16 Jul 2025 10:39:57 +0200
From: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
To: Christoph Hellwig <hch@...radead.org>
Cc: Al Viro <viro@...iv.linux.org.uk>,
Luis Chamberlain <mcgrof@...nel.org>, Kees Cook <kees@...nel.org>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, linux-mm@...ck.org,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 06/15] fs,fork,exit: export symbols necessary for
KUnit UAPI support
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
Powered by blists - more mailing lists