| Message ID | 1556034515-28792-1-git-send-email-paul.gortmaker@windriver.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | wdt: clean up unused modular infrastructure | expand |
I saw many drivers that are putting "#ifdef MODULE" around module specific code. I think it answers all 4 concerns above. It also covers the case where a developer made the driver available to be compiled as a module and in the Kconfig didn't make it tristate. What do you think? On Thu, May 9, 2019 at 9:17 AM Paul Gortmaker <paul.gortmaker@windriver.com> wrote: > > People can embed modular includes and modular exit functions into code > that never use any of it, and they won't get any errors or warnings. > > Using modular infrastructure in non-modules might seem harmless, but some > of the downfalls this leads to are: > > (1) it is easy to accidentally write unused module_exit/remove code > (2) it can be misleading when reading the source, thinking a driver can > be modular when the Makefile and/or Kconfig prohibit it > (3) an unused include of the module.h header file will in turn > include nearly everything else; adding a lot to CPP overhead. > (4) it gets copied/replicated into other drivers and can spread. > > As a data point for #3 above, an empty C file that just includes the > module.h header generates over 750kB of CPP output. Repeating the same > experiment with init.h and the result is less than 12kB; with export.h > it is only about 1/2kB; with both it still is less than 12kB. > > Here, In this series, we do what has been done for other subsystems, > like, net, x86, mfd, iommu.... and audit for uses of modular > infrastructure inside code that currently can't be built as a module. > > As always, the option exists for driver authors to convert their code > to tristate, if there is a valid use case for it to be so. But since > I don't have the context for each driver to know if such a use case > exists, I limit myself to simply removing the unused code in order to > make the driver consistent with the Makefile/Kconfig settings that > control it. > > Paul. > > --- > > Cc: Benjamin Fair <benjaminfair@google.com> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: linux-watchdog@vger.kernel.org > Cc: Nancy Yuen <yuenn@google.com> > Cc: openbmc@lists.ozlabs.org > Cc: Patrick Venture <venture@google.com> > Cc: Tali Perry <tali.perry1@gmail.com> > Cc: Tomer Maimon <tmaimon77@gmail.com> > Cc: Wim Van Sebroeck <wim@iguana.be> > Cc: Wim Van Sebroeck <wim@linux-watchdog.org> > > > Paul Gortmaker (5): > watchdog: rtd119x: drop unused module.h include > watchdog: watchdog_core: make it explicitly non-modular > watchdog: npcm: make it explicitly non-modular > watchdog: intel_scu: make it explicitly non-modular > watchdog: coh901327: make it explicitly non-modular > > drivers/watchdog/coh901327_wdt.c | 24 ++++-------------------- > drivers/watchdog/intel_scu_watchdog.c | 18 ------------------ > drivers/watchdog/npcm_wdt.c | 13 ++++++------- > drivers/watchdog/rtd119x_wdt.c | 1 - > drivers/watchdog/watchdog_core.c | 15 +-------------- > 5 files changed, 11 insertions(+), 60 deletions(-) > > -- > 2.7.4 >
People can embed modular includes and modular exit functions into code that never use any of it, and they won't get any errors or warnings. Using modular infrastructure in non-modules might seem harmless, but some of the downfalls this leads to are: (1) it is easy to accidentally write unused module_exit/remove code (2) it can be misleading when reading the source, thinking a driver can be modular when the Makefile and/or Kconfig prohibit it (3) an unused include of the module.h header file will in turn include nearly everything else; adding a lot to CPP overhead. (4) it gets copied/replicated into other drivers and can spread. As a data point for #3 above, an empty C file that just includes the module.h header generates over 750kB of CPP output. Repeating the same experiment with init.h and the result is less than 12kB; with export.h it is only about 1/2kB; with both it still is less than 12kB. Here, In this series, we do what has been done for other subsystems, like, net, x86, mfd, iommu.... and audit for uses of modular infrastructure inside code that currently can't be built as a module. As always, the option exists for driver authors to convert their code to tristate, if there is a valid use case for it to be so. But since I don't have the context for each driver to know if such a use case exists, I limit myself to simply removing the unused code in order to make the driver consistent with the Makefile/Kconfig settings that control it. Paul. --- Cc: Benjamin Fair <benjaminfair@google.com> Cc: Guenter Roeck <linux@roeck-us.net> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: linux-watchdog@vger.kernel.org Cc: Nancy Yuen <yuenn@google.com> Cc: openbmc@lists.ozlabs.org Cc: Patrick Venture <venture@google.com> Cc: Tali Perry <tali.perry1@gmail.com> Cc: Tomer Maimon <tmaimon77@gmail.com> Cc: Wim Van Sebroeck <wim@iguana.be> Cc: Wim Van Sebroeck <wim@linux-watchdog.org> Paul Gortmaker (5): watchdog: rtd119x: drop unused module.h include watchdog: watchdog_core: make it explicitly non-modular watchdog: npcm: make it explicitly non-modular watchdog: intel_scu: make it explicitly non-modular watchdog: coh901327: make it explicitly non-modular drivers/watchdog/coh901327_wdt.c | 24 ++++-------------------- drivers/watchdog/intel_scu_watchdog.c | 18 ------------------ drivers/watchdog/npcm_wdt.c | 13 ++++++------- drivers/watchdog/rtd119x_wdt.c | 1 - drivers/watchdog/watchdog_core.c | 15 +-------------- 5 files changed, 11 insertions(+), 60 deletions(-)