Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH] Factor out common MODULE_INFO content from module*.h files.

  

 
Linux kernel RSS feed   Index | Next | Previous | View Threaded


rpjday at mindspring

May 20, 2007, 12:06 PM

Post #1 of 9 (1194 views)
Permalink
[PATCH] Factor out common MODULE_INFO content from module*.h files.
In order to eventually break the interdependency between the module.h
and moduleparam.h header files, factor out the common MODULE_INFO
content into a new header file.

Signed-off-by: Robert P. J. Day <rpjday [at] mindspring>

---

this patch doesn't actually start cleaning up the mess related to
module.h and moduleparam.h files, but it at least allows that cleanup
process to start. a major goal would be to eventually remove that
inclusion of moduleparam.h from module.h, which simply shouldn't be
there.

compile-tested with "make allmodconfig" on x86.


include/linux/module.h | 12 +++---------
include/linux/moduleparam.h | 16 +++++-----------
2 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index e6e0f86..d9a9a13 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -15,7 +15,8 @@
#include <linux/elf.h>
#include <linux/stringify.h>
#include <linux/kobject.h>
-#include <linux/moduleparam.h>
+#include <linux/moduleparam.h> /* Ideally, this shouldn't be necessary. */
+#include <linux/moduleinfo.h>
#include <asm/local.h>

#include <asm/module.h>
@@ -88,9 +89,6 @@ extern struct module __this_module;
#define THIS_MODULE ((struct module *)0)
#endif

-/* Generic info of form tag = "info" */
-#define MODULE_INFO(tag, info) __MODULE_INFO(tag, tag, info)
-
/* For userspace: you can also call me... */
#define MODULE_ALIAS(_alias) MODULE_INFO(alias, _alias)

@@ -130,11 +128,6 @@ extern struct module __this_module;
/* What your module does. */
#define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)

