modules: sysfs - export: taint, address, size

PATCH Applied Kay Sievers 6 participants 10 messages Jan 7, 2012 – Jan 11, 2012

lore

Share & tools

lore

Kay Sievers

author

2012-01-07 15:44 UTC #0 lore raw

PATCH 1/1 modules: sysfs - export: taint, address, size

From: Kay Sievers <[email protected]>
Subject: modules: sysfs - export taint, address, size

Recent tools do not use /proc to retrieve module information. A few values are currently missing from sysfs.

Cc Lucas De Marchi <[email protected]>

Signed-off-by Kay Sievers <[email protected]>

---
 kernel/module.c |   89 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 62 insertions(+), 27 deletions(-)

kernel/module.c +62 ~~-27~~

@@ -849,6 +849,26 @@ out:
 	return ret;
 }
 
+static size_t module_flags_taint(struct module *mod, char *buf)
+{
+	size_t l = 0;
+
+	if (mod->taints & (1 << TAINT_PROPRIETARY_MODULE))
+		buf[l++] = 'P';
+	else if (mod->taints & (1 << TAINT_OOT_MODULE))
+		buf[l++] = 'O';
+	if (mod->taints & (1 << TAINT_FORCED_MODULE))
+		buf[l++] = 'F';
+	if (mod->taints & (1 << TAINT_CRAP))
+		buf[l++] = 'C';
+	/*
+	 * TAINT_FORCED_RMMOD: could be added.
+	 * TAINT_UNSAFE_SMP, TAINT_MACHINE_CHECK, TAINT_BAD_PAGE don't
+	 * apply to modules.
+	 */
+	return l;
+}
+
 static inline void print_unload_info(struct seq_file *m, struct module *mod)
 {
 	struct module_use *use;

@@ -907,10 +927,8 @@ static ssize_t show_refcnt(struct module
 	return sprintf(buffer, "%u\n", module_refcount(mk->mod));
 }
 
-static struct module_attribute refcnt = {
-	.attr = { .name = "refcnt", .mode = 0444 },
-	.show = show_refcnt,
-};
+static struct module_attribute refcnt =
+	__ATTR(refcnt, 0444, show_refcnt, NULL);
 
 void module_put(struct module *module)
 {

@@ -970,10 +988,8 @@ static ssize_t show_initstate(struct mod
 	return sprintf(buffer, "%s\n", state);
 }
 
-static struct module_attribute initstate = {
-	.attr = { .name = "initstate", .mode = 0444 },
-	.show = show_initstate,
-};
+static struct module_attribute module_initstate =
+	__ATTR(initstate, 0444, show_initstate, NULL);
 
 static ssize_t store_uevent(struct module_attribute *mattr,
 			    struct module_kobject *mk,

@@ -986,16 +1002,48 @@ static ssize_t store_uevent(struct modul
 	return count;
 }
 
-struct module_attribute module_uevent = {
-	.attr = { .name = "uevent", .mode = 0200 },
-	.store = store_uevent,
-};
+struct module_attribute module_uevent =
+	__ATTR(uevent, 0200, NULL, store_uevent);
+
+static ssize_t show_address(struct module_attribute *mattr,
+			    struct module_kobject *mk, char *buffer)
+{
+	return sprintf(buffer, "0x%pK\n", mk->mod->module_core);
+}
+
+struct module_attribute module_address =
+	__ATTR(address, 0444, show_address, NULL);
+
+static ssize_t show_size(struct module_attribute *mattr,
+			struct module_kobject *mk, char *buffer)
+{
+	return sprintf(buffer, "%u\n", mk->mod->init_size + mk->mod->core_size);
+}
+
+struct module_attribute module_size =
+	__ATTR(size, 0444, show_size, NULL);
+
+static ssize_t show_taint(struct module_attribute *mattr,
+			  struct module_kobject *mk, char *buffer)
+{
+	size_t l;
+
+	l = module_flags_taint(mk->mod, buffer);
+	buffer[l++] = '\n';
+	return l;
+}
+
+struct module_attribute module_taint =
+	__ATTR(taint, 0444, show_taint, NULL);
 
 static struct module_attribute *modinfo_attrs[] = {
 	&modinfo_version,
 	&modinfo_srcversion,
-	&initstate,
+	&module_initstate,
 	&module_uevent,
+	&module_address,
+	&module_size,
+	&module_taint,
 #ifdef CONFIG_MODULE_UNLOAD
 	&refcnt,
 #endif

@@ -3256,20 +3304,7 @@ static char *module_flags(struct module
 	    mod->state == MODULE_STATE_GOING ||
 	    mod->state == MODULE_STATE_COMING) {
 		buf[bx++] = '(';
-		if (mod->taints & (1 << TAINT_PROPRIETARY_MODULE))
-			buf[bx++] = 'P';
-		else if (mod->taints & (1 << TAINT_OOT_MODULE))
-			buf[bx++] = 'O';
-		if (mod->taints & (1 << TAINT_FORCED_MODULE))
-			buf[bx++] = 'F';
-		if (mod->taints & (1 << TAINT_CRAP))
-			buf[bx++] = 'C';
-		/*
-		 * TAINT_FORCED_RMMOD: could be added.
-		 * TAINT_UNSAFE_SMP, TAINT_MACHINE_CHECK, TAINT_BAD_PAGE don't
-		 * apply to modules.
-		 */
-
+		bx += module_flags_taint(mod, buf + bx);
 		/* Show a - for module-is-being-unloaded */
 		if (mod->state == MODULE_STATE_GOING)
 			buf[bx++] = '-';

Rusty Russell

2012-01-09 07:29 UTC #1 lore raw

Quoted 5 lines · Kay Sievers

From: Kay Sievers <[email protected]>
Subject: modules: sysfs - export taint, address, size
Recent tools do not use /proc to retrieve module information. A few values are currently missing from sysfs.

Well, strace says lsmod still does. Is libkmod doing something different? Should we be deprecating /proc/modules?

Quoted 21 lines

Cc Lucas De Marchi <[email protected]>

Signed-off-by Kay Sievers <[email protected]>

---
 kernel/module.c |   89 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 62 insertions(+), 27 deletions(-)

kernel/module.c +9 -0

@@ -849,6 +849,26 @@ out:
 	return ret;
 }
 
+static size_t module_flags_taint(struct module *mod, char *buf)
+{
+	size_t l = 0;
+
+	if (mod->taints & (1 << TAINT_PROPRIETARY_MODULE))
+		buf[l++] = 'P';
+	else if (mod->taints & (1 << TAINT_OOT_MODULE))
+		buf[l++] = 'O';
+	if (mod->taints & (1 << TAINT_FORCED_MODULE))

kernel/module.c out: · line 870

Quoted 10 lines

kernel/module.c +10 -0

+		buf[l++] = 'F';
+	if (mod->taints & (1 << TAINT_CRAP))
+		buf[l++] = 'C';
+	/*
+	 * TAINT_FORCED_RMMOD: could be added.
+	 * TAINT_UNSAFE_SMP, TAINT_MACHINE_CHECK, TAINT_BAD_PAGE don't
+	 * apply to modules.
+	 */
+	return l;
+}

The else here is weird. Shouldn't we leave the exclusion elsewhere?

Quoted 7 lines

kernel/module.c +7 -0

+static ssize_t show_address(struct module_attribute *mattr,
+			    struct module_kobject *mk, char *buffer)
+{
+	return sprintf(buffer, "0x%pK\n", mk->mod->module_core);
+}
+
+struct module_attribute module_address =

kernel/module.c static ssize_t store_uevent(struct modul · line 1024

Quoted 10 lines

kernel/module.c +10 -0

+	__ATTR(address, 0444, show_address, NULL);
+
+static ssize_t show_size(struct module_attribute *mattr,
+			struct module_kobject *mk, char *buffer)
+{
+	return sprintf(buffer, "%u\n", mk->mod->init_size + mk->mod->core_size);
+}
+
+struct module_attribute module_size =
+	__ATTR(size, 0444, show_size, NULL);

This copies a past mistake, and is definitely wrong. Either expose both pointers and sizes, or don't include init_size here. Sure, it'll normally be 0, but if not it's confusing...

But the bigger question is: Why are we exposing these sizes? /proc/modules did since 2.2, or before, but that doesn't make it the best option...

Cheers,
Rusty.

Kay Sievers

author

2012-01-09 12:45 UTC #2 lore raw

Quoted 7 lines · Rusty Russell

Quoted 2 lines · Kay Sievers
Recent tools do not use /proc to retrieve module information. A few values are currently missing from sysfs.
Well, strace says lsmod still does. Is libkmod doing something different?

Yes, kmod used /sys only.

There is current code to read the size, to provide the 'lsmod' output, but that will be removed.

Quoted 1 line

Should we be deprecating /proc/modules?

In the longer run, yes.

We still aim for leaving everything that isn't process- or namespace-related (which, with some stretch is always process-related) alone, and use /sys for it.

Quoted 13 lines

kernel/module.c +13 -0

+static size_t module_flags_taint(struct module *mod, char *buf)
+{
+     size_t l = 0;
+
+     if (mod->taints & (1 << TAINT_PROPRIETARY_MODULE))
+             buf[l++] = 'P';
+     else if (mod->taints & (1 << TAINT_OOT_MODULE))
+             buf[l++] = 'O';
+     if (mod->taints & (1 << TAINT_FORCED_MODULE))
+             buf[l++] = 'F';
+     if (mod->taints & (1 << TAINT_CRAP))
+             buf[l++] = 'C';
+     /*

Quoted 8 lines

Quoted 6 lines
+ * TAINT_FORCED_RMMOD: could be added.
+ * TAINT_UNSAFE_SMP, TAINT_MACHINE_CHECK, TAINT_BAD_PAGE don't
+ * apply to modules.
+ */
+ return l;
+}
The else here is weird. Shouldn't we leave the exclusion elsewhere?

You mean the 'else if ... TAINT_OOT_MODULE'? It's a one-to-one copy of the current code, which just moved up a bit.

Disconnect the two flags form each other?

Quoted 13 lines

kernel/module.c +13 -0

+static ssize_t show_address(struct module_attribute *mattr,
+                         struct module_kobject *mk, char *buffer)
+{
+     return sprintf(buffer, "0x%pK\n", mk->mod->module_core);
+}
+
+struct module_attribute module_address =
+     __ATTR(address, 0444, show_address, NULL);
+
+static ssize_t show_size(struct module_attribute *mattr,
+                     struct module_kobject *mk, char *buffer)
+{
+     return sprintf(buffer, "%u\n", mk->mod->init_size + mk->mod->core_size);

Quoted 8 lines

kernel/module.c static ssize_t store_uevent(struct modul · line 1024
Quoted 4 lines
kernel/module.c +4 -0
+}
+
+struct module_attribute module_size =
+     __ATTR(size, 0444, show_size, NULL);
This copies a past mistake, and is definitely wrong. Either expose both pointers and sizes, or don't include init_size here. Sure, it'll normally be 0, but if not it's confusing...

Ah, good to know, mod->init_size is 0 for all modules here, so we should just drop mod->init_size and maybe name the 'size' attribute to 'coresize'?

Quoted 3 lines

But the bigger question is: Why are we exposing these sizes? /proc/modules did since 2.2, or before, but that doesn't make it the best option...

Good question, I doubt it is too useful, it's just that 'lsmod' shows it, so we wanted to show too.

Kay

Randy Dunlap

2012-01-09 17:41 UTC #3 lore raw

Quoted 9 lines · Kay Sievers

Quoted 7 lines · Rusty Russell
Quoted 2 lines · Kay Sievers
Recent tools do not use /proc to retrieve module information. A few values are currently missing from sysfs.
Well, strace says lsmod still does. Is libkmod doing something different?

Quoted 8 lines

Yes, kmod used /sys only.
There is current code to read the size, to provide the 'lsmod' output, but that will be removed.
Quoted 1 line
Should we be deprecating /proc/modules?
In the longer run, yes.

Deprecate it for udev? OK.
what about other users of it?

Quoted 3 lines

We still aim for leaving everything that isn't process- or namespace-related (which, with some stretch is always process-related) alone, and use /sys for it.

Signature · 2 lines

~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Rusty Russell

2012-01-09 22:54 UTC #4 lore raw

In reply to Kay Sievers

Quoted 7 lines · Kay Sievers

Quoted 1 line · Rusty Russell
The else here is weird. Shouldn't we leave the exclusion elsewhere?
You mean the 'else if ... TAINT_OOT_MODULE'? It's a one-to-one copy of the current code, which just moved up a bit.
Disconnect the two flags form each other?

Yes, I think so.

Quoted 7 lines

Quoted 3 lines
This copies a past mistake, and is definitely wrong. Either expose both pointers and sizes, or don't include init_size here. Sure, it'll normally be 0, but if not it's confusing...
Ah, good to know, mod->init_size is 0 for all modules here, so we should just drop mod->init_size and maybe name the 'size' attribute to 'coresize'?

If a module is still initializing, mod->init_size may well be non-zero. Let's rename it to coresize, and add initsize.

Quoted 6 lines

Quoted 3 lines
But the bigger question is: Why are we exposing these sizes? /proc/modules did since 2.2, or before, but that doesn't make it the best option...
Good question, I doubt it is too useful, it's just that 'lsmod' shows it, so we wanted to show too.

And breaking lsmod output might kill some scripts. So it stays.

Let's drop the address stuff though.

We can actually do something more radical: we could change the kernel to call modprobe to resolve unresolved symbols. We already support symbol:<symbol> for symbol_request().

This means that modprobe still needs to maintain a sym->mod mapping (though I would argue depmod should be moved into the kernel source), but not any dependency mapping.

Thanks,
Rusty.

Kay Sievers

author

2012-01-10 16:47 UTC #5 lore raw

Quoted 26 lines · Rusty Russell

Quoted 7 lines · Kay Sievers
Quoted 1 line · Rusty Russell
The else here is weird. Shouldn't we leave the exclusion elsewhere?
You mean the 'else if ... TAINT_OOT_MODULE'? It's a one-to-one copy of the current code, which just moved up a bit.
Disconnect the two flags form each other?
Yes, I think so.
Quoted 7 lines
Quoted 3 lines
This copies a past mistake, and is definitely wrong. Either expose both pointers and sizes, or don't include init_size here. Sure, it'll normally be 0, but if not it's confusing...
Ah, good to know, mod->init_size is 0 for all modules here, so we should just drop mod->init_size and maybe name the 'size' attribute to 'coresize'?
If a module is still initializing, mod->init_size may well be non-zero. Let's rename it to coresize, and add initsize.
Quoted 4 lines
Quoted 3 lines
But the bigger question is: Why are we exposing these sizes? /proc/modules did since 2.2, or before, but that doesn't make it the best option...

Quoted 6 lines

Quoted 2 lines
Good question, I doubt it is too useful, it's just that 'lsmod' shows it, so we wanted to show too.
And breaking lsmod output might kill some scripts. So it stays.
Let's drop the address stuff though.

From: Kay Sievers <[email protected]>
Subject: modules: sysfs - export: taint, coresize, initsize

Recent tools do not want to use /proc to retrieve module information. A few values are currently missing from sysfs to replace the information available in /proc/modules.

This adds /sys/module/*/{coresize,initsize,taint} attributes.

TAINT_PROPRIETARY_MODULE (P) and TAINT_OOT_MODULE (O) flags are both always shown now, and do no longer exclude each other, also in /proc/modules.

Replace the open-coded sysfs attribute initializers with the __ATTR() macro.

Add the new attributes to Documentation/ABI.

Cc Lucas De Marchi <[email protected]>

Signed-off-by Kay Sievers <[email protected]>

---
 Documentation/ABI/testing/sysfs-module |   16 +++++
 kernel/module.c                        |   93 ++++++++++++++++++++++-----------
 2 files changed, 80 insertions(+), 29 deletions(-)

Documentation/ABI/testing/sysfs-module +16 -0

@@ -33,3 +33,19 @@ Description:	Maximum time allowed for pe
 		Beware, non-standard modes are usually not thoroughly tested by
 		hardware designers, and the hardware can malfunction when this
 		setting differ from default 100.
+
+What:		/sys/module/*/{coresize,initsize}
+Date:		Jan 2012
+KernelVersion:»·3.3
+Contact:	Kay Sievers <[email protected]>
+Description:	Module size in bytes.
+
+What:		/sys/module/*/taint
+Date:		Jan 2012
+KernelVersion:»·3.3
+Contact:	Kay Sievers <[email protected]>
+Description:	Module taint flags:
+			P - proprietary module
+			O - out-of-tree module
+			F - force-loaded module
+			C - staging driver module

kernel/module.c +64 ~~-29~~

128 lines not on this page · raw message

Rusty Russell

Applied

2012-01-11 01:31 UTC #6 lore raw

Quoted 12 lines · Kay Sievers

From: Kay Sievers <[email protected]>
Subject: modules: sysfs - export: taint, coresize, initsize
Recent tools do not want to use /proc to retrieve module information. A few values are currently missing from sysfs to replace the information available in /proc/modules.
This adds /sys/module/*/{coresize,initsize,taint} attributes.
TAINT_PROPRIETARY_MODULE (P) and TAINT_OOT_MODULE (O) flags are both always shown now, and do no longer exclude each other, also in /proc/modules.

Quoted 6 lines

Replace the open-coded sysfs attribute initializers with the __ATTR() macro.
Add the new attributes to Documentation/ABI.
Cc Lucas De Marchi <[email protected]>
Signed-off-by Kay Sievers <[email protected]>

Thanks, applied.

Cheers,
Rusty.
PS. You could have split this into four separate patches, like the
pedants wanted, and got more gititude!
PPS. I'm so glad you didn't.

Lucas De Marchi

2012-01-11 01:56 UTC #7 lore raw

Quoted 13 lines · Rusty Russell

Quoted 12 lines · Kay Sievers
From: Kay Sievers <[email protected]>
Subject: modules: sysfs - export: taint, coresize, initsize
Recent tools do not want to use /proc to retrieve module information. A few values are currently missing from sysfs to replace the information available in /proc/modules.
This adds /sys/module/*/{coresize,initsize,taint} attributes.
TAINT_PROPRIETARY_MODULE (P) and TAINT_OOT_MODULE (O) flags are both always shown now, and do no longer exclude each other, also in /proc/modules.

Quoted 9 lines

Quoted 6 lines
Replace the open-coded sysfs attribute initializers with the __ATTR() macro.
Add the new attributes to Documentation/ABI.
Cc Lucas De Marchi <[email protected]>
Signed-off-by Kay Sievers <[email protected]>
Thanks, applied.

Nice to see this patch applied. I'll prepare the changes to kmod.

Thanks
Lucas De Marchi

Nick Bowler

2012-01-09 16:02 UTC #8 lore raw

In reply to Kay Sievers

Quoted 5 lines · Kay Sievers

From: Kay Sievers <[email protected]>
Subject: modules: sysfs - export taint, address, size
Recent tools do not use /proc to retrieve module information. A few values are currently missing from sysfs.

[...]

kernel/module.c static ssize_t show_refcnt(struct module · line 931

Quoted 10 lines

kernel/module.c +2 -4

@@ -907,10 +927,8 @@ static ssize_t show_refcnt(struct module
 	return sprintf(buffer, "%u\n", module_refcount(mk->mod));
 }
 
-static struct module_attribute refcnt = {
-	.attr = { .name = "refcnt", .mode = 0444 },
-	.show = show_refcnt,
-};
+static struct module_attribute refcnt =
+	__ATTR(refcnt, 0444, show_refcnt, NULL);

This change seems unrelated to the patch description, and is not mentioned in the changelog. Several instances of this.

[...]

kernel/module.c static ssize_t store_uevent(struct modul · line 1042

Quoted 5 lines

kernel/module.c +1 -1

 static struct module_attribute *modinfo_attrs[] = {
 	&modinfo_version,
 	&modinfo_srcversion,
-	&initstate,
+	&module_initstate,

Another unrelated change?

kernel/module.c static ssize_t store_uevent(struct modul · line 1046

Quoted 4 lines

kernel/module.c +3 -0

 	&module_uevent,
+	&module_address,
+	&module_size,
+	&module_taint,

You've forgotten to update Documentation/ABI to describe these new attributes.

Cheers,

Signature · 1 line

Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

Greg KH

2012-01-09 23:07 UTC #9 lore raw

In reply to Kay Sievers

Quoted 10 lines · Kay Sievers

From: Kay Sievers <[email protected]>
Subject: modules: sysfs - export taint, address, size
Recent tools do not use /proc to retrieve module information. A few values are currently missing from sysfs.
Cc Lucas De Marchi <[email protected]>
Signed-off-by Kay Sievers <[email protected]>
---
 kernel/module.c |   89 +++++++++++++++++++++++++++++++++++++++-----------------

You forgot the Documentation/ABI/ update as well :)