[PATCH v3] module: print version for external modules in print_modules()

[PATCH v3] module: print version for external modules in print_modules()

[Yafang Shao]

We maintain a vmcore analysis script on each server that automatically
parses /var/crash/XXXX/vmcore-dmesg.txt to categorize vmcores. This helps
us save considerable effort by avoiding analysis of known bugs.

For vmcores triggered by a driver bug, the system calls print_modules() to
list the loaded modules. However, print_modules() does not output module
version information. Across a large fleet of servers, there are often many
different module versions running simultaneously, and we need to know which
driver version caused a given vmcore.

Currently, the only reliable way to obtain the module version associated
with a vmcore is to analyze the /var/crash/XXXX/vmcore file itself—an
operation that is resource-intensive. Therefore, we propose printing the
driver version directly in the log, which is far more efficient.

The motivation behind this change is that the external NVIDIA driver
[0] frequently causes kernel panics across our server fleet.
While we continuously upgrade to newer NVIDIA driver versions,
upgrading the entire fleet is time-consuming. Therefore, we need to
identify which driver version is responsible for each panic.

In-tree modules are tied to the specific kernel version already, so
printing their versions is redundant. However, for external drivers (like
proprietary networking or GPU stacks), the version is the single most
critical piece of metadata for triage. Therefore, to avoid bloating the
information about loaded modules, we only print the version for external
modules.

- Before this patch

  Modules linked in: mlx5_core(O) nvidia(PO) nvme_core

- After this patch

  Modules linked in: mlx5_core-5.8-2.0.3(O) nvidia-535.274.02(PO) nvme_core
                              ^^^^^^^^^^          ^^^^^^^^^^^

Note: nvme_core is a in-tree module[1], so its version isn't printed.

As pointed out by Sami, we must ensure mod->version is valid in
print_modules():

 : We release the memory for mod->version in:
 :
 :   free_module
 :     -> module_remove_modinfo_attrs
 :     -> attr->free = free_modinfo_version
 :
 : And this happens before the module is removed from the list.
 : Couldn't there be a race condition where we read a non-NULL
 : mod->version here, but the buffer is being concurrently released
 : by another core that's unloading the module, resulting in a
 : use-after-free in the pr_cont call?
 :
 : In order to do this safely, we should presumably drop the attr->free
 : call from module_remove_modinfo_attrs and release the attributes
 : only after the synchronize_rcu call in free_module (there's already
 : free_modinfo we can use), so mod->version is valid for the entire
 : time the module is on the list.

Link: https://github.com/NVIDIA/open-gpu-kernel-modules/tags [0]
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/nvme/host/core.c?h=v6.19-rc3#n5448 [1]
Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
Suggested-by: Sami Tolvanen <samitolvanen@google.com>
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 kernel/module/main.c  | 29 +++++++++++++++++------------
 kernel/module/sysfs.c |  2 --
 2 files changed, 17 insertions(+), 14 deletions(-)

---
v2->v3:
- ensure mod->version is valid when printing it. (Sami)

v1->v2:
- print it for external module only (Petr, Aaron)
- add comment for it (Aaron)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 2bac4c7cd019..c8f41fa90f8a 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1384,6 +1384,17 @@ static void free_mod_mem(struct module *mod)
 	module_memory_free(mod, MOD_DATA);
 }
 
+static void free_modinfo(struct module *mod)
+{
+	const struct module_attribute *attr;
+	int i;
+
+	for (i = 0; (attr = modinfo_attrs[i]); i++) {
+		if (attr->free)
+			attr->free(mod);
+	}
+}
+
 /* Free a module, remove from lists, etc. */
 static void free_module(struct module *mod)
 {
@@ -1422,6 +1433,7 @@ static void free_module(struct module *mod)
 	module_bug_cleanup(mod);
 	/* Wait for RCU synchronizing before releasing mod->list and buglist. */
 	synchronize_rcu();
+	free_modinfo(mod);
 	if (try_add_tainted_module(mod))
 		pr_err("%s: adding tainted module to the unloaded tainted modules list failed.\n",
 		       mod->name);
@@ -1779,17 +1791,6 @@ static int setup_modinfo(struct module *mod, struct load_info *info)
 	return 0;
 }
 