-/* One for each parameter, describing how to use it. Some files do
- multiple of these per line, so can't just use MODULE_INFO. */
-#define MODULE_PARM_DESC(_parm, desc) \
- __MODULE_INFO(parm, _parm, #_parm ":" desc)
-
#define MODULE_DEVICE_TABLE(type,name) \
MODULE_GENERIC_TABLE(type##_device,name)

@@ -576,6 +569,7 @@ struct device_driver;
struct module;

extern struct kset module_subsys;
+struct kernel_param;

int mod_sysfs_init(struct module *mod);
int mod_sysfs_setup(struct module *mod,
diff --git a/include/linux/moduleinfo.h b/include/linux/moduleinfo.h
new file mode 100644
index 0000000..cd9937b
--- /dev/null
+++ b/include/linux/moduleinfo.h
@@ -0,0 +1,18 @@
+#ifndef _LINUX_MODULEINFO_H
+#define _LINUX_MODULEINFO_H
+
+#ifdef MODULE
+#include <linux/stringify.h>
+#define ___module_cat(a,b) __mod_ ## a ## b
+#define __module_cat(a,b) ___module_cat(a,b)
+#define __MODULE_INFO(tag, name, info) \
+static const char __module_cat(name,__LINE__)[] \
+ __maybe_unused \
+ __attribute__((section(".modinfo"))) = __stringify(tag) "=" info
+#else /* !MODULE */
+#define __MODULE_INFO(tag, name, info)
+#endif
+
+#define MODULE_INFO(tag, info) __MODULE_INFO(tag, tag, info)
+
+#endif // _LINUX_MODULEINFO_H
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index c83588c..650328d 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -4,6 +4,7 @@
#include <linux/init.h>
#include <linux/stringify.h>
#include <linux/kernel.h>
+#include <linux/moduleinfo.h>

/* You can override this manually, but generally this should match the
module name. */
@@ -13,19 +14,12 @@
#define MODULE_PARAM_PREFIX KBUILD_MODNAME "."
#endif

-#ifdef MODULE
-#define ___module_cat(a,b) __mod_ ## a ## b
-#define __module_cat(a,b) ___module_cat(a,b)
-#define __MODULE_INFO(tag, name, info) \
-static const char __module_cat(name,__LINE__)[] \
- __attribute_used__ \
- __attribute__((section(".modinfo"),unused)) = __stringify(tag) "=" info
-#else /* !MODULE */
-#define __MODULE_INFO(tag, name, info)
-#endif
-#define __MODULE_PARM_TYPE(name, _type) \
+#define __MODULE_PARM_TYPE(name, _type) \
__MODULE_INFO(parmtype, name##type, #name ":" _type)

+#define MODULE_PARM_DESC(_parm, desc) \
+ __MODULE_INFO(parm, _parm, #_parm ":" desc)
+
struct kernel_param;

/* Returns 0, or -errno. arg is in kp->arg. */

--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


sam at ravnborg

May 20, 2007, 12:15 PM

Post #2 of 9 (1130 views)
Permalink
Re: [PATCH] Factor out common MODULE_INFO content from module*.h files. [In reply to]
On Sun, May 20, 2007 at 03:06:15PM -0400, Robert P. J. Day wrote:
>
> In order to eventually break the interdependency between the module.h
> and moduleparam.h header files, factor out the common MODULE_INFO
> content into a new header file.

The moduleinfo.h file looks redundant at first look.
Why not push relevant parts from moduleparam.h (the
MODULE_INFO bits) to module.h and let go of
the include of moduleparam.h in module.h (when you
have fixed the users)?

In this way we do not add an extra .h file.
And files that needs moduleparam.h will anyway always need module.h.
But not the other way around.

Sam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rpjday at mindspring

May 20, 2007, 12:51 PM

Post #3 of 9 (1129 views)
Permalink
Re: [PATCH] Factor out common MODULE_INFO content from module*.h files. [In reply to]
On Sun, 20 May 2007, Sam Ravnborg wrote:

> On Sun, May 20, 2007 at 03:06:15PM -0400, Robert P. J. Day wrote:
> >
> > In order to eventually break the interdependency between the module.h
> > and moduleparam.h header files, factor out the common MODULE_INFO
> > content into a new header file.
>
> The moduleinfo.h file looks redundant at first look.
> Why not push relevant parts from moduleparam.h (the
> MODULE_INFO bits) to module.h and let go of
> the include of moduleparam.h in module.h (when you
> have fixed the users)?
>
> In this way we do not add an extra .h file.
> And files that needs moduleparam.h will anyway always need module.h.
> But not the other way around.

no problem, i can go that way, too, but there's just one (admittedly
picky) issue associated with that.

based on the above, we would have:

1) module.h handling all generic module content, and
2) moduleparam.h would "#include" module.h and add the parameter
stuff.

fair enough, but note that, with that, if you wanted parameter
support, you would need to include *only* "moduleparam.h". are you
good with that? (as i said, it's picky, but you'd probably still have
a lot of people who, through force of habit, would still #include both
just because they think it's necessary. wouldn't hurt, of course,
since module.h would be protected against multiple inclusion.)

so if you're good with all of the above, i can do that.

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rpjday at mindspring

May 20, 2007, 1:06 PM

Post #4 of 9 (1130 views)
Permalink
Re: [PATCH] Factor out common MODULE_INFO content from module*.h files. [In reply to]
On Sun, 20 May 2007, Sam Ravnborg wrote:

> On Sun, May 20, 2007 at 03:06:15PM -0400, Robert P. J. Day wrote:
> >
> > In order to eventually break the interdependency between the module.h
> > and moduleparam.h header files, factor out the common MODULE_INFO
> > content into a new header file.
>
> The moduleinfo.h file looks redundant at first look. Why not push
> relevant parts from moduleparam.h (the MODULE_INFO bits) to module.h
> and let go of the include of moduleparam.h in module.h (when you
> have fixed the users)?
>
> In this way we do not add an extra .h file. And files that needs
> moduleparam.h will anyway always need module.h. But not the other
> way around.

crap, now i remember why i did it the way i did it.

yes, the way you describe it is a simpler solution, but it would break
all of the files in the tree that use module parameters and have
included *only* module.h, and have been getting away with it all this
time only because module.h currently includes moduleparam.h.

based on a simple script i have, there are currently 583 files under
the drivers/ directory *alone* that are like that. that is, 583 files
that would need to include moduleparam.h instead of module.h simply to
continue to compile if the obvious header file fix were made.

in that situation, the proper (initial) patch would be one that

1) didn't break what was already there, and
2) allowed people to start cleaning up their files little by little

