Re: [PATCH] RFC: ksmbd: provide MODULE_VERSION()

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Sat, May 31, 2025 at 08:42:44AM +0200, Linus Walleij wrote:
> Adding MODULE_VERSION("1.0") to ksmbd makes /sys/modules/ksmbd
> appear even when ksmbd is compiled into the kernel.
> 
> Adding a version as a way to get a module name is documented in
> Documentation/ABI/stable/sysfs-module.

MODULE_VERSIONS() mean nothing and should be removed entirely from the
kernel tree.  The only "version" that is an issue is the kernel version.

>  #define KSMBD_GENL_NAME		"SMBD_GENL"
>  #define KSMBD_GENL_VERSION		0x01
> +#define KSMBD_GENL_VERSION_STRING	"1.0"

So you are overloading the module version attribute to somehow reflect
a user/kernel api version number instead?  That feels wrong and not a
good idea.  Export that "version" somewhere else please if userspace
really needs it (and I would strongly argue that it should NOT need it
as you aren't allowed to make breaking user/kernel api changes anyway,
so why would a version number matter?)

> --- a/fs/smb/server/server.c
> +++ b/fs/smb/server/server.c
> @@ -632,5 +632,7 @@ MODULE_SOFTDEP("pre: aead2");
>  MODULE_SOFTDEP("pre: ccm");
>  MODULE_SOFTDEP("pre: gcm");
>  MODULE_SOFTDEP("pre: crc32");
> +/* MODULE_VERSION() Makes /sys/module/ksmbd appear when compiled-in */
> +MODULE_VERSION(KSMBD_GENL_VERSION_STRING);

Again, no, please don't do this.

greg k-h