linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>
To: Christoph Hellwig <hch@infradead.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	 Luis Chamberlain <mcgrof@kernel.org>,
	Kees Cook <kees@kernel.org>,
	 Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	linux-mm@kvack.org,  linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 06/15] fs,fork,exit: export symbols necessary for KUnit UAPI support
Date: Wed, 16 Jul 2025 10:39:57 +0200	[thread overview]
Message-ID: <20250716101207-c4201cef-abbe-481d-bca5-c2b27f324506@linutronix.de> (raw)
In-Reply-To: <aHdE9zgr_vjQlpwH@infradead.org>

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


  reply	other threads:[~2025-07-16  8:40 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-26  6:10 [PATCH v4 00/15] kunit: Introduce UAPI testing framework Thomas Weißschuh
2025-06-26  6:10 ` [PATCH v4 06/15] fs,fork,exit: export symbols necessary for KUnit UAPI support Thomas Weißschuh
2025-07-11 10:35   ` Thomas Weißschuh
2025-07-11 15:44     ` Al Viro
2025-07-14  5:52       ` Thomas Weißschuh
2025-07-14  8:12         ` Christian Brauner
2025-07-16  5:30         ` Thomas Weißschuh
2025-07-16  6:21           ` Christoph Hellwig
2025-07-16  8:39             ` Thomas Weißschuh [this message]
2025-07-16 11:11               ` Christoph Hellwig
2025-07-16 11:33                 ` Thomas Weißschuh
2025-07-16 11:36                   ` Christoph Hellwig
2025-07-16 12:47                     ` Thomas Weißschuh
2025-07-16 12:57                       ` Christoph Hellwig
2025-06-26  6:10 ` [PATCH v4 12/15] kunit: Introduce UAPI testing framework Thomas Weißschuh
2025-06-26 18:11   ` Benjamin Berg
2025-06-27  4:20     ` Thomas Weißschuh
2025-06-27  6:58       ` Benjamin Berg
2025-06-27  8:27         ` Thomas Weißschuh
2025-07-07 18:18 ` [PATCH v4 00/15] " Jonathan Corbet
2025-07-08  5:51   ` Thomas Weißschuh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250716101207-c4201cef-abbe-481d-bca5-c2b27f324506@linutronix.de \
    --to=thomas.weissschuh@linutronix.de \
    --cc=brauner@kernel.org \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=kees@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mcgrof@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).