and when all of the cleanup was allegedly complete, the header files
could be adjusted to their final form.

is any of this making sense? yes, sam's suggestion is clearly the
simpler one, but it can't be applied without cleaning everything up
*first*. and without it, people can't start cleaning up code on their
own.

this *has* to be a two-step process. you may not like the
moduleinfo.h file but think of it as a bridge to get to the final
result, when it can finally be thrown away.

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


sam at ravnborg

May 20, 2007, 1:13 PM

Post #5 of 9 (1126 views)
Permalink
Re: [PATCH] Factor out common MODULE_INFO content from module*.h files. [In reply to]
On Sun, May 20, 2007 at 03:51:18PM -0400, Robert P. J. Day wrote:
> On Sun, 20 May 2007, Sam Ravnborg wrote:
>
> > On Sun, May 20, 2007 at 03:06:15PM -0400, Robert P. J. Day wrote:
> > >
> > > In order to eventually break the interdependency between the module.h
> > > and moduleparam.h header files, factor out the common MODULE_INFO
> > > content into a new header file.
> >
> > The moduleinfo.h file looks redundant at first look.
> > Why not push relevant parts from moduleparam.h (the
> > MODULE_INFO bits) to module.h and let go of
> > the include of moduleparam.h in module.h (when you
> > have fixed the users)?
> >
> > In this way we do not add an extra .h file.
> > And files that needs moduleparam.h will anyway always need module.h.
> > But not the other way around.
>
> no problem, i can go that way, too, but there's just one (admittedly
> picky) issue associated with that.
>
> based on the above, we would have:
>
> 1) module.h handling all generic module content, and
> 2) moduleparam.h would "#include" module.h and add the parameter
> stuff.
>
> fair enough, but note that, with that, if you wanted parameter
> support, you would need to include *only* "moduleparam.h". are you
> good with that? (as i said, it's picky, but you'd probably still have
> a lot of people who, through force of habit, would still #include both
> just because they think it's necessary. wouldn't hurt, of course,
> since module.h would be protected against multiple inclusion.)
>
> so if you're good with all of the above, i can do that.

The above is fine and better than having an extra file.

Sam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


sam at ravnborg

May 20, 2007, 1:51 PM

Post #6 of 9 (1132 views)
Permalink
Re: [PATCH] Factor out common MODULE_INFO content from module*.h files. [In reply to]
On Sun, May 20, 2007 at 04:06:40PM -0400, Robert P. J. Day wrote:
> On Sun, 20 May 2007, Sam Ravnborg wrote:
>
> > On Sun, May 20, 2007 at 03:06:15PM -0400, Robert P. J. Day wrote:
> > >
> > > In order to eventually break the interdependency between the module.h
> > > and moduleparam.h header files, factor out the common MODULE_INFO
> > > content into a new header file.
> >
> > The moduleinfo.h file looks redundant at first look. Why not push
> > relevant parts from moduleparam.h (the MODULE_INFO bits) to module.h
> > and let go of the include of moduleparam.h in module.h (when you
> > have fixed the users)?
> >
> > In this way we do not add an extra .h file. And files that needs
> > moduleparam.h will anyway always need module.h. But not the other
> > way around.
>
> crap, now i remember why i did it the way i did it.
>
> yes, the way you describe it is a simpler solution, but it would break
> all of the files in the tree that use module parameters and have
> included *only* module.h, and have been getting away with it all this
> time only because module.h currently includes moduleparam.h.
>
> based on a simple script i have, there are currently 583 files under
> the drivers/ directory *alone* that are like that. that is, 583 files
> that would need to include moduleparam.h instead of module.h simply to
> continue to compile if the obvious header file fix were made.
The pain is too high for this.
Is seems worthwhile to make the change to module.h but
adding an additional include to > 500 drivers is not worth it.

