2016-03-27 22:09:53

by Paul Gortmaker

[permalink] [raw]
Subject: [PATCH 0/3] powerpc: remove unused modular code from non-modular drivers

My ongoing audit looking for non-modular code that needlessly uses
modular macros (vs. built-in equivalents) and/or has dead code
relating to module unloading that can never be executed led to the
creation of these three powerpc related commits.

One is of the trivial kind, where we substitute in the non-modular
versions that CPP would have put in place anyway, resulting in no
actual changes, even at the binary output level.

The other two are almost as trivial. In addition to the above, we
toss out the __exit function registered by module_exit, since that
will never get called for non modular code/drivers.

For anyone new to the underlying goal of this cleanup, we are trying to
not use module support for code that can never be built as a module since:

(1) it is easy to accidentally write unused module_exit and remove code
(2) it can be misleading when reading the source, thinking it can be
modular when the Makefile and/or Kconfig prohibit it
(3) it requires the include of the module.h header file which in turn
includes nearly everything else, thus adding to CPP overhead.
(4) it gets copied/replicated into other code and spreads like weeds.

Build tested on v4.6-rc1 to ensure no silly typos that would break
compilation crept in.

---
Cc: Andrzej Hajda <[email protected]>
Cc: Anton Blanchard <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Christian Krafft <[email protected]>
Cc: Hari Bathini <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Nathan Fontenot <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: [email protected]
Cc: [email protected]

Paul Gortmaker (3):
powerpc: make cell/spu_base.c explicitly non-modular
powerpc: make kernel/nvram_64.c explicitly non-modular
drivers/cpufreq: make ppc_cbe_cpufreq_pmi driver explicitly
non-modular

arch/powerpc/kernel/nvram_64.c | 12 +-----------
arch/powerpc/platforms/cell/spu_base.c | 7 ++-----
drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 15 ++-------------
3 files changed, 5 insertions(+), 29 deletions(-)

--
2.6.1


2016-03-27 22:09:41

by Paul Gortmaker

[permalink] [raw]
Subject: [PATCH 1/3] powerpc: make cell/spu_base.c explicitly non-modular

The Kconfig currently controlling compilation of this code is:

arch/powerpc/platforms/cell/Kconfig:config SPU_BASE
arch/powerpc/platforms/cell/Kconfig: bool

...meaning that it currently is not being built as a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.

We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.

Cc: Arnd Bergmann <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
arch/powerpc/platforms/cell/spu_base.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spu_base.c b/arch/powerpc/platforms/cell/spu_base.c
index f7af74f83693..3ede04ffdeea 100644
--- a/arch/powerpc/platforms/cell/spu_base.c
+++ b/arch/powerpc/platforms/cell/spu_base.c
@@ -24,7 +24,7 @@

#include <linux/interrupt.h>
#include <linux/list.h>
-#include <linux/module.h>
+#include <linux/init.h>
#include <linux/ptrace.h>
#include <linux/slab.h>
#include <linux/wait.h>
@@ -805,7 +805,4 @@ static int __init init_spu_base(void)
out:
return ret;
}
-module_init(init_spu_base);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Arnd Bergmann <[email protected]>");
+device_initcall(init_spu_base);
--
2.6.1

2016-03-27 22:09:47

by Paul Gortmaker

[permalink] [raw]
Subject: [PATCH 3/3] drivers/cpufreq: make ppc_cbe_cpufreq_pmi driver explicitly non-modular

The Kconfig for this driver is currently:

config CPU_FREQ_CBE_PMI
bool "CBE frequency scaling using PMI interface"

...meaning that it currently is not being built as a module by
anyone. Lets remove the modular and unused code here, so that
when reading the driver there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.

We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.

Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: Christian Krafft <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
index 7969f7690498..7c4cd5c634f2 100644
--- a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
+++ b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
@@ -23,7 +23,7 @@
#include <linux/kernel.h>
#include <linux/types.h>
#include <linux/timer.h>
-#include <linux/module.h>
+#include <linux/init.h>
#include <linux/of_platform.h>