-static void free_modinfo(struct module *mod)
-{
-	const struct module_attribute *attr;
-	int i;
-
-	for (i = 0; (attr = modinfo_attrs[i]); i++) {
-		if (attr->free)
-			attr->free(mod);
-	}
-}
-
 bool __weak module_init_section(const char *name)
 {
 	return strstarts(name, ".init");
@@ -3901,7 +3902,11 @@ void print_modules(void)
 	list_for_each_entry_rcu(mod, &modules, list) {
 		if (mod->state == MODULE_STATE_UNFORMED)
 			continue;
-		pr_cont(" %s%s", mod->name, module_flags(mod, buf, true));
+		pr_cont(" %s", mod->name);
+		/* Only append version for out-of-tree modules */
+		if (mod->version && test_bit(TAINT_OOT_MODULE, &mod->taints))
+			pr_cont("-%s", mod->version);
+		pr_cont("%s", module_flags(mod, buf, true));
 	}
 
 	print_unloaded_tainted_modules();
diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c
index 01c65d608873..17d1796d6dc7 100644
--- a/kernel/module/sysfs.c
+++ b/kernel/module/sysfs.c
@@ -278,8 +278,6 @@ static void module_remove_modinfo_attrs(struct module *mod, int end)
 		if (!attr->attr.name)
 			break;
 		sysfs_remove_file(&mod->mkobj.kobj, &attr->attr);
-		if (attr->free)
-			attr->free(mod);
 	}
 	kfree(mod->modinfo_attrs);
 }
-- 
2.47.3

Re: [PATCH v3] module: print version for external modules in print_modules()

[Christoph Hellwig]

On Tue, Mar 10, 2026 at 10:38:07AM +0800, Yafang Shao wrote:
> For vmcores triggered by a driver bug, the system calls print_modules() to
> list the loaded modules. However, print_modules() does not output module
> version information.

And it should not.

>
> Across a large fleet of servers, there are often many
> different module versions running simultaneously, and we need to know which
> driver version caused a given vmcore.

Then don't run extetrnal modules, which are not a first part citizen.
Get your changeas upstream instead of just leeching the upstream
developers work.

Re: [PATCH v3] module: print version for external modules in print_modules()

[Yafang Shao]

 > Then don't run extetrnal modules, which are not a first part citizen.
 > Get your changeas upstream instead of just leeching the upstream
 > developers work.

That doesn't make any sense.
Could you please explain the rationale behind introducing 
EXPORT_SYMBOL_GPL() and TAINT_OOT_MODULE ?

Re: [PATCH v3] module: print version for external modules in print_modules()

[Christoph Hellwig]

On Tue, Mar 10, 2026 at 09:04:14PM +0800, Yafang Shao wrote:
> 
> 
> > Then don't run extetrnal modules, which are not a first part citizen.
> > Get your changeas upstream instead of just leeching the upstream
> > developers work.
> 
> That doesn't make any sense.
> Could you please explain the rationale behind introducing
> EXPORT_SYMBOL_GPL() and TAINT_OOT_MODULE ?

Your straw man doesn't make any sense to me.

Re: [PATCH v3] module: print version for external modules in print_modules()

[Yafang Shao]

On Tue, Mar 10, 2026 at 9:07 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Mar 10, 2026 at 09:04:14PM +0800, Yafang Shao wrote:
> >
> >
> > > Then don't run extetrnal modules, which are not a first part citizen.
> > > Get your changeas upstream instead of just leeching the upstream
> > > developers work.
> >
> > That doesn't make any sense.
> > Could you please explain the rationale behind introducing
> > EXPORT_SYMBOL_GPL() and TAINT_OOT_MODULE ?
>
> Your straw man doesn't make any sense to me.

This solution has been approved by all MODULE SUPPORT maintainers. Are
you on the maintainer list?
Please help keep our discussions polite.

-- 
Regards
Yafang

Re: [PATCH v3] module: print version for external modules in print_modules()

[Christoph Hellwig]

On Tue, Mar 10, 2026 at 09:11:48PM +0800, Yafang Shao wrote:
> > > Could you please explain the rationale behind introducing
> > > EXPORT_SYMBOL_GPL() and TAINT_OOT_MODULE ?
> >
> > Your straw man doesn't make any sense to me.
> 
> This solution has been approved by all MODULE SUPPORT maintainers. Are
> you on the maintainer list?
> Please help keep our discussions polite.

I don't know what your are trying to say.  You brought up a totally
unrelated strawman argument, and when I remind you of that you're
arguing about politeness and maintainer files?

Re: [PATCH v3] module: print version for external modules in print_modules()

[Yafang Shao]

