Re: [PATCH] modules: sysfs - export: taint, address, size

From: Kay Sievers
Date: Mon Jan 09 2012 - 07:45:22 EST


On Mon, Jan 9, 2012 at 08:27, Rusty Russell <rusty@xxxxxxxxxxxxxxx> wrote:
> On Sat, 07 Jan 2012 16:44:36 +0100, Kay Sievers <kay.sievers@xxxxxxxx> wrote:

>> 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.

> 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.

>> +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;
>> +}
>
> 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?

>> +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);
>
> 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'?

> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/