#include <asm/processor.h>
@@ -142,15 +142,4 @@ static int __init cbe_cpufreq_pmi_init(void)

return 0;
}
-
-static void __exit cbe_cpufreq_pmi_exit(void)
-{
- cpufreq_unregister_notifier(&pmi_notifier_block, CPUFREQ_POLICY_NOTIFIER);
- pmi_unregister_handler(&cbe_pmi_handler);
-}
-
-module_init(cbe_cpufreq_pmi_init);
-module_exit(cbe_cpufreq_pmi_exit);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Christian Krafft <[email protected]>");
+device_initcall(cbe_cpufreq_pmi_init);
--
2.6.1

2016-03-27 22:10:12

by Paul Gortmaker

[permalink] [raw]
Subject: [PATCH 2/3] powerpc: make kernel/nvram_64.c explicitly non-modular

The Makefile/Kconfig currently controlling compilation of this code is:

obj-$(CONFIG_PPC64) += setup_64.o sys_ppc32.o \
signal_64.o ptrace32.o \
paca.o nvram_64.o firmware.o

arch/powerpc/platforms/Kconfig.cputype:config PPC64
arch/powerpc/platforms/Kconfig.cputype: bool "64-bit kernel"

...meaning that it currently is not being built as a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.

We don't replace module.h with init.h since the file already has that.

We delete the MODULE_LICENSE tag since that information is already
contained at the top of the file in the comments.

Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Hari Bathini <[email protected]>
Cc: Nathan Fontenot <[email protected]>
Cc: Andrzej Hajda <[email protected]>
Cc: Anton Blanchard <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
arch/powerpc/kernel/nvram_64.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index 0cab9e8c3794..856f9a7944cd 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -15,8 +15,6 @@
* parsing code.
*/

-#include <linux/module.h>
-
#include <linux/types.h>
#include <linux/errno.h>
#include <linux/fs.h>
@@ -1231,12 +1229,4 @@ static int __init nvram_init(void)

return rc;
}
-
-static void __exit nvram_cleanup(void)
-{
- misc_deregister( &nvram_dev );
-}
-
-module_init(nvram_init);
-module_exit(nvram_cleanup);
-MODULE_LICENSE("GPL");
+device_initcall(nvram_init);
--
2.6.1

2016-03-28 02:33:20

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH 3/3] drivers/cpufreq: make ppc_cbe_cpufreq_pmi driver explicitly non-modular

On 27-03-16, 18:08, Paul Gortmaker wrote:
> The Kconfig for this driver is currently:
>
> config CPU_FREQ_CBE_PMI
> bool "CBE frequency scaling using PMI interface"
>
> ...meaning that it currently is not being built as a module by
> anyone. Lets remove the modular and unused code here, so that
> when reading the driver there is no doubt it is builtin-only.
>
> Since module_init translates to device_initcall in the non-modular
> case, the init ordering remains unchanged with this commit.
>
> We also delete the MODULE_LICENSE tag etc. since all that information
> is already contained at the top of the file in the comments.
>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Viresh Kumar <[email protected]>
> Cc: Christian Krafft <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Paul Gortmaker <[email protected]>
> ---
> drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 15 ++-------------
> 1 file changed, 2 insertions(+), 13 deletions(-)

Acked-by: Viresh Kumar <[email protected]>

--
viresh

2016-03-28 14:22:25

by Nathan Fontenot

[permalink] [raw]
Subject: Re: [PATCH 2/3] powerpc: make kernel/nvram_64.c explicitly non-modular