On Tue, Mar 10, 2026 at 9:14 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Mar 10, 2026 at 09:11:48PM +0800, Yafang Shao wrote:
> > > > Could you please explain the rationale behind introducing
> > > > EXPORT_SYMBOL_GPL() and TAINT_OOT_MODULE ?
> > >
> > > Your straw man doesn't make any sense to me.
> >
> > This solution has been approved by all MODULE SUPPORT maintainers. Are
> > you on the maintainer list?
> > Please help keep our discussions polite.
>
> I don't know what your are trying to say.  You brought up a totally
> unrelated strawman argument, and when I remind you of that you're
> arguing about politeness and maintainer files?

To reiterate a point that may have been overlooked:

Given that the Linux kernel officially supports out-of-tree modules,
we should aim to support them where reasonable. My change is generic
and improves the core code without being tied to any specific external
module.

-- 
Regards
Yafang

Re: [PATCH v3] module: print version for external modules in print_modules()

[Christoph Hellwig]

On Tue, Mar 10, 2026 at 09:19:22PM +0800, Yafang Shao wrote:
> Given that the Linux kernel officially supports out-of-tree modules,

It does not officially support them as a first class entity.

> we should aim to support them where reasonable. My change is generic
> and improves the core code without being tied to any specific external
> module.

It adds overhead to the kernel just for leechers like you that don't
actually contibute their code upstream.  That's always a bad idea.

Re: [PATCH v3] module: print version for external modules in print_modules()

[Yafang Shao]

On Tue, Mar 10, 2026 at 9:21 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Mar 10, 2026 at 09:19:22PM +0800, Yafang Shao wrote:
> > Given that the Linux kernel officially supports out-of-tree modules,
>
> It does not officially support them as a first class entity.

But out-of-tree modules are still a supported entity by the Linux
kernel, correct?

>
> > we should aim to support them where reasonable. My change is generic
> > and improves the core code without being tied to any specific external
> > module.
>
> It adds overhead

Could you please explain what overhead this might introduce?

>  to the kernel just for leechers like you that don't
> actually contibute their code upstream.  That's always a bad idea.

I am not the GPU vendor, so I am unable to upstream the relevant code myself.

By the way, here is a record of my contributions to the Linux kernel
over the past few years:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?qt=author&q=yafang

Given this history, I am puzzled by the "leechers" characterization.

--
Regards

Yafang

Re: [PATCH v3] module: print version for external modules in print_modules()

[Christoph Hellwig]

On Tue, Mar 10, 2026 at 09:30:06PM +0800, Yafang Shao wrote:
> On Tue, Mar 10, 2026 at 9:21 PM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Tue, Mar 10, 2026 at 09:19:22PM +0800, Yafang Shao wrote:
> > > Given that the Linux kernel officially supports out-of-tree modules,
> >
> > It does not officially support them as a first class entity.
> 
> But out-of-tree modules are still a supported entity by the Linux
> kernel, correct?

They are not supported, and any support request that includes them is
typically rejected.

> > It adds overhead
> 
> Could you please explain what overhead this might introduce?

It adds code that needs to be maintained and which is built into every
kernel.

> 
> >  to the kernel just for leechers like you that don't
> > actually contibute their code upstream.  That's always a bad idea.
> 
> I am not the GPU vendor, so I am unable to upstream the relevant code myself.

Well, that's not the kernels problem.

> By the way, here is a record of my contributions to the Linux kernel
> over the past few years:

Why would that matter?  You don't get a wild card to do things otherwise
rejected because you contributed something before.

Re: [PATCH v3] module: print version for external modules in print_modules()

[Yafang Shao]

On Tue, Mar 10, 2026 at 9:33 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Mar 10, 2026 at 09:30:06PM +0800, Yafang Shao wrote:
> > On Tue, Mar 10, 2026 at 9:21 PM Christoph Hellwig <hch@infradead.org> wrote:
> > >
> > > On Tue, Mar 10, 2026 at 09:19:22PM +0800, Yafang Shao wrote:
> > > > Given that the Linux kernel officially supports out-of-tree modules,
> > >
> > > It does not officially support them as a first class entity.
> >
> > But out-of-tree modules are still a supported entity by the Linux
> > kernel, correct?
>
> They are not supported, and any support request that includes them is
> typically rejected.
>
> > > It adds overhead
> >
> > Could you please explain what overhead this might introduce?
>
> It adds code that needs to be maintained and which is built into every
> kernel.
>
> >
> > >  to the kernel just for leechers like you that don't
> > > actually contibute their code upstream.  That's always a bad idea.
> >
> > I am not the GPU vendor, so I am unable to upstream the relevant code myself.
>
> Well, that's not the kernels problem.
>
> > By the way, here is a record of my contributions to the Linux kernel
> > over the past few years:
>
> Why would that matter?  You don't get a wild card to do things otherwise
> rejected because you contributed something before.

