In the previous merge window, we made changes to allow better
delineation between modular and non-modular code in commit
0fd972a7d91d6e15393c449492a04d94c0b89351 ("module: relocate module_init
from init.h to module.h"). This allows us to now ensure module code
looks modular and non-modular code does not accidentally look modular
without suffering build breakage.
Here we target code that is, by nature of their Kconfig settings, only
available to be built-in, but implicitly presenting itself as being
possibly modular by way of using modular headers, macros, and functions.
The goal here is to remove that illusion of modularity from these
drivers, but in a way that leaves the actual runtime unchanged.
In doing so, we remove code that has never been tested and adds
no value to the tree. And we advance the process of expecting a
level of consistency between the Kconfig of a driver and the code
that the driver uses.
Build tested on and x86-64, and on ia64 for snsc.c after applying
to a baseline of char-misc/char-misc-next
Paul.
---
Cc: Arnd Bergmann <[email protected]>
Cc: Clemens Ladisch <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jason Wessel <[email protected]>
Cc: [email protected]
Paul Gortmaker (4):
drivers/char: make efirtc.c driver explicitly non-modular
drivers/char: make SGI snsc.c driver explicitly non-modular
drivers/char: make hpet.c explicitly non-modular
drivers/misc: make kgdbts.c slightly more explicitly non-modular
drivers/char/efirtc.c | 13 +++----------
drivers/char/hpet.c | 25 +++----------------------
drivers/char/snsc.c | 5 ++---
drivers/misc/kgdbts.c | 10 +++++-----
4 files changed, 13 insertions(+), 40 deletions(-)
--
2.5.0
The Kconfig for this driver is currently:
config EFI_RTC
bool "EFI Real Time Clock Services"
...meaning that it currently is not being built as a module by anyone.
Lets remove all modular references, 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 leave some tags like MODULE_LICENSE for documentation purposes.
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/char/efirtc.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/char/efirtc.c b/drivers/char/efirtc.c
index e39e7402e623..dc62568b7dde 100644
--- a/drivers/char/efirtc.c
+++ b/drivers/char/efirtc.c
@@ -30,7 +30,6 @@
#include <linux/types.h>
#include <linux/errno.h>
#include <linux/miscdevice.h>
-#include <linux/module.h>
#include <linux/init.h>
#include <linux/rtc.h>
#include <linux/proc_fs.h>
@@ -395,14 +394,8 @@ efi_rtc_init(void)
}
return 0;
}
+device_initcall(efi_rtc_init);
-static void __exit
-efi_rtc_exit(void)
-{
- /* not yet used */
-}
-
-module_init(efi_rtc_init);
-module_exit(efi_rtc_exit);
-
+/*
MODULE_LICENSE("GPL");
+*/
--
2.5.0
The Kconfig for this driver is currently:
config SGI_SNSC
bool "SGI Altix system controller communication support"
...meaning that it currently is not being built as a module by anyone.
Lets remove all modular references, 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.
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/char/snsc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/char/snsc.c b/drivers/char/snsc.c
index 8a80ead8d316..94006f9c2e43 100644
--- a/drivers/char/snsc.c
+++ b/drivers/char/snsc.c
@@ -19,7 +19,7 @@
#include <linux/sched.h>
#include <linux/device.h>
#include <linux/poll.h>
-#include <linux/module.h>
+#include <linux/init.h>
#include <linux/slab.h>
#include <linux/mutex.h>
#include <asm/sn/io.h>
@@ -461,5 +461,4 @@ scdrv_init(void)
}
return 0;
}
-
-module_init(scdrv_init);
+device_initcall(scdrv_init);
--
2.5.0
The Kconfig currently controlling compilation of this code is:
char/Kconfig:config HPET
char/Kconfig: bool "HPET - High Precision Event Timer" if (X86 || IA64)
...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.
Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.
We leave some tags like MODULE_AUTHOR for documentation purposes.
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Clemens Ladisch <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/char/hpet.c | 25 +++----------------------
1 file changed, 3 insertions(+), 22 deletions(-)
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index 5c0baa9ffc64..240b6cf1d97c 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -12,7 +12,6 @@
*/
#include <linux/interrupt.h>
-#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/types.h>
#include <linux/miscdevice.h>
@@ -1043,24 +1042,16 @@ static int hpet_acpi_add(struct acpi_device *device)
return hpet_alloc(&data);
}
-static int hpet_acpi_remove(struct acpi_device *device)
-{
- /* XXX need to unregister clocksource, dealloc mem, etc */
- return -EINVAL;
-}
-
static const struct acpi_device_id hpet_device_ids[] = {
{"PNP0103", 0},
{"", 0},
};
-MODULE_DEVICE_TABLE(acpi, hpet_device_ids);
static struct acpi_driver hpet_acpi_driver = {
.name = "hpet",
.ids = hpet_device_ids,
.ops = {
.add = hpet_acpi_add,
- .remove = hpet_acpi_remove,
},
};
@@ -1086,19 +1077,9 @@ static int __init hpet_init(void)
return 0;
}
+device_initcall(hpet_init);
-static void __exit hpet_exit(void)
-{
- acpi_bus_unregister_driver(&hpet_acpi_driver);
-
- if (sysctl_header)
- unregister_sysctl_table(sysctl_header);
- misc_deregister(&hpet_misc);
-
- return;
-}
-
-module_init(hpet_init);
-module_exit(hpet_exit);
+/*
MODULE_AUTHOR("Bob Picco <[email protected]>");
MODULE_LICENSE("GPL");
+*/
--
2.5.0
The Kconfig currently controlling compilation of this code is:
lib/Kconfig.kgdb:config KGDB_TESTS
lib/Kconfig.kgdb: bool "KGDB: internal test suite"
...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.
We can't remove the module.h include since we've kept the use of
module_param in this file for now.
Cc: Jason Wessel <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/misc/kgdbts.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
index 9a60bd4d3c49..99635dd9dbac 100644
--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -1112,6 +1112,7 @@ static int __init init_kgdbts(void)
return configure_kgdbts();
}
+device_initcall(init_kgdbts);
static int kgdbts_get_char(void)
{
@@ -1180,10 +1181,9 @@ static struct kgdb_io kgdbts_io_ops = {
.post_exception = kgdbts_post_exp_handler,
};
-module_init(init_kgdbts);
+/*
+ * not really modular, but the easiest way to keep compat with existing
+ * bootargs behaviour is to continue using module_param here.
+ */
module_param_call(kgdbts, param_set_kgdbts_var, param_get_string, &kps, 0644);
MODULE_PARM_DESC(kgdbts, "<A|V1|V2>[F#|S#][N#]");
-MODULE_DESCRIPTION("KGDB Test Suite");
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Wind River Systems, Inc.");
-
--
2.5.0
On 08/08/2015 03:35 PM, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
>
> lib/Kconfig.kgdb:config KGDB_TESTS
> lib/Kconfig.kgdb: bool "KGDB: internal test suite"
>
> ...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.
>
> We can't remove the module.h include since we've kept the use of
> module_param in this file for now.
This is correct, if you remove that there is no way to invoke the test suite later on at run time.
I tried out the patch and it works fine with no regressions.
Unrelated to this it seems there is a problem with the read/write of the break points when crossing the point where the kernel write protects the read-only data. Basically, the break point is implanted into the code page, which is made read-only, and then it can no longer be removed. What we typically do in that case "after read-only pages are established", is to use COW pages for the break points. I'll have to look further into what if anything we might have to do about it. At least the emergency printk logic worked to show there is a problem. :-)
Acked-by: Jason Wessel <[email protected]>