[PATCH 2.6.11-rc2] modules: add version and srcversion to sysfs

[PATCH 2.6.11-rc2] modules: add version and srcversion to sysfs

[Matt Domsch]

Module:  Add module version and srcversion to the sysfs tree

There are two ways one could go with this:
1) strdup() the interesting fields from modinfo section before it is discarded.
That's what this patch does, and what Greg's original patch did too,
despite his reservations about using strdup().
2) don't drop the modinfo section on load, and use those strings directly.

The problem with 2) is that the modinfo section can be quite large,
compared to the few bytes that the "interesting" fields consume.  And
it would have to be kmalloc'd and copied from the vmalloc area before
that area is dropped.

So, I did #1.

It's trivial to add new fields now.  For my purposes, version and
srcversion are all I need, so that's all I'm exposing via sysfs.
Others may be added later as needed.

As the idea originated from GregKH, I leave his Signed-off-by: intact,
though the implementation is nearly completely new.  Compiled and run
on x86_64.

Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>
Signed-off-by: Matt Domsch <Matt_Domsch@dell.com>

Rusty, thoughts?

Thanks,
Matt

-- 
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

===== include/linux/module.h 1.92 vs edited =====
--- 1.92/include/linux/module.h	2005-01-10 13:28:15 -06:00
+++ edited/include/linux/module.h	2005-01-25 22:25:15 -06:00
@@ -51,6 +51,9 @@ struct module_attribute {
         ssize_t (*show)(struct module_attribute *, struct module *, char *);
         ssize_t (*store)(struct module_attribute *, struct module *,
 			 const char *, size_t count);
+	void (*setup)(struct module *, const char *);
+	int (*test)(struct module *);
+	void (*free)(struct module *);
 };
 
 struct module_kobject
@@ -249,6 +252,8 @@ struct module
 	/* Sysfs stuff. */
 	struct module_kobject mkobj;
 	struct module_param_attrs *param_attrs;
+	const char *version;
+	const char *srcversion;
 
 	/* Exported symbols */
 	const struct kernel_symbol *syms;
===== kernel/module.c 1.132 vs edited =====
--- 1.132/kernel/module.c	2005-01-11 18:42:57 -06:00
+++ edited/kernel/module.c	2005-01-25 23:42:17 -06:00
@@ -663,6 +663,57 @@ static struct module_attribute refcnt = 
 	.show = show_refcnt,
 };
 