Sam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rpjday at mindspring

May 20, 2007, 2:12 PM

Post #7 of 9 (1130 views)
Permalink
Re: [PATCH] Factor out common MODULE_INFO content from module*.h files. [In reply to]
On Sun, 20 May 2007, Sam Ravnborg wrote:

> On Sun, May 20, 2007 at 04:06:40PM -0400, Robert P. J. Day wrote:
> > On Sun, 20 May 2007, Sam Ravnborg wrote:
> >
> > > On Sun, May 20, 2007 at 03:06:15PM -0400, Robert P. J. Day wrote:
> > > >
> > > > In order to eventually break the interdependency between the module.h
> > > > and moduleparam.h header files, factor out the common MODULE_INFO
> > > > content into a new header file.
> > >
> > > The moduleinfo.h file looks redundant at first look. Why not push
> > > relevant parts from moduleparam.h (the MODULE_INFO bits) to module.h
> > > and let go of the include of moduleparam.h in module.h (when you
> > > have fixed the users)?
> > >
> > > In this way we do not add an extra .h file. And files that needs
> > > moduleparam.h will anyway always need module.h. But not the other
> > > way around.
> >
> > crap, now i remember why i did it the way i did it.
> >
> > yes, the way you describe it is a simpler solution, but it would break
> > all of the files in the tree that use module parameters and have
> > included *only* module.h, and have been getting away with it all this
> > time only because module.h currently includes moduleparam.h.
> >
> > based on a simple script i have, there are currently 583 files under
> > the drivers/ directory *alone* that are like that. that is, 583 files
> > that would need to include moduleparam.h instead of module.h simply to
> > continue to compile if the obvious header file fix were made.

> The pain is too high for this. Is seems worthwhile to make the
> change to module.h but adding an additional include to > 500 drivers
> is not worth it.

under the circumstances, is there *any* cleanup worth doing WRT to
this issue? because of the fact that module.h currently includes
moduleparam.h, developers have been able to get away with being
incredibly sloppy in their includes.

based on a short script i wrote, here are some stats for the drivers/
directory:

$ . ../moduleparam.sh drivers
Number of source files found: 5042
Number of files that include module.h: 3788
Number of files that include moduleparam.h: 358
Number of files that include both: 347
Number of files that include ONLY module.h: 3440
Number of files that include ONLY moduleparam.h: 10
Number of files needing moduleparam.h: 583
Number of files with unnecessary moduleparam.h: 45

interpreting the above:

* there are 347 files that include both module.h and moduleparam.h
when (at the moment) they need to include only the former,

* there are 10 files that include *only* moduleparam.h (which does not
include module.h, so how the heck *those* still build is a mystery),

* there are 583 files that use module parameters in some way that
don't include moduleparam.h, and

* conversely, there are 45 source files that unnecessarily include
moduleparam.h when they don't use module parameters at all.

at this point, you may be right. trying to fix this may be more
grief than it's worth.

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


sam at ravnborg

May 20, 2007, 9:52 PM

Post #8 of 9 (1113 views)
Permalink
Re: [PATCH] Factor out common MODULE_INFO content from module*.h files. [In reply to]
>
> under the circumstances, is there *any* cleanup worth doing WRT to
> this issue? because of the fact that module.h currently includes
> moduleparam.h, developers have been able to get away with being
> incredibly sloppy in their includes.