Could you please explain why I am being characterized as a "leecher"?

-- 
Regards
Yafang

Re: [PATCH v3] module: print version for external modules in print_modules()

[Christoph Hellwig]

On Tue, Mar 10, 2026 at 09:35:23PM +0800, Yafang Shao wrote:
> > Why would that matter?  You don't get a wild card to do things otherwise
> > rejected because you contributed something before.
> 
> Could you please explain why I am being characterized as a "leecher"?

Because you're adding overhead to the kernel for your out of tree code,
which only you and not the kernel project itself benefits from.

Re: [PATCH v3] module: print version for external modules in print_modules()

[Yafang Shao]

On Tue, Mar 10, 2026 at 9:43 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Mar 10, 2026 at 09:35:23PM +0800, Yafang Shao wrote:
> > > Why would that matter?  You don't get a wild card to do things otherwise
> > > rejected because you contributed something before.
> >
> > Could you please explain why I am being characterized as a "leecher"?
>
> Because you're adding overhead to the kernel for your out of tree code,
> which only you and not the kernel project itself benefits from.

Why do you believe the kernel project cannot benefit from them, given
that all module maintainers have acknowledged their value?

-- 
Regards
Yafang

Re: [PATCH v3] module: print version for external modules in print_modules()

[Christoph Hellwig]

On Tue, Mar 10, 2026 at 09:44:56PM +0800, Yafang Shao wrote:
> On Tue, Mar 10, 2026 at 9:43 PM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Tue, Mar 10, 2026 at 09:35:23PM +0800, Yafang Shao wrote:
> > > > Why would that matter?  You don't get a wild card to do things otherwise
> > > > rejected because you contributed something before.
> > >
> > > Could you please explain why I am being characterized as a "leecher"?
> >
> > Because you're adding overhead to the kernel for your out of tree code,
> > which only you and not the kernel project itself benefits from.
> 
> Why do you believe the kernel project cannot benefit from them, given
> that all module maintainers have acknowledged their value?

Because by definition it does not benefit from making life for
out of tree modules easier.  And I really don't get your whole
call to authority here, it makes hard for me to take you serious.

Re: [PATCH v3] module: print version for external modules in print_modules()

[Yafang Shao]

On Tue, Mar 10, 2026 at 9:47 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Mar 10, 2026 at 09:44:56PM +0800, Yafang Shao wrote:
> > On Tue, Mar 10, 2026 at 9:43 PM Christoph Hellwig <hch@infradead.org> wrote:
> > >
> > > On Tue, Mar 10, 2026 at 09:35:23PM +0800, Yafang Shao wrote:
> > > > > Why would that matter?  You don't get a wild card to do things otherwise
> > > > > rejected because you contributed something before.
> > > >
> > > > Could you please explain why I am being characterized as a "leecher"?
> > >
> > > Because you're adding overhead to the kernel for your out of tree code,
> > > which only you and not the kernel project itself benefits from.
> >
> > Why do you believe the kernel project cannot benefit from them, given
> > that all module maintainers have acknowledged their value?
>
> Because by definition it does not benefit from making life for
> out of tree modules easier.  And I really don't get your whole
> call to authority here, it makes hard for me to take you serious.

I think we'll have to agree to disagree on this.
Ultimately, it's up to the module maintainers to decide whether to accept it.

-- 
Regards
Yafang

Re: [PATCH v3] module: print version for external modules in print_modules()

[Sami Tolvanen]

On Mon, Mar 09, 2026 at 11:31:28PM -0700, Christoph Hellwig wrote:
> On Tue, Mar 10, 2026 at 10:38:07AM +0800, Yafang Shao wrote:
> > For vmcores triggered by a driver bug, the system calls print_modules() to
> > list the loaded modules. However, print_modules() does not output module
> > version information.
> 
> And it should not.
> 
> >
> > Across a large fleet of servers, there are often many
> > different module versions running simultaneously, and we need to know which
> > driver version caused a given vmcore.
> 
> Then don't run extetrnal modules, which are not a first part citizen.
> Get your changeas upstream instead of just leeching the upstream
> developers work.

As much as I would like to see these modules upstreamed, distributions
do ship out-of-tree modules to users. If adding the OOT module version
to print_modules() helps folks better handle the resulting bug reports,
and maybe even indirectly keeps some of the noise away from upstream, I
feel it's worth the small maintenance burden from this change.

Sami