Currently only drivers that are built as modules have their versions
shown in /sys/module/<module_name>/version, but this information might
also be useful for built-in drivers as well. This especially important
for drivers that do not define any parameters - such drivers, if
built-in, are completely invisible from userspace.
This patch changes MODULE_VERSION() macro so that in case when we are
compiling built-in module, version information is stored in a separate
section. Kernel then uses this data to create 'version' sysfs attribute
in the same fashion it creates attributes for module parameters.
Signed-off-by: Dmitry Torokhov <[email protected]>
---
This change is driven by our desire to detect whether vmw_balloon driver
is built-in into the kernel when we installing our tool package in the
guest and avoid installing our own version of the driver.
Since vmw_balloon does not have any module parameter nor registers any
driver core devices, without this patch it is completely invisible from
userspace when built into the kernel.
Thanks,
Dmitry
include/asm-generic/vmlinux.lds.h | 7 ++++
include/linux/module.h | 27 +++++++++++++++
kernel/params.c | 65 ++++++++++++++++++++++++++++++------
3 files changed, 88 insertions(+), 11 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index bd69d79..0d83dd1 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -355,6 +355,13 @@
VMLINUX_SYMBOL(__start___param) = .; \
*(__param) \
VMLINUX_SYMBOL(__stop___param) = .; \
+ } \
+ \
+ /* Built-in module versions. */ \
+ __modver : AT(ADDR(__modver) - LOAD_OFFSET) { \
+ VMLINUX_SYMBOL(__start___modver) = .; \
+ *(__modver) \
+ VMLINUX_SYMBOL(__stop___modver) = .; \
. = ALIGN((align)); \
VMLINUX_SYMBOL(__end_rodata) = .; \
} \
diff --git a/include/linux/module.h b/include/linux/module.h
index 7575bbb..f74ddda 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -58,6 +58,12 @@ struct module_attribute {
void (*free)(struct module *);
};
+struct module_version_attribute {
+ struct module_attribute mattr;
+ const char *module_name;
+ const char *version;
+};
+
struct module_kobject
{
struct kobject kobj;
@@ -161,7 +167,28 @@ extern struct module __this_module;
Using this automatically adds a checksum of the .c files and the
local headers in "srcversion".
*/
+
+#ifdef MODULE
#define MODULE_VERSION(_version) MODULE_INFO(version, _version)
+#else
+#define MODULE_VERSION(_version) \
+ extern ssize_t __modver_version_show(struct module_attribute *, \
+ struct module *, char *); \
+ static struct module_version_attribute __modver_version_attr \
+ __used \
+ __attribute__ ((unused,__section__ ("__modver"),aligned(sizeof(void *)))) \
+ = { \
+ .mattr = { \
+ .attr = { \
+ .name = "version", \
+ .mode = S_IRUGO, \
+ }, \
+ .show = __modver_version_show, \
+ }, \
+ .module_name = KBUILD_MODNAME, \
+ .version = _version, \
+ }
+#endif
/* Optional firmware file (or files) needed by the module
* format is simply firmware file name. Multiple firmware
diff --git a/kernel/params.c b/kernel/params.c
index 08107d1..0da1411 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -719,9 +719,7 @@ void destroy_params(const struct kernel_param *params, unsigned num)
params[i].ops->free(params[i].arg);
}
-static void __init kernel_add_sysfs_param(const char *name,
- struct kernel_param *kparam,
- unsigned int name_skip)
+static struct module_kobject * __init locate_module_kobject(const char *name)
{
struct module_kobject *mk;
struct kobject *kobj;
@@ -729,10 +727,7 @@ static void __init kernel_add_sysfs_param(const char *name,
kobj = kset_find_obj(module_kset, name);
if (kobj) {
- /* We already have one. Remove params so we can add more. */
mk = to_module_kobject(kobj);
- /* We need to remove it before adding parameters. */
- sysfs_remove_group(&mk->kobj, &mk->mp->grp);
} else {
mk = kzalloc(sizeof(struct module_kobject), GFP_KERNEL);
BUG_ON(!mk);
@@ -743,15 +738,36 @@ static void __init kernel_add_sysfs_param(const char *name,
"%s", name);
if (err) {
kobject_put(&mk->kobj);
- printk(KERN_ERR "Module '%s' failed add to sysfs, "
- "error number %d\n", name, err);
- printk(KERN_ERR "The system will be unstable now.\n");
- return;
+ printk(KERN_ERR
+ "Module '%s' failed add to sysfs, error number %d\n",
+ name, err);
+ printk(KERN_ERR
+ "The system will be unstable now.\n");
+ return NULL;
}
- /* So that exit path is even. */
+
+ /* So that we hold reference in both cases. */
kobject_get(&mk->kobj);
}
+ return mk;
+}
+
+static void __init kernel_add_sysfs_param(const char *name,
+ struct kernel_param *kparam,
+ unsigned int name_skip)
+{
+ struct module_kobject *mk;
+ int err;
+
+ mk = locate_module_kobject(name);
+ if (!mk)
+ return;
+
+ /* We need to remove old parameters before adding more. */
+ if (mk->mp)
+ sysfs_remove_group(&mk->kobj, &mk->mp->grp);
+
/* These should not fail at boot. */
err = add_sysfs_param(mk, kparam, kparam->name + name_skip);
BUG_ON(err);
@@ -796,6 +812,32 @@ static void __init param_sysfs_builtin(void)
}
}
+ssize_t __modver_version_show(struct module_attribute *mattr,
+ struct module *mod, char *buf)
+{
+ struct module_version_attribute *vattr =
+ container_of(mattr, struct module_version_attribute, mattr);
+
+ return sprintf(buf, "%s\n", vattr->version);
+}
+
+extern struct module_version_attribute __start___modver[], __stop___modver[];
+
+static void __init version_sysfs_builtin(void)
+{
+ const struct module_version_attribute *vattr;
+ struct module_kobject *mk;
+ int err;
+
+ for (vattr = __start___modver; vattr < __stop___modver; vattr++) {
+ mk = locate_module_kobject(vattr->module_name);
+ if (mk) {
+ err = sysfs_create_file(&mk->kobj, &vattr->mattr.attr);
+ kobject_uevent(&mk->kobj, KOBJ_ADD);
+ kobject_put(&mk->kobj);
+ }
+ }
+}
/* module-related sysfs stuff */
@@ -875,6 +917,7 @@ static int __init param_sysfs_init(void)
}
module_sysfs_initialized = 1;
+ version_sysfs_builtin();
param_sysfs_builtin();
return 0;
--
1.7.3.2
On Thu, Dec 16, 2010 at 12:00 AM, Dmitry Torokhov <[email protected]> wrote:
> Currently only drivers that are built as modules have their versions
> shown in /sys/module/<module_name>/version, but this information might
Can you create just /sys/module/x?
Module version info is quite useless by itself.
On Wed, Dec 15, 2010 at 02:06:03PM -0800, Alexey Dobriyan wrote:
> On Thu, Dec 16, 2010 at 12:00 AM, Dmitry Torokhov <[email protected]> wrote:
> > Currently only drivers that are built as modules have their versions
> > shown in /sys/module/<module_name>/version, but this information might
>
> Can you create just /sys/module/x?
> Module version info is quite useless by itself.
It is as useful as in the case where driver is built as a module. I am
just trying to unify the behavior a bit.
Code-wise it is almost as cheap to add /sys/module/x/version as adding
just the directory.
Thanks,
Dmitry
On Thu, Dec 16, 2010 at 12:14 AM, Dmitry Torokhov <[email protected]> wrote:
> On Wed, Dec 15, 2010 at 02:06:03PM -0800, Alexey Dobriyan wrote:
>> On Thu, Dec 16, 2010 at 12:00 AM, Dmitry Torokhov <[email protected]> wrote:
>> > Currently only drivers that are built as modules have their versions
>> > shown in /sys/module/<module_name>/version, but this information might
>>
>> Can you create just /sys/module/x?
>> Module version info is quite useless by itself.
>
> It is as useful as in the case where driver is built as a module. I am
> just trying to unify the behavior a bit.
>
> Code-wise it is almost as cheap to add /sys/module/x/version as adding
> just the directory.
OK.
Still, people are using /sys/module/x presence as indicator of modular
built (unsurprisingly).
http://codesearch.google.com/codesearch/p?hl=en#wZuuyuB8jKQ/src/third_party/autotest/files/client/profilers/powertop/src/bluetooth.c&q=/sys/modules&sa=N&cd=3&ct=rc&l=105
On Wed, Dec 15, 2010 at 03:25:27PM -0800, Alexey Dobriyan wrote:
> On Thu, Dec 16, 2010 at 12:14 AM, Dmitry Torokhov <[email protected]> wrote:
> > On Wed, Dec 15, 2010 at 02:06:03PM -0800, Alexey Dobriyan wrote:
> >> On Thu, Dec 16, 2010 at 12:00 AM, Dmitry Torokhov <[email protected]> wrote:
> >> > Currently only drivers that are built as modules have their versions
> >> > shown in /sys/module/<module_name>/version, but this information might
> >>
> >> Can you create just /sys/module/x?
> >> Module version info is quite useless by itself.
> >
> > It is as useful as in the case where driver is built as a module. I am
> > just trying to unify the behavior a bit.
> >
> > Code-wise it is almost as cheap to add /sys/module/x/version as adding
> > just the directory.
>
> OK.
>
> Still, people are using /sys/module/x presence as indicator of modular
> built (unsurprisingly).
>
> http://codesearch.google.com/codesearch/p?hl=en#wZuuyuB8jKQ/src/third_party/autotest/files/client/profilers/powertop/src/bluetooth.c&q=/sys/modules&sa=N&cd=3&ct=rc&l=105
Yeah, I hate to disappoint them but it will break as soon as someone
adds a module_param() to one of the objects... That's even without my
changes.
--
Dmitry
On Wed, Dec 15, 2010 at 03:53:12PM -0800, Dmitry Torokhov wrote:
> On Wed, Dec 15, 2010 at 03:25:27PM -0800, Alexey Dobriyan wrote:
> > On Thu, Dec 16, 2010 at 12:14 AM, Dmitry Torokhov <[email protected]> wrote:
> > > On Wed, Dec 15, 2010 at 02:06:03PM -0800, Alexey Dobriyan wrote:
> > >> On Thu, Dec 16, 2010 at 12:00 AM, Dmitry Torokhov <[email protected]> wrote:
> > >> > Currently only drivers that are built as modules have their versions
> > >> > shown in /sys/module/<module_name>/version, but this information might
> > >>
> > >> Can you create just /sys/module/x?
> > >> Module version info is quite useless by itself.
> > >
> > > It is as useful as in the case where driver is built as a module. I am
> > > just trying to unify the behavior a bit.
> > >
> > > Code-wise it is almost as cheap to add /sys/module/x/version as adding
> > > just the directory.
> >
> > OK.
> >
> > Still, people are using /sys/module/x presence as indicator of modular
> > built (unsurprisingly).
> >
> > http://codesearch.google.com/codesearch/p?hl=en#wZuuyuB8jKQ/src/third_party/autotest/files/client/profilers/powertop/src/bluetooth.c&q=/sys/modules&sa=N&cd=3&ct=rc&l=105
>
> Yeah, I hate to disappoint them but it will break as soon as someone
> adds a module_param() to one of the objects... That's even without my
> changes.
Looking at this some more they are concerned whether bluetooth is
present in the kernel, not whether it is loaded: if you care about power
consumption you need to shut off BT interface whether BT core is a
module or built-in. In this regard I'd say my change will make that code
behave better ;)
--
Dmitry
On Thu, Dec 16, 2010 at 1:53 AM, Dmitry Torokhov <[email protected]> wrote:
> Yeah, I hate to disappoint them but it will break as soon as someone
> adds a module_param() to one of the objects... That's even without my
> changes.
Yeah, it should have been renamed to /sys/tristate :-\
On Thu, 16 Dec 2010 08:30:19 am Dmitry Torokhov wrote:
> Currently only drivers that are built as modules have their versions
> shown in /sys/module/<module_name>/version, but this information might
> also be useful for built-in drivers as well. This especially important
> for drivers that do not define any parameters - such drivers, if
> built-in, are completely invisible from userspace.
>
> This patch changes MODULE_VERSION() macro so that in case when we are
> compiling built-in module, version information is stored in a separate
> section. Kernel then uses this data to create 'version' sysfs attribute
> in the same fashion it creates attributes for module parameters.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
>
> This change is driven by our desire to detect whether vmw_balloon driver
> is built-in into the kernel when we installing our tool package in the
> guest and avoid installing our own version of the driver.
>
> Since vmw_balloon does not have any module parameter nor registers any
> driver core devices, without this patch it is completely invisible from
> userspace when built into the kernel.
>
> Thanks,
> Dmitry
>
> include/asm-generic/vmlinux.lds.h | 7 ++++
> include/linux/module.h | 27 +++++++++++++++
> kernel/params.c | 65 ++++++++++++++++++++++++++++++------
> 3 files changed, 88 insertions(+), 11 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index bd69d79..0d83dd1 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -355,6 +355,13 @@
> VMLINUX_SYMBOL(__start___param) = .; \
> *(__param) \
> VMLINUX_SYMBOL(__stop___param) = .; \
> + } \
> + \
> + /* Built-in module versions. */ \
> + __modver : AT(ADDR(__modver) - LOAD_OFFSET) { \
> + VMLINUX_SYMBOL(__start___modver) = .; \
> + *(__modver) \
> + VMLINUX_SYMBOL(__stop___modver) = .; \
> . = ALIGN((align)); \
> VMLINUX_SYMBOL(__end_rodata) = .; \
> } \
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 7575bbb..f74ddda 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -58,6 +58,12 @@ struct module_attribute {
> void (*free)(struct module *);
> };
>
> +struct module_version_attribute {
> + struct module_attribute mattr;
> + const char *module_name;
> + const char *version;
> +};
> +
> struct module_kobject
> {
> struct kobject kobj;
> @@ -161,7 +167,28 @@ extern struct module __this_module;
> Using this automatically adds a checksum of the .c files and the
> local headers in "srcversion".
> */
> +
> +#ifdef MODULE
> #define MODULE_VERSION(_version) MODULE_INFO(version, _version)
> +#else
> +#define MODULE_VERSION(_version) \
> + extern ssize_t __modver_version_show(struct module_attribute *, \
> + struct module *, char *); \
> + static struct module_version_attribute __modver_version_attr \
> + __used \
> + __attribute__ ((unused,__section__ ("__modver"),aligned(sizeof(void *)))) \
__used and unused seems overkill, and confused.
Removed unused.
Cheers,
Rusty.
On Tue, Dec 21, 2010 at 05:02:40PM -0800, Rusty Russell wrote:
> On Thu, 16 Dec 2010 08:30:19 am Dmitry Torokhov wrote:
> > Currently only drivers that are built as modules have their versions
> > shown in /sys/module/<module_name>/version, but this information might
> > also be useful for built-in drivers as well. This especially important
> > for drivers that do not define any parameters - such drivers, if
> > built-in, are completely invisible from userspace.
> >
> > This patch changes MODULE_VERSION() macro so that in case when we are
> > compiling built-in module, version information is stored in a separate
> > section. Kernel then uses this data to create 'version' sysfs attribute
> > in the same fashion it creates attributes for module parameters.
> >
> > Signed-off-by: Dmitry Torokhov <[email protected]>
> > ---
> >
> > This change is driven by our desire to detect whether vmw_balloon driver
> > is built-in into the kernel when we installing our tool package in the
> > guest and avoid installing our own version of the driver.
> >
> > Since vmw_balloon does not have any module parameter nor registers any
> > driver core devices, without this patch it is completely invisible from
> > userspace when built into the kernel.
> >
> > Thanks,
> > Dmitry
> >
> > include/asm-generic/vmlinux.lds.h | 7 ++++
> > include/linux/module.h | 27 +++++++++++++++
> > kernel/params.c | 65 ++++++++++++++++++++++++++++++------
> > 3 files changed, 88 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index bd69d79..0d83dd1 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -355,6 +355,13 @@
> > VMLINUX_SYMBOL(__start___param) = .; \
> > *(__param) \
> > VMLINUX_SYMBOL(__stop___param) = .; \
> > + } \
> > + \
> > + /* Built-in module versions. */ \
> > + __modver : AT(ADDR(__modver) - LOAD_OFFSET) { \
> > + VMLINUX_SYMBOL(__start___modver) = .; \
> > + *(__modver) \
> > + VMLINUX_SYMBOL(__stop___modver) = .; \
> > . = ALIGN((align)); \
> > VMLINUX_SYMBOL(__end_rodata) = .; \
> > } \
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 7575bbb..f74ddda 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -58,6 +58,12 @@ struct module_attribute {
> > void (*free)(struct module *);
> > };
> >
> > +struct module_version_attribute {
> > + struct module_attribute mattr;
> > + const char *module_name;
> > + const char *version;
> > +};
> > +
> > struct module_kobject
> > {
> > struct kobject kobj;
> > @@ -161,7 +167,28 @@ extern struct module __this_module;
> > Using this automatically adds a checksum of the .c files and the
> > local headers in "srcversion".
> > */
> > +
> > +#ifdef MODULE
> > #define MODULE_VERSION(_version) MODULE_INFO(version, _version)
> > +#else
> > +#define MODULE_VERSION(_version) \
> > + extern ssize_t __modver_version_show(struct module_attribute *, \
> > + struct module *, char *); \
> > + static struct module_version_attribute __modver_version_attr \
> > + __used \
> > + __attribute__ ((unused,__section__ ("__modver"),aligned(sizeof(void *)))) \
>
> __used and unused seems overkill, and confused.
>
Must admit that I copied used/unused verbatim from linux/moduleparam.h:
/* This is the fundamental function for registering boot/module
parameters. */
#define __module_param_call(prefix, name, ops, arg, isbool, perm) \
/* Default value instead of permissions? */ \
static int __param_perm_check_##name __attribute__((unused)) = \
BUILD_BUG_ON_ZERO((perm) < 0 || (perm) > 0777 || ((perm) & 2)) \
+ BUILD_BUG_ON_ZERO(sizeof(""prefix) > MAX_PARAM_PREFIX_LEN); \
static const char __param_str_##name[] = prefix #name; \
static struct kernel_param __moduleparam_const __param_##name \
__used \
__attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *)))) \
= { __param_str_##name, ops, perm, isbool ? KPARAM_ISBOOL : 0, \
{ arg } }
So should it be removed from here as well?
Thanks,
Dmitry
On Thu, 16 Dec 2010 11:28:18 pm Alexey Dobriyan wrote:
> On Thu, Dec 16, 2010 at 1:53 AM, Dmitry Torokhov <[email protected]> wrote:
> > Yeah, I hate to disappoint them but it will break as soon as someone
> > adds a module_param() to one of the objects... That's even without my
> > changes.
>
> Yeah, it should have been renamed to /sys/tristate :-\
Anyway, I've appled the patch.
Thanks,
Rusty.
On Wed, 22 Dec 2010 11:47:12 am Dmitry Torokhov wrote:
> On Tue, Dec 21, 2010 at 05:02:40PM -0800, Rusty Russell wrote:
> > On Thu, 16 Dec 2010 08:30:19 am Dmitry Torokhov wrote:
> > > Currently only drivers that are built as modules have their versions
> > > shown in /sys/module/<module_name>/version, but this information might
> > > also be useful for built-in drivers as well. This especially important
> > > for drivers that do not define any parameters - such drivers, if
> > > built-in, are completely invisible from userspace.
> > >
> > > This patch changes MODULE_VERSION() macro so that in case when we are
> > > compiling built-in module, version information is stored in a separate
> > > section. Kernel then uses this data to create 'version' sysfs attribute
> > > in the same fashion it creates attributes for module parameters.
> > >
> > > Signed-off-by: Dmitry Torokhov <[email protected]>
> > > ---
> > >
> > > This change is driven by our desire to detect whether vmw_balloon driver
> > > is built-in into the kernel when we installing our tool package in the
> > > guest and avoid installing our own version of the driver.
> > >
> > > Since vmw_balloon does not have any module parameter nor registers any
> > > driver core devices, without this patch it is completely invisible from
> > > userspace when built into the kernel.
> > >
> > > Thanks,
> > > Dmitry
> > >
> > > include/asm-generic/vmlinux.lds.h | 7 ++++
> > > include/linux/module.h | 27 +++++++++++++++
> > > kernel/params.c | 65 ++++++++++++++++++++++++++++++------
> > > 3 files changed, 88 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > > index bd69d79..0d83dd1 100644
> > > --- a/include/asm-generic/vmlinux.lds.h
> > > +++ b/include/asm-generic/vmlinux.lds.h
> > > @@ -355,6 +355,13 @@
> > > VMLINUX_SYMBOL(__start___param) = .; \
> > > *(__param) \
> > > VMLINUX_SYMBOL(__stop___param) = .; \
> > > + } \
> > > + \
> > > + /* Built-in module versions. */ \
> > > + __modver : AT(ADDR(__modver) - LOAD_OFFSET) { \
> > > + VMLINUX_SYMBOL(__start___modver) = .; \
> > > + *(__modver) \
> > > + VMLINUX_SYMBOL(__stop___modver) = .; \
> > > . = ALIGN((align)); \
> > > VMLINUX_SYMBOL(__end_rodata) = .; \
> > > } \
> > > diff --git a/include/linux/module.h b/include/linux/module.h
> > > index 7575bbb..f74ddda 100644
> > > --- a/include/linux/module.h
> > > +++ b/include/linux/module.h
> > > @@ -58,6 +58,12 @@ struct module_attribute {
> > > void (*free)(struct module *);
> > > };
> > >
> > > +struct module_version_attribute {
> > > + struct module_attribute mattr;
> > > + const char *module_name;
> > > + const char *version;
> > > +};
> > > +
> > > struct module_kobject
> > > {
> > > struct kobject kobj;
> > > @@ -161,7 +167,28 @@ extern struct module __this_module;
> > > Using this automatically adds a checksum of the .c files and the
> > > local headers in "srcversion".
> > > */
> > > +
> > > +#ifdef MODULE
> > > #define MODULE_VERSION(_version) MODULE_INFO(version, _version)
> > > +#else
> > > +#define MODULE_VERSION(_version) \
> > > + extern ssize_t __modver_version_show(struct module_attribute *, \
> > > + struct module *, char *); \
> > > + static struct module_version_attribute __modver_version_attr \
> > > + __used \
> > > + __attribute__ ((unused,__section__ ("__modver"),aligned(sizeof(void *)))) \
> >
> > __used and unused seems overkill, and confused.
> >
>
> Must admit that I copied used/unused verbatim from linux/moduleparam.h:
>
> /* This is the fundamental function for registering boot/module
> parameters. */
> #define __module_param_call(prefix, name, ops, arg, isbool, perm) \
> /* Default value instead of permissions? */ \
> static int __param_perm_check_##name __attribute__((unused)) = \
> BUILD_BUG_ON_ZERO((perm) < 0 || (perm) > 0777 || ((perm) & 2)) \
> + BUILD_BUG_ON_ZERO(sizeof(""prefix) > MAX_PARAM_PREFIX_LEN); \
> static const char __param_str_##name[] = prefix #name; \
> static struct kernel_param __moduleparam_const __param_##name \
> __used \
> __attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *)))) \
> = { __param_str_##name, ops, perm, isbool ? KPARAM_ISBOOL : 0, \
> { arg } }
>
>
> So should it be removed from here as well?
I think so, but (as always!) check git blame.
Patch welcome!
Rusty.
On Tue, Dec 21, 2010 at 05:48:45PM -0800, Rusty Russell wrote:
> On Wed, 22 Dec 2010 11:47:12 am Dmitry Torokhov wrote:
> > On Tue, Dec 21, 2010 at 05:02:40PM -0800, Rusty Russell wrote:
> > > On Thu, 16 Dec 2010 08:30:19 am Dmitry Torokhov wrote:
> > > > +
> > > > +#ifdef MODULE
> > > > #define MODULE_VERSION(_version) MODULE_INFO(version, _version)
> > > > +#else
> > > > +#define MODULE_VERSION(_version) \
> > > > + extern ssize_t __modver_version_show(struct module_attribute *, \
> > > > + struct module *, char *); \
> > > > + static struct module_version_attribute __modver_version_attr \
> > > > + __used \
> > > > + __attribute__ ((unused,__section__ ("__modver"),aligned(sizeof(void *)))) \
> > >
> > > __used and unused seems overkill, and confused.
> > >
> >
> > Must admit that I copied used/unused verbatim from linux/moduleparam.h:
> >
> > /* This is the fundamental function for registering boot/module
> > parameters. */
> > #define __module_param_call(prefix, name, ops, arg, isbool, perm) \
> > /* Default value instead of permissions? */ \
> > static int __param_perm_check_##name __attribute__((unused)) = \
> > BUILD_BUG_ON_ZERO((perm) < 0 || (perm) > 0777 || ((perm) & 2)) \
> > + BUILD_BUG_ON_ZERO(sizeof(""prefix) > MAX_PARAM_PREFIX_LEN); \
> > static const char __param_str_##name[] = prefix #name; \
> > static struct kernel_param __moduleparam_const __param_##name \
> > __used \
> > __attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *)))) \
> > = { __param_str_##name, ops, perm, isbool ? KPARAM_ISBOOL : 0, \
> > { arg } }
> >
> >
> > So should it be removed from here as well?
>
> I think so, but (as always!) check git blame.
>
Whew, found it:
commit 4d62364652499ef106d1c0737968a879ef077bd4
Author: akpm <akpm>
Date: Tue Jan 20 05:13:24 2004 +0000
[PATCH] make gcc 3.4 compilation work
From: David Mosberger <[email protected]>
With gcc-3.4 we need "attribute((used))" declarations to get "make
modules_install" to work.
Otherwise these sections get dropped from the final image (I assume).
BKrev: 400cb8f4ByHxZCElstAaZ3mBZ2oflQ
...
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 0a5becb..cbca007 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -52,6 +52,7 @@ struct kparam_array
#define __module_param_call(prefix, name, set, get, arg, perm) \
static char __param_str_##name[] __initdata = prefix #name; \
static struct kernel_param const __param_##name \
+ __attribute_used__ \
__attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *)))) \
= { __param_str_##name, perm, set, get, arg }
Since we still claim to support GCC 3.4 I guess __used is still
needed...
Thanks,
Dmitry
On Thu, 23 Dec 2010 11:08:23 am Dmitry Torokhov wrote:
> On Tue, Dec 21, 2010 at 05:48:45PM -0800, Rusty Russell wrote:
> > On Wed, 22 Dec 2010 11:47:12 am Dmitry Torokhov wrote:
> > > On Tue, Dec 21, 2010 at 05:02:40PM -0800, Rusty Russell wrote:
> > > > On Thu, 16 Dec 2010 08:30:19 am Dmitry Torokhov wrote:
> > > > > +
> > > > > +#ifdef MODULE
> > > > > #define MODULE_VERSION(_version) MODULE_INFO(version, _version)
> > > > > +#else
> > > > > +#define MODULE_VERSION(_version) \
> > > > > + extern ssize_t __modver_version_show(struct module_attribute *, \
> > > > > + struct module *, char *); \
> > > > > + static struct module_version_attribute __modver_version_attr \
> > > > > + __used \
> > > > > + __attribute__ ((unused,__section__ ("__modver"),aligned(sizeof(void *)))) \
> > > >
> > > > __used and unused seems overkill, and confused.
> > > >
> > >
> > > Must admit that I copied used/unused verbatim from linux/moduleparam.h:
> > >
> > > /* This is the fundamental function for registering boot/module
> > > parameters. */
> > > #define __module_param_call(prefix, name, ops, arg, isbool, perm) \
> > > /* Default value instead of permissions? */ \
> > > static int __param_perm_check_##name __attribute__((unused)) = \
> > > BUILD_BUG_ON_ZERO((perm) < 0 || (perm) > 0777 || ((perm) & 2)) \
> > > + BUILD_BUG_ON_ZERO(sizeof(""prefix) > MAX_PARAM_PREFIX_LEN); \
> > > static const char __param_str_##name[] = prefix #name; \
> > > static struct kernel_param __moduleparam_const __param_##name \
> > > __used \
> > > __attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *)))) \
> > > = { __param_str_##name, ops, perm, isbool ? KPARAM_ISBOOL : 0, \
> > > { arg } }
> > >
> > >
> > > So should it be removed from here as well?
> >
> > I think so, but (as always!) check git blame.
> >
>
> Whew, found it:
>
> commit 4d62364652499ef106d1c0737968a879ef077bd4
> Author: akpm <akpm>
> Date: Tue Jan 20 05:13:24 2004 +0000
>
> [PATCH] make gcc 3.4 compilation work
>
> From: David Mosberger <[email protected]>
>
> With gcc-3.4 we need "attribute((used))" declarations to get "make
> modules_install" to work.
>
> Otherwise these sections get dropped from the final image (I assume).
>
> BKrev: 400cb8f4ByHxZCElstAaZ3mBZ2oflQ
>
> ...
>
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index 0a5becb..cbca007 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -52,6 +52,7 @@ struct kparam_array
> #define __module_param_call(prefix, name, set, get, arg, perm) \
> static char __param_str_##name[] __initdata = prefix #name; \
> static struct kernel_param const __param_##name \
> + __attribute_used__ \
> __attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *)))) \
> = { __param_str_##name, perm, set, get, arg }
>
>
> Since we still claim to support GCC 3.4 I guess __used is still
> needed...
Well, __used is correct. But this unused should have been removed
at the same time.
Cheers,
Rusty.