It is wortwhile to make module.h independent of moduleparam.h.
The MODULE_INFO stuff has nothing to do with module parameters.

But keep the include so you do not break the > 500 drivers.

Sam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


rpjday at mindspring

May 21, 2007, 1:37 AM

Post #9 of 9 (1112 views)
Permalink
Re: [PATCH] Factor out common MODULE_INFO content from module*.h files. [In reply to]
On Mon, 21 May 2007, Sam Ravnborg wrote:

> rday wrote:

> > under the circumstances, is there *any* cleanup worth doing WRT to
> > this issue? because of the fact that module.h currently includes
> > moduleparam.h, developers have been able to get away with being
> > incredibly sloppy in their includes.
>
> It is wortwhile to make module.h independent of moduleparam.h.
> The MODULE_INFO stuff has nothing to do with module parameters.
>
> But keep the include so you do not break the > 500 drivers.

ok, i can do that. but, at the very least, it might be worth it for
some people to stop including moduleparam.h in their code when that
code does nothing with module parameters. my scanning script picked
out the following files under drivers/ that *seem* to be doing just
that, so respective maintainers might want to toss those useless
includes. (obviously, at the moment, *all* includes of moduleparam.h
are useless, but the ones in the files below are *doubly* useless. :-)


drivers/pcmcia/socket_sysfs.c
drivers/pcmcia/au1000_generic.c
drivers/net/macb.c
drivers/net/ixp2000/ixpdev.c
drivers/net/ixp2000/enp2611.c
drivers/net/arm/ep93xx_eth.c
drivers/net/irda/kingsun-sir.c
drivers/scsi/aacraid/linit.c
drivers/scsi/3w-9xxx.c
drivers/scsi/arcmsr/arcmsr_hba.c
drivers/scsi/megaraid/megaraid_sas.c
drivers/scsi/3w-xxxx.c
drivers/bluetooth/bt3c_cs.c
drivers/bluetooth/btuart_cs.c
drivers/bluetooth/bluecard_cs.c
drivers/bluetooth/dtl1_cs.c
drivers/acpi/processor_core.c
drivers/serial/mpsc.c
drivers/mmc/card/block.c
drivers/mmc/host/at91_mci.c
drivers/mmc/host/omap.c
drivers/char/hvc_rtas.c
drivers/char/hw_random/ixp4xx-rng.c
drivers/char/mbcs.c
drivers/char/watchdog/mtx-1_wdt.c
drivers/usb/gadget/fsl_usb2_udc.c
drivers/usb/atm/cxacru.c
drivers/usb/host/sl811-hcd.c
drivers/media/dvb/frontends/isl6421.c
drivers/media/dvb/frontends/lnbp21.c
drivers/media/dvb/frontends/dvb_dummy_fe.c
drivers/media/common/ir-keymaps.c
drivers/media/video/ov7670.c
drivers/media/video/cx88/cx88-vp3054-i2c.c
drivers/infiniband/hw/mthca/mthca_profile.c
drivers/infiniband/hw/amso1100/c2_rnic.c
drivers/infiniband/hw/amso1100/c2_provider.c
drivers/infiniband/hw/cxgb3/iwch.c
drivers/infiniband/hw/cxgb3/iwch_provider.c
drivers/input/mouse/trackpoint.c
drivers/input/serio/libps2.c
drivers/w1/slaves/w1_ds2433.c
drivers/w1/slaves/w1_therm.c
drivers/w1/slaves/w1_smem.c
drivers/s390/net/qeth_main.c

the same could be said for these files under net/:

net/netfilter/nf_conntrack_proto.c
net/ipv6/netfilter/ip6t_LOG.c
net/ipv4/netfilter/nf_nat_h323.c
net/ipv4/netfilter/nf_nat_tftp.c
net/iucv/iucv.c


rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.