+static char *strdup(const char *str)
+{
+	char *s;
+
+	if (!str)
+		return NULL;
+	s = kmalloc(strlen(str)+1, GFP_KERNEL);
+	if (!s)
+		return NULL;
+	strcpy(s, str);
+	return s;
+}
+
+#define MODINFO_ATTR(field)	\
+static void setup_modinfo_##field(struct module *mod, const char *s)  \
+{                                                                     \
+	mod->field = strdup(s);                                       \
+}                                                                     \
+static ssize_t show_modinfo_##field(struct module_attribute *mattr,   \
+	                struct module *mod, char *buffer)             \
+{                                                                     \
+	return sprintf(buffer, "%s\n", mod->field);                   \
+}                                                                     \
+static int modinfo_##field##_exists(struct module *mod)               \
+{                                                                     \
+	return mod->field != NULL;                                    \
+}                                                                     \
+static void free_modinfo_##field(struct module *mod)                  \
+{                                                                     \
+        kfree(mod->field);                                            \
+        mod->field = NULL;                                            \
+}                                                                     \
+static struct module_attribute modinfo_##field = {                    \
+	.attr = { .name = __stringify(field), .mode = 0444,           \
+		  .owner = THIS_MODULE },                             \
+	.show = show_modinfo_##field,                                 \
+	.setup = setup_modinfo_##field,                               \
+	.test = modinfo_##field##_exists,                             \
+	.free = free_modinfo_##field,                                 \
+};
+
+
+MODINFO_ATTR(version);
+MODINFO_ATTR(srcversion);
+
+static struct module_attribute *modinfo_attrs[] = {
+	&modinfo_version,
+	&modinfo_srcversion,
+	NULL,
+};
+
 #else /* !CONFIG_MODULE_UNLOAD */
 static void print_unload_info(struct seq_file *m, struct module *mod)
 {
@@ -1031,6 +1082,29 @@ static void module_remove_refcnt_attr(st
 }
 #endif
 
+static int module_add_modinfo_attrs(struct module *mod)
+{
+	struct module_attribute * attr;
+	int error = 0;
+	int i;
+
+	for (i = 0; (attr = modinfo_attrs[i]) && !error; i++) {
+		if (!attr->test ||
+		    (attr->test && attr->test(mod)))
+			error = sysfs_create_file(&mod->mkobj.kobj,&attr->attr);
+	}
+}
+
+static void module_remove_modinfo_attrs(struct module *mod)
+{
+	struct module_attribute * attr;
+	int i;
+
+	for (i = 0; (attr = modinfo_attrs[i]); i++) {
+		sysfs_remove_file(&mod->mkobj.kobj,&attr->attr);
+		attr->free(mod);
+	}
+}
 
 static int mod_sysfs_setup(struct module *mod,
 			   struct kernel_param *kparam,
@@ -1056,6 +1130,10 @@ static int mod_sysfs_setup(struct module
 	if (err)
 		goto out_unreg;
 
+	err = module_add_modinfo_attrs(mod);
+	if (err)
+		goto out_unreg;
+
 	return 0;
 
 out_unreg:
@@ -1066,6 +1144,7 @@ out:
 
 static void mod_kobject_remove(struct module *mod)
 {
+	module_remove_modinfo_attrs(mod);
 	module_remove_refcnt_attr(mod);
 	module_param_sysfs_remove(mod);
 
@@ -1303,6 +1382,21 @@ static char *get_modinfo(Elf_Shdr *sechd
 	return NULL;
 }
 
+static void setup_modinfo(struct module *mod, Elf_Shdr *sechdrs,
+			  unsigned int infoindex)
+{
+	struct module_attribute * attr;
+	int i;
+
+	for (i = 0; (attr = modinfo_attrs[i]); i++) {
+		if (attr->setup)
+			attr->setup(mod,
+				    get_modinfo(sechdrs,
+						infoindex,
+						attr->attr.name));
+	}
+}
+
 #ifdef CONFIG_KALLSYMS
 int is_exported(const char *name, const struct module *mod)
 {
@@ -1606,6 +1700,9 @@ static struct module *load_module(void _
 
 	/* Set up license info based on the info section */
 	set_license(mod, get_modinfo(sechdrs, infoindex, "license"));
+
+	/* Set up MODINFO_ATTR fields */
+	setup_modinfo(mod, sechdrs, infoindex);
 
 	/* Fix up syms, so that st_value is a pointer to location. */
 	err = simplify_symbols(sechdrs, symindex, strtab, versindex, pcpuindex,

Re: [PATCH 2.6.11-rc2] modules: add version and srcversion to sysfs

[Andreas Gruenbacher]

Hello,

On Wednesday 26 January 2005 07:05, Matt Domsch wrote:
> Module:  Add module version and srcversion to the sysfs tree

why do you need this?

Thanks,
-- 
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX PRODUCTS GMBH

Re: [PATCH 2.6.11-rc2] modules: add version and srcversion to sysfs

[Matt Domsch]

On Wed, Jan 26, 2005 at 10:22:29AM +0100, Andreas Gruenbacher wrote:
> On Wednesday 26 January 2005 07:05, Matt Domsch wrote:
> > Module:  Add module version and srcversion to the sysfs tree
> 
> why do you need this?

a) Tools like DKMS, which deal with changing out individual kernel
modules without replacing the whole kernel, can behave smarter if they
can tell the version of a given module.  The autoinstaller feature,
for example, which determines if your system has a "good" version of a
driver (i.e. if the one provided by DKMS has a newer verson than that
provided by the kernel package installed), and to automatically
compile and install a newer version if DKMS has it but your kernel
doesn't yet have that version.

b) Because tools like DKMS can switch out modules, you can't count on
'modinfo foo.ko', which looks at
/lib/modules/${kernelver}/... actually matching what is loaded into
the kernel already.  Hence asking sysfs for this.

c) as the unbind-driver-from-device work takes shape, it will be
possible to rebind a driver that's built-in (no .ko to modinfo for the
version) to a newly loaded module.  sysfs will have the
currently-built-in version info, for comparison.

d) tech support scripts can then easily grab the version info for
what's running presently - a question I get often.

Thanks,
Matt


-- 
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

Re: [PATCH 2.6.11-rc2] modules: add version and srcversion to sysfs

[Andreas Gruenbacher]

On Wed, 2005-01-26 at 15:09, Matt Domsch wrote:
> On Wed, Jan 26, 2005 at 10:22:29AM +0100, Andreas Gruenbacher wrote:
> > On Wednesday 26 January 2005 07:05, Matt Domsch wrote:
> > > Module:  Add module version and srcversion to the sysfs tree
> > 
> > why do you need this?
> 
> a) Tools like DKMS, which deal with changing out individual kernel
> modules without replacing the whole kernel, can behave smarter if they
> can tell the version of a given module.

They can look at the modules in /lib/modules/$(uname -r).

> The autoinstaller feature,
> for example, which determines if your system has a "good" version of a
> driver (i.e. if the one provided by DKMS has a newer verson than that
> provided by the kernel package installed), and to automatically
> compile and install a newer version if DKMS has it but your kernel
> doesn't yet have that version.

I find the autoinstaller feature quite scary.

> b) Because tools like DKMS can switch out modules, you can't count on
> 'modinfo foo.ko', which looks at
> /lib/modules/${kernelver}/... actually matching what is loaded into
> the kernel already.  Hence asking sysfs for this.