On 03/27/2016 05:08 PM, Paul Gortmaker wrote:
> The Makefile/Kconfig currently controlling compilation of this code is:
>
> obj-$(CONFIG_PPC64) += setup_64.o sys_ppc32.o \
> signal_64.o ptrace32.o \
> paca.o nvram_64.o firmware.o
>
> arch/powerpc/platforms/Kconfig.cputype:config PPC64
> arch/powerpc/platforms/Kconfig.cputype: bool "64-bit kernel"
>
> ...meaning that it currently is not being built as a module by anyone.
>
> Lets remove the modular code that is essentially orphaned, so that
> when reading the driver there is no doubt it is builtin-only.
>
> Since module_init translates to device_initcall in the non-modular
> case, the init ordering remains unchanged with this commit.
>
> We don't replace module.h with init.h since the file already has that.
>
> We delete the MODULE_LICENSE tag since that information is already
> contained at the top of the file in the comments.
>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Hari Bathini <[email protected]>
> Cc: Nathan Fontenot <[email protected]>
> Cc: Andrzej Hajda <[email protected]>
> Cc: Anton Blanchard <[email protected]>
> Cc: [email protected]
> Signed-off-by: Paul Gortmaker <[email protected]>
> ---

I think at some point in the past we thought this may be useful as a module
but I'm not sure it has ever been used that way.

Reviewed-by: Nathan Fontenot <[email protected]>

> arch/powerpc/kernel/nvram_64.c | 12 +-----------
> 1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> index 0cab9e8c3794..856f9a7944cd 100644
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -15,8 +15,6 @@
> * parsing code.
> */
>
> -#include <linux/module.h>
> -
> #include <linux/types.h>
> #include <linux/errno.h>
> #include <linux/fs.h>
> @@ -1231,12 +1229,4 @@ static int __init nvram_init(void)
>
> return rc;
> }
> -
> -static void __exit nvram_cleanup(void)
> -{
> - misc_deregister( &nvram_dev );
> -}
> -
> -module_init(nvram_init);
> -module_exit(nvram_cleanup);
> -MODULE_LICENSE("GPL");
> +device_initcall(nvram_init);
>

2016-04-11 12:35:09

by Michael Ellerman

[permalink] [raw]
Subject: Re: [1/3] powerpc: make cell/spu_base.c explicitly non-modular

On Sun, 2016-27-03 at 22:08:15 UTC, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
>
> arch/powerpc/platforms/cell/Kconfig:config SPU_BASE
> arch/powerpc/platforms/cell/Kconfig: bool
>
> ...meaning that it currently is not being built as a module by anyone.
>
> Lets remove the modular code that is essentially orphaned, so that
> when reading the driver there is no doubt it is builtin-only.
...
>
> Signed-off-by: Paul Gortmaker <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/8038665fb54f1e54785c63be11

cheers

2016-04-11 12:35:13

by Michael Ellerman

[permalink] [raw]
Subject: Re: [2/3] powerpc: make kernel/nvram_64.c explicitly non-modular

On Sun, 2016-27-03 at 22:08:16 UTC, Paul Gortmaker wrote:
> The Makefile/Kconfig currently controlling compilation of this code is:
>
> obj-$(CONFIG_PPC64) += setup_64.o sys_ppc32.o \
> signal_64.o ptrace32.o \
> paca.o nvram_64.o firmware.o
>
> arch/powerpc/platforms/Kconfig.cputype:config PPC64
> arch/powerpc/platforms/Kconfig.cputype: bool "64-bit kernel"
>
> ...meaning that it currently is not being built as a module by anyone.
>
> Lets remove the modular code that is essentially orphaned, so that
> when reading the driver there is no doubt it is builtin-only.
...
> Signed-off-by: Paul Gortmaker <[email protected]>
> Reviewed-by: Nathan Fontenot <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/c0c523897d1f83bc8484cb58d1

cheers

2016-04-11 12:35:19

by Michael Ellerman

[permalink] [raw]
Subject: Re: [3/3] drivers/cpufreq: make ppc_cbe_cpufreq_pmi driver explicitly non-modular

On Sun, 2016-27-03 at 22:08:17 UTC, Paul Gortmaker wrote:
> The Kconfig for this driver is currently:
>
> config CPU_FREQ_CBE_PMI
> bool "CBE frequency scaling using PMI interface"
>
> ...meaning that it currently is not being built as a module by
> anyone. Lets remove the modular and unused code here, so that
> when reading the driver there is no doubt it is builtin-only.
>
...
> Signed-off-by: Paul Gortmaker <[email protected]>
> Acked-by: Viresh Kumar <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/6a0bcab9c6c337e14689cabd27

cheers