linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] RFC: ksmbd: Create module_kobject if builtin
@ 2025-10-23 22:49 Linus Walleij
  2025-10-24  5:08 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Linus Walleij @ 2025-10-23 22:49 UTC (permalink / raw)
  To: Namjae Jeon, Steve French, Sergey Senozhatsky, Tom Talpey
  Cc: linux-cifs, Greg Kroah-Hartman, Rosen Penev, Linus Walleij

Adding a call to lookup_or_create_module_kobject() to ksmbd
makes /sys/modules/ksmbd appear even when ksmbd is compiled
into the kernel.

This is nice when you boot a custom kernel on OpenWrt because
the startup script does things such as:

[ ! -e /sys/module/ksmbd ] && modprobe ksmbd 2> /dev/null
if [ ! -e /sys/module/ksmbd ]; then
    logger -p daemon.error -t 'ksmbd' "modprobe of ksmbd "
    "module failed, can\'t start ksmbd!"
     exit 1
fi

which makes the script not work with a compiled-in ksmbd.

Since I actually turn off modules and compile all my modules
into the kernel, I can't change the script to just check
cat /lib/modules/$(uname -r)/modules.builtin | grep ksmbd
either: no /lib/modules directory.

An option would be to change the script to proceed and
just assume the module is compiled in but it feels wrong.

If this approach is acceptable I am happy to generalize this
to something that any module that wants a /sys/modules/foo
file can use to get just that.

I can think of other ways to just create a dummy dir in
/sys/modules but it is probably unwise if someone would later
actually add a parameter or version string to the module and
get unpredictable bugs. It's probably better like this if
we do this thing.

Cc: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Changes in v2:
- Adding MODULE_VERSION() just to get /sys/module/ksmbd was wrong.
- Try this other approach to call lookup_or_create_module_kobject()
  if and only if ksmbd is compiled in.
- Link to v1: https://lore.kernel.org/r/20250531-ksmbd-sysfs-module-v1-1-248cf10fa87d@linaro.org
---
 fs/smb/server/server.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/fs/smb/server/server.c b/fs/smb/server/server.c
index 40420544cc25a2f7767695641e85d126022a30f9..f12ec149899664efdc9587db749dc8a1d6ccfdae 100644
--- a/fs/smb/server/server.c
+++ b/fs/smb/server/server.c
@@ -588,8 +588,24 @@ static int __init ksmbd_server_init(void)
 	if (ret)
 		goto err_crypto_destroy;
 
+	if (IS_BUILTIN(CONFIG_SMB_SERVER)) {
+		static struct module_kobject *mk;
+
+		/*
+		 * All this does is assure that /sys/module/ksmbd comes
+		 * into existence if ksmbd is compiled in so that userspace
+		 * can rely on its existence to identify that ksmbd is
+		 * available.
+		 */
+		mk = lookup_or_create_module_kobject(KBUILD_MODNAME);
+		if (!mk)
+			goto err_workqueue_destroy;
+	}
+
 	return 0;
 
+err_workqueue_destroy:
+	ksmbd_workqueue_destroy();
 err_crypto_destroy:
 	ksmbd_crypto_destroy();
 err_release_inode_hash:

---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20250531-ksmbd-sysfs-module-efaf2d1a86ca

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>


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

* Re: [PATCH v2] RFC: ksmbd: Create module_kobject if builtin
  2025-10-23 22:49 [PATCH v2] RFC: ksmbd: Create module_kobject if builtin Linus Walleij
@ 2025-10-24  5:08 ` Greg Kroah-Hartman
  2025-10-24  6:05   ` Linus Walleij
  0 siblings, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2025-10-24  5:08 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Namjae Jeon, Steve French, Sergey Senozhatsky, Tom Talpey,
	linux-cifs, Rosen Penev

On Fri, Oct 24, 2025 at 12:49:44AM +0200, Linus Walleij wrote:
> Adding a call to lookup_or_create_module_kobject() to ksmbd
> makes /sys/modules/ksmbd appear even when ksmbd is compiled
> into the kernel.

Ick, no, please do not.  Otherwise you will see this pop up in all
drivers/modules for the same "reason".