DKMS doesn't manage loading modules, does it? If it does, then at least
it shouldn't; that's even more scary than the autoinstaller. From the
point of view of the kernel, the modules relevant for the running kernel
are those below /lib/modules/$(uname -r)/. If DKMS replaces things
there, it'd better keep proper track of what it did.

I never want to see DKMS try to remove a module from the running kernel
or insmod a new one.

> c) as the unbind-driver-from-device work takes shape, it will be
> possible to rebind a driver that's built-in (no .ko to modinfo for the
> version) to a newly loaded module.  sysfs will have the
> currently-built-in version info, for comparison.
>
> d) tech support scripts can then easily grab the version info for
> what's running presently - a question I get often.

That's something you can do entirely in userspace by looking at the *.ko
files.

Regards,
-- 
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX GMBH

Re: [PATCH 2.6.11-rc2] modules: add version and srcversion to sysfs

[Matt Domsch]

On Wed, Jan 26, 2005 at 05:38:51PM +0100, Andreas Gruenbacher wrote:
> > The autoinstaller feature,
> > for example, which determines if your system has a "good" version of a
> > driver (i.e. if the one provided by DKMS has a newer verson than that
> > provided by the kernel package installed), and to automatically
> > compile and install a newer version if DKMS has it but your kernel
> > doesn't yet have that version.
> 
> I find the autoinstaller feature quite scary.

The autoinstaller itself is mechanism.  When it's invoked is a policy
decision.  I described the policies that Dell employs with it in my
OLS paper last summer.  For non-distribution-provided drivers
(ppp_mppe, ALSA in some cases, 3rd party video), the autoinstaller is
enabled, and it rebuilds and installs those drivers at new kernel boot
time.

