On Mon, Sep 30, 2024 at 4:27 PM Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote:
>
> On Sun, Sep 29, 2024 at 08:21:03PM GMT, Alexander Mikhalitsyn wrote:
> >Add an explicit MODULE_VERSION("0.0.1") specification for the vhost_vsock module.
> >
> >It is useful because it allows userspace to check if vhost_vsock is there when it is
> >configured as a built-in.
> >
> >This is what we have *without* this change and when vhost_vsock is configured
> >as a module and loaded:
> >
> >$ ls -la /sys/module/vhost_vsock
> >total 0
> >drwxr-xr-x 5 root root 0 Sep 29 19:00 .
> >drwxr-xr-x 337 root root 0 Sep 29 18:59 ..
> >-r--r--r-- 1 root root 4096 Sep 29 20:05 coresize
> >drwxr-xr-x 2 root root 0 Sep 29 20:05 holders
> >-r--r--r-- 1 root root 4096 Sep 29 20:05 initsize
> >-r--r--r-- 1 root root 4096 Sep 29 20:05 initstate
> >drwxr-xr-x 2 root root 0 Sep 29 20:05 notes
> >-r--r--r-- 1 root root 4096 Sep 29 20:05 refcnt
> >drwxr-xr-x 2 root root 0 Sep 29 20:05 sections
> >-r--r--r-- 1 root root 4096 Sep 29 20:05 srcversion
> >-r--r--r-- 1 root root 4096 Sep 29 20:05 taint
> >--w------- 1 root root 4096 Sep 29 19:00 uevent
> >
> >When vhost_vsock is configured as a built-in there is *no* /sys/module/vhost_vsock directory at all.
> >And this looks like an inconsistency.
> >
> >With this change, when vhost_vsock is configured as a built-in we get:
> >$ ls -la /sys/module/vhost_vsock/
> >total 0
> >drwxr-xr-x 2 root root 0 Sep 26 15:59 .
> >drwxr-xr-x 100 root root 0 Sep 26 15:59 ..
> >--w------- 1 root root 4096 Sep 26 15:59 uevent
> >-r--r--r-- 1 root root 4096 Sep 26 15:59 version
> >
> >Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@xxxxxxxxxxxxx>
> >---
> > drivers/vhost/vsock.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> >index 802153e23073..287ea8e480b5 100644
> >--- a/drivers/vhost/vsock.c
> >+++ b/drivers/vhost/vsock.c
> >@@ -956,6 +956,7 @@ static void __exit vhost_vsock_exit(void)
> >
> > module_init(vhost_vsock_init);
> > module_exit(vhost_vsock_exit);
> >+MODULE_VERSION("0.0.1");
Hi Stefano,
>
> I was looking at other commits to see how versioning is handled in order
> to make sense (e.g. using the same version of the kernel), and I saw
> many commits that are removing MODULE_VERSION because they say it
> doesn't make sense in in-tree modules.
Yeah, I agree absolutely. I guess that's why all vhost modules have
had version 0.0.1 for years now
and there is no reason to increment version numbers at all.
My proposal is not about version itself, having MODULE_VERSION
specified is a hack which
makes a built-in module appear in /sys/modules/ directory.
I spent some time reading the code in kernel/params.c and
kernel/module/sysfs.c to figure out
why there is no /sys/module/vhost_vsock directory when vhost_vsock is
built-in. And figured out the
precise conditions which must be satisfied to have a module listed in
/sys/module.
To be more precise, built-in module X appears in /sys/module/X if one
of two conditions are met:
- module has MODULE_VERSION declared
- module has any parameter declared
Then I found "module: show version information for built-in modules in sysfs":
https://github.com/torvalds/linux/commit/e94965ed5beb23c6fabf7ed31f625e66d7ff28de
and it inspired me to make this minimalistic change.
>
> In particular the interesting thing is from nfp, where
> `MODULE_VERSION(UTS_RELEASE);` was added with this commit:
>
> 1a5e8e350005 ("nfp: populate MODULE_VERSION")
>
> And then removed completely with this commit:
>
> b4f37219813f ("net/nfp: Update driver to use global kernel version")
>
> CCing Jakub since he was involved, so maybe he can give us some
> pointers.
Kind regards,
Alex
>
> Thanks,
> Stefano
>
> > MODULE_LICENSE("GPL v2");
> > MODULE_AUTHOR("Asias He");
> > MODULE_DESCRIPTION("vhost transport for vsock ");
> >--
> >2.34.1
> >
>