> This is nice when you boot a custom kernel on OpenWrt because
> the startup script does things such as:
> 
> [ ! -e /sys/module/ksmbd ] && modprobe ksmbd 2> /dev/null
> if [ ! -e /sys/module/ksmbd ]; then
>     logger -p daemon.error -t 'ksmbd' "modprobe of ksmbd "
>     "module failed, can\'t start ksmbd!"
>      exit 1
> fi
> 
> which makes the script not work with a compiled-in ksmbd.

Please fix the start up script to not do this.  Don't work around broken
userspace by changing the kernel.

> Since I actually turn off modules and compile all my modules
> into the kernel, I can't change the script to just check
> cat /lib/modules/$(uname -r)/modules.builtin | grep ksmbd
> either: no /lib/modules directory.

Then do something else to determine if ksmbd is in the running kernel,
don't rely on a modules sysfs file like that.

> An option would be to change the script to proceed and
> just assume the module is compiled in but it feels wrong.

Agreed, surely there is some other way to "know" if that kernel feature
is present or not, right?  Mount a filesystem?  Something else?

> If this approach is acceptable I am happy to generalize this
> to something that any module that wants a /sys/modules/foo
> file can use to get just that.

No, please do not do that.  Just fix userspace.

thanks,

greg k-h

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

* Re: [PATCH v2] RFC: ksmbd: Create module_kobject if builtin
  2025-10-24  5:08 ` Greg Kroah-Hartman
@ 2025-10-24  6:05   ` Linus Walleij
  2025-10-24  6:37     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Linus Walleij @ 2025-10-24  6:05 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Namjae Jeon, Steve French, Sergey Senozhatsky, Tom Talpey,
	linux-cifs, Rosen Penev

On Fri, Oct 24, 2025 at 7:08 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:

> > Since I actually turn off modules and compile all my modules
> > into the kernel, I can't change the script to just check
> > cat /lib/modules/$(uname -r)/modules.builtin | grep ksmbd
> > either: no /lib/modules directory.
>
> Then do something else to determine if ksmbd is in the running kernel,
> don't rely on a modules sysfs file like that.

There really isn't a good way for a script to detect if a module is
compiled-in that I know of.

You did suggest simply create this file also for compiled-in
module, always:
https://lore.kernel.org/all/2025060254-unscathed-junior-1b43@gregkh/

It just seemed a bit cluttery, but maybe the $subject approach
is even more cluttery by creating special cases.

Yours,
Linus Walleij

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

* Re: [PATCH v2] RFC: ksmbd: Create module_kobject if builtin
  2025-10-24  6:05   ` Linus Walleij
@ 2025-10-24  6:37     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2025-10-24  6:37 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Namjae Jeon, Steve French, Sergey Senozhatsky, Tom Talpey,
	linux-cifs, Rosen Penev

On Fri, Oct 24, 2025 at 08:05:29AM +0200, Linus Walleij wrote:
> On Fri, Oct 24, 2025 at 7:08 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> 
> > > Since I actually turn off modules and compile all my modules
> > > into the kernel, I can't change the script to just check
> > > cat /lib/modules/$(uname -r)/modules.builtin | grep ksmbd
> > > either: no /lib/modules directory.
> >
> > Then do something else to determine if ksmbd is in the running kernel,
> > don't rely on a modules sysfs file like that.
> 
> There really isn't a good way for a script to detect if a module is
> compiled-in that I know of.

That should be a per-feature type of thing.  Surely ksmbd has some sort
of user/kernel api that can be tested if it is present or not?

> You did suggest simply create this file also for compiled-in
> module, always:
> https://lore.kernel.org/all/2025060254-unscathed-junior-1b43@gregkh/
> 
> It just seemed a bit cluttery, but maybe the $subject approach
> is even more cluttery by creating special cases.

Yes, let's not special-case this, either make it so that every module
name shows up in sysfs (if it has a parameter or not), or keep it as-is.
Don't do module-specific hacks like you are proposing here.

thanks,

greg k-h

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

end of thread, other threads:[~2025-10-24  6:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-23 22:49 [PATCH v2] RFC: ksmbd: Create module_kobject if builtin Linus Walleij
2025-10-24  5:08 ` Greg Kroah-Hartman
2025-10-24  6:05   ` Linus Walleij
2025-10-24  6:37     ` Greg Kroah-Hartman

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).