For distribution-provided drivers, we default the autoinstaller
off.  But, this has bitten us.  RHEL3 Update 2 and Update 3 both
shipped with an aacraid (boot storage) driver that crashed the kernel
on insmod.  As fate would have it, Dell provided an updated driver
source package in DKMS format to solve this.  An intelligent
autoinstaller could have looked at the version fields and determined
that what was in the "newest" Update kernel was in fact older than
what DKMS had available to it, and used that instead.  Which is why
we've been pushing for MODULE_VERSION() fields in all the drivers, and
why the srcversion field can exist for all drivers now.

 
> > b) Because tools like DKMS can switch out modules, you can't count on
> > 'modinfo foo.ko', which looks at
> > /lib/modules/${kernelver}/... actually matching what is loaded into
> > the kernel already.  Hence asking sysfs for this.
> 
> DKMS doesn't manage loading modules, does it? If it does, then at least
> it shouldn't; that's even more scary than the autoinstaller. From the
> point of view of the kernel, the modules relevant for the running kernel
> are those below /lib/modules/$(uname -r)/. If DKMS replaces things
> there, it'd better keep proper track of what it did.

It does keep track, quite well.
 
> I never want to see DKMS try to remove a module from the running kernel
> or insmod a new one.

Ahh, but that's a policy decision to be made.  I don't have a
production example of needing to do this yet, so it doesn't.  But I
wouldn't rule out the future need for such.  (i.e. wanting to
automate upgrading a NIC driver without rebooting the server).


> > c) as the unbind-driver-from-device work takes shape, it will be
> > possible to rebind a driver that's built-in (no .ko to modinfo for the
> > version) to a newly loaded module.  sysfs will have the
> > currently-built-in version info, for comparison.

As it happens, right now built-in modules don't have modinfo sections,
so this piece doesn't work.  There's no way to ask the kernel for the
version of built-in modules then, it doesn't know.  But with unbind
happening, that will be useful information to have, I'll have to look
into building parts of that section in.  Thanks for pointing this
deficiency out.

Thanks,
Matt

-- 
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

Re: [PATCH 2.6.11-rc2] modules: add version and srcversion to sysfs

[Bill Davidsen]

Andreas Gruenbacher wrote:
> On Wed, 2005-01-26 at 15:09, Matt Domsch wrote:
> 
>>On Wed, Jan 26, 2005 at 10:22:29AM +0100, Andreas Gruenbacher wrote:
>>
>>>On Wednesday 26 January 2005 07:05, Matt Domsch wrote:
>>>
>>>>Module:  Add module version and srcversion to the sysfs tree
>>>
>>>why do you need this?
>>
>>a) Tools like DKMS, which deal with changing out individual kernel
>>modules without replacing the whole kernel, can behave smarter if they
>>can tell the version of a given module.
> 
> 
> They can look at the modules in /lib/modules/$(uname -r).

I think he means he wants to know what's in memory, not on the disk.

[ snip ]
> 
>>c) as the unbind-driver-from-device work takes shape, it will be
>>possible to rebind a driver that's built-in (no .ko to modinfo for the
>>version) to a newly loaded module.  sysfs will have the
>>currently-built-in version info, for comparison.
>>
>>d) tech support scripts can then easily grab the version info for
>>what's running presently - a question I get often.
> 
> 
> That's something you can do entirely in userspace by looking at the *.ko
> files.

How do you find which *.ko file was used to load the module in memory? I 
think you are talking about two things here.

-- 
    -bill davidsen (davidsen@tmr.com)
"The secret to procrastination is to put things off until the
  last possible moment - but no longer"  -me

Re: [PATCH 2.6.11-rc2] modules: add version and srcversion to sysfs

[Paulo Marques]

Matt Domsch wrote:
> [...]
>  
> +static char *strdup(const char *str)
> +{
> +	char *s;
> +
> +	if (!str)
> +		return NULL;
> +	s = kmalloc(strlen(str)+1, GFP_KERNEL);
> +	if (!s)
> +		return NULL;
> +	strcpy(s, str);
> +	return s;
> +}
> +

There is already this code in sound/core/memory.c:

char *snd_kmalloc_strdup(const char *string, int flags)
{
	size_t len;
	char *ptr;

	if (!string)
		return NULL;
	len = strlen(string) + 1;
	ptr = _snd_kmalloc(len, flags);
	if (ptr)
		memcpy(ptr, string, len);
	return ptr;
}

and grep'ing the includes for "strdup" gives this:

./linux/netdevice.h:extern char *net_sysctl_strdup(const char *s);
./linux/parser.h:char *match_strdup(substring_t *);
./sound/core.h:char *snd_kmalloc_strdup(const char *string, int flags);

Actually, I've just grep'ed the entire tree and there are about 7 
similar implementations all over the place:

./arch/um/kernel/process_kern.c:char *uml_strdup(char *string)
./drivers/parport/probe.c:static char *strdup(char *str)
./drivers/md/dm-ioctl.c:static inline char *kstrdup(const char *str)
./net/core/sysctl_net_core.c:char *net_sysctl_strdup(const char *s)
./net/sunrpc/svcauth_unix.c:static char *strdup(char *s)
./sound/core/memory.c:char *snd_kmalloc_strdup(const char *string, int 
flags)
./lib/parser.c:char *match_strdup(substring_t *s)

So maybe we should turn this into a library function and modify the 
callers, so that we have only one implementation. The implementation 
from sound/core seems better for a library function, because of the 
flags argument (and it seems a little more eficient too).

-- 
Paulo Marques - www.grupopie.com

"A journey of a thousand miles begins with a single step."
Lao-tzu, The Way of Lao-tzu

Re: [PATCH 2.6.11-rc2] modules: add version and srcversion to sysfs

[Rusty Russell]

On Wed, 2005-01-26 at 14:32 +0000, Paulo Marques wrote:
> Matt Domsch wrote:
> > [...]
> >  
> > +static char *strdup(const char *str)
...
> Actually, I've just grep'ed the entire tree and there are about 7 
> similar implementations all over the place:

Wow, I'd never noticed.  Linus, please apply 8)

Rusty.
Name: kstrdup
Author: Neil Brown, Rusty Russell and Robert Love
Status: Trivial

Everyone loves reimplementing strdup.  Give them a kstrdup.

Index: linux-2.6.11-rc2-bk4-Misc/include/linux/string.h
===================================================================
--- linux-2.6.11-rc2-bk4-Misc.orig/include/linux/string.h	2004-05-10 15:13:54.000000000 +1000
+++ linux-2.6.11-rc2-bk4-Misc/include/linux/string.h	2005-01-27 13:08:30.042035568 +1100
@@ -88,6 +88,8 @@
 extern void * memchr(const void *,int,__kernel_size_t);
 #endif
 
+extern char *kstrdup(const char *s, int gfp);
+
 #ifdef __cplusplus
 }
 #endif
Index: linux-2.6.11-rc2-bk4-Misc/lib/string.c
===================================================================
--- linux-2.6.11-rc2-bk4-Misc.orig/lib/string.c	2005-01-27 11:26:15.000000000 +1100
+++ linux-2.6.11-rc2-bk4-Misc/lib/string.c	2005-01-27 13:08:30.080029792 +1100
@@ -23,6 +23,7 @@
 #include <linux/string.h>
 #include <linux/ctype.h>
 #include <linux/module.h>
+#include <linux/slab.h>
 
 #ifndef __HAVE_ARCH_STRNICMP
 /**
@@ -599,3 +600,19 @@
 }
 EXPORT_SYMBOL(memchr);
 #endif
+
+/*
+ * kstrdup - allocate space for and copy an existing string
+ *
+ * @s: the string to duplicate
+ * @gfp: the GFP mask used in the kmalloc() call when allocating memory
+ */
+char *kstrdup(const char *s, int gfp)
+{
+	char *buf = kmalloc(strlen(s)+1, gfp);
+	if (buf)
+		strcpy(buf, s);
+	return buf;
+}
+
+EXPORT_SYMBOL(kstrdup);

-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman