This series of commits is a slice of a larger project to ensure
people don't needlessly use modular support functions in non-modular
code. Overall there was roughly 5k lines of unused _exit code and
".remove" functions in the kernel due to this. So far we've fixed
several areas, like tty, x86, net, etc. and we continue here in fs/
There are several reasons to not use module helpers for code that can
never be built as a module, but the big ones are:
(1) it is easy to accidentally code up an unused module_exit function
(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 increasing CPP overhead.
Fortunately the code here is core fs code and not strictly a driver
in the sense that a UART or GPIO driver is. So we don't have a lot
of unused code to remove, and mainly gain in avoiding #2 and #3 above.
Here we convert some module_init() calls into fs_initcall(). In doing
so we must note that this changes the init ordering slightly, since
module_init() becomes device_initcall() in the non-modular case, and
that comes after fs_initcall().
We could have used device_initcall here to strictly preserve the old
ordering, and that largely makes sense for drivers/* dirs. But using
device_initcall in the fs/ dir just seems wrong when we have the staged
initcall system and a fs_initcall bucket in that tiered system.
The hugetlb patch warrants special mention. It has code in both the
fs dir and the mm dir, with initcalls in each. There is an implicit
requirement that the mm one be executed prior to the fs one, else
the runtime will splat. Currently we achieve that only by luck of the
link order. With the changes made here, we use our existing initcall
buckets properly to guarantee that ordering.
Since I have a large queue, I'm hoping to get as many of these as
possible in via maintainers, so that I'm not left someday asking
Linus to pull a giant series.
[v1 --> v2: drop 2 patches merged elsewhere, combine mm & fs chunks
of hugetlb patch into one & update log accordingly.]
---
Cc: Al Viro <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: David Howells <[email protected]>
Cc: Davidlohr Bueso <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Eric Paris <[email protected]>
Cc: Hillf Danton <[email protected]>
Cc: "J. Bruce Fields" <[email protected]>
Cc: Jeff Layton <[email protected]>
Cc: Josh Triplett <[email protected]>
Cc: Mike Kravetz <[email protected]>
Cc: Nadia Yvette Chambers <[email protected]>
Cc: Naoya Horiguchi <[email protected]>
Cc: Peter Hurley <[email protected]>
Cc: [email protected]
Cc: [email protected]
The following changes since commit 9f9499ae8e6415cefc4fe0a96ad0e27864353c89:
Linux 4.4-rc5 (2015-12-13 17:42:58 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux.git fs_initcall
for you to fetch changes up to 7763d3b42ca62dc967cc56218df8007401e63cd0:
fs: make binfmt_elf.c explicitly non-modular (2015-12-17 12:11:19 -0500)
----------------------------------------------------------------
Paul Gortmaker (8):
hugetlb: make mm and fs code explicitly non-modular
fs: make notify dnotify.c explicitly non-modular
fs: make fcntl.c explicitly non-modular
fs: make filesystems.c explicitly non-modular
fs: make locks.c explicitly non-modular
fs: make direct-io.c explicitly non-modular
fs: make devpts/inode.c explicitly non-modular
fs: make binfmt_elf.c explicitly non-modular
fs/binfmt_elf.c | 11 +----------
fs/devpts/inode.c | 3 +--
fs/direct-io.c | 4 ++--
fs/fcntl.c | 4 +---
fs/filesystems.c | 2 +-
fs/hugetlbfs/inode.c | 27 ++-------------------------
fs/locks.c | 3 +--
fs/notify/dnotify/dnotify.c | 4 +---
mm/hugetlb.c | 39 +--------------------------------------
9 files changed, 11 insertions(+), 86 deletions(-)
--
2.6.1
The Kconfig currently controlling compilation of this code is:
config HUGETLBFS
bool "HugeTLB file system support"
...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 gets moved to earlier levels when we use the
more appropriate initcalls here.
Originally I had the fs part and the mm part as separate commits,
just by happenstance of the nature of how I detected these
non-modular use cases. But that can possibly introduce regressions
if the patch merge ordering puts the fs part 1st -- as the 0-day
testing reported a splat at mount time.
Investigating with "initcall_debug" showed that the delta was
init_hugetlbfs_fs being called _before_ hugetlb_init instead of
after. So both the fs change and the mm change are here together.
In addition, it worked before due to luck of link order, since they
were both in the same initcall category. So we now have the fs
part using fs_initcall, and the mm part using subsys_initcall,
which puts it one bucket earlier. It now passes the basic sanity
test that failed in earlier 0-day testing.
We delete the MODULE_LICENSE tag and capture that information at the
top of the file alongside author comments, etc.
We don't replace module.h with init.h since the file already has that.
Also note that MODULE_ALIAS is a no-op for non-modular code.
Cc: Nadia Yvette Chambers <[email protected]>
Cc: Alexander Viro <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Naoya Horiguchi <[email protected]>
Cc: Mike Kravetz <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Hillf Danton <[email protected]>
Cc: Davidlohr Bueso <[email protected]>
Cc: [email protected]
Cc: [email protected]
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
fs/hugetlbfs/inode.c | 27 ++-------------------------
mm/hugetlb.c | 39 +--------------------------------------
2 files changed, 3 insertions(+), 63 deletions(-)
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index de4bdfac0cec..dd04c2ad23b3 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -4,11 +4,11 @@
* Nadia Yvette Chambers, 2002
*
* Copyright (C) 2002 Linus Torvalds.
+ * License: GPL
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include <linux/module.h>
#include <linux/thread_info.h>
#include <asm/current.h>
#include <linux/sched.h> /* remove ASAP */
@@ -1201,7 +1201,6 @@ static struct file_system_type hugetlbfs_fs_type = {
.mount = hugetlbfs_mount,
.kill_sb = kill_litter_super,
};
-MODULE_ALIAS_FS("hugetlbfs");
static struct vfsmount *hugetlbfs_vfsmount[HUGE_MAX_HSTATE];
@@ -1355,26 +1354,4 @@ static int __init init_hugetlbfs_fs(void)
out2:
return error;
}
-
-static void __exit exit_hugetlbfs_fs(void)
-{
- struct hstate *h;
- int i;
-
-
- /*
- * Make sure all delayed rcu free inodes are flushed before we
- * destroy cache.
- */
- rcu_barrier();
- kmem_cache_destroy(hugetlbfs_inode_cachep);
- i = 0;
- for_each_hstate(h)
- kern_unmount(hugetlbfs_vfsmount[i++]);
- unregister_filesystem(&hugetlbfs_fs_type);
-}
-
-module_init(init_hugetlbfs_fs)
-module_exit(exit_hugetlbfs_fs)
-
-MODULE_LICENSE("GPL");
+fs_initcall(init_hugetlbfs_fs)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index ef6963b577fd..be934df69b85 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4,7 +4,6 @@
*/
#include <linux/list.h>
#include <linux/init.h>
-#include <linux/module.h>
#include <linux/mm.h>
#include <linux/seq_file.h>
#include <linux/sysctl.h>
@@ -2549,25 +2548,6 @@ static void hugetlb_unregister_node(struct node *node)
nhs->hugepages_kobj = NULL;
}
-/*
- * hugetlb module exit: unregister hstate attributes from node devices
- * that have them.
- */
-static void hugetlb_unregister_all_nodes(void)
-{
- int nid;
-
- /*
- * disable node device registrations.
- */
- register_hugetlbfs_with_node(NULL, NULL);
-
- /*
- * remove hstate attributes from any nodes that have them.
- */
- for (nid = 0; nid < nr_node_ids; nid++)
- hugetlb_unregister_node(node_devices[nid]);
-}
/*
* Register hstate attributes for a single node device.
@@ -2632,27 +2612,10 @@ static struct hstate *kobj_to_node_hstate(struct kobject *kobj, int *nidp)
return NULL;
}
-static void hugetlb_unregister_all_nodes(void) { }
-
static void hugetlb_register_all_nodes(void) { }
#endif
-static void __exit hugetlb_exit(void)
-{
- struct hstate *h;
-
- hugetlb_unregister_all_nodes();
-
- for_each_hstate(h) {
- kobject_put(hstate_kobjs[hstate_index(h)]);
- }
-
- kobject_put(hugepages_kobj);
- kfree(hugetlb_fault_mutex_table);
-}
-module_exit(hugetlb_exit);
-
static int __init hugetlb_init(void)
{
int i;
@@ -2690,7 +2653,7 @@ static int __init hugetlb_init(void)
mutex_init(&hugetlb_fault_mutex_table[i]);
return 0;
}
-module_init(hugetlb_init);
+subsys_initcall(hugetlb_init);
/* Should be called on processing a hugepagesz=... option */
void __init hugetlb_add_hstate(unsigned int order)
--
2.6.1
The Kconfig currently controlling compilation of this code is:
config DNOTIFY
bool "Dnotify support"
...meaning that it currently is not being built as a module by anyone.
Lets remove the couple traces of modularity so that when reading the
code there is no doubt it is builtin-only.
Since module_init translates to device_initcall in the non-modular
case, the init ordering gets bumped to one level earlier when we
use the more appropriate fs_initcall here. However we've made similar
changes before without any fallout and none is expected here either.
We don't replace module.h with init.h since the file already has that.
Cc: Eric Paris <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
fs/notify/dnotify/dnotify.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c
index 6faaf710e563..f67c82e72617 100644
--- a/fs/notify/dnotify/dnotify.c
+++ b/fs/notify/dnotify/dnotify.c
@@ -17,7 +17,6 @@
* General Public License for more details.
*/
#include <linux/fs.h>
-#include <linux/module.h>
#include <linux/sched.h>
#include <linux/dnotify.h>
#include <linux/init.h>
@@ -390,5 +389,4 @@ static int __init dnotify_init(void)
panic("unable to allocate fsnotify group for dnotify\n");
return 0;
}
-
-module_init(dnotify_init)
+fs_initcall(dnotify_init)
--
2.6.1
The Makefile currently controlling compilation of this code is obj-y
meaning that it currently is not being built as a module by anyone.
Lets remove the couple traces of modularity so that when reading the
code there is no doubt it is builtin-only.
Since module_init translates to device_initcall in the non-modular
case, the init ordering gets bumped to one level earlier when we
use the more appropriate fs_initcall here. However we've made similar
changes before without any fallout and none is expected here either.
We don't replace module.h with init.h since the file already has that.
Cc: Alexander Viro <[email protected]>
Cc: Jeff Layton <[email protected]>
Cc: "J. Bruce Fields" <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
fs/fcntl.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c
index ee85cd4e136a..b485864eb357 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -13,7 +13,6 @@
#include <linux/capability.h>
#include <linux/dnotify.h>
#include <linux/slab.h>
-#include <linux/module.h>
#include <linux/pipe_fs_i.h>
#include <linux/security.h>
#include <linux/ptrace.h>
@@ -755,5 +754,4 @@ static int __init fcntl_init(void)
sizeof(struct fasync_struct), 0, SLAB_PANIC, NULL);
return 0;
}
-
-module_init(fcntl_init)
+fs_initcall(fcntl_init)
--
2.6.1
The Makefile currently controlling compilation of this code is obj-y
meaning that it currently is not being built as a module by anyone.
Lets remove the traces of modularity that we can so that when reading
the code there is no doubt it is builtin-only.
Since module_init translates to device_initcall in the non-modular
case, the init ordering gets bumped to one level earlier when we
use the more appropriate fs_initcall here. However we've made similar
changes before without any fallout and none is expected here either.
We can't remove module.h from the file as we've done in similar
cleanups, since other code in the file is using module functions
to try and load filesystem drivers etc.
Cc: Alexander Viro <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
fs/filesystems.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/filesystems.c b/fs/filesystems.c
index 5797d45a78cb..3bca451b1e80 100644
--- a/fs/filesystems.c
+++ b/fs/filesystems.c
@@ -253,7 +253,7 @@ static int __init proc_filesystems_init(void)
proc_create("filesystems", 0, NULL, &filesystems_proc_fops);
return 0;
}
-module_init(proc_filesystems_init);
+fs_initcall(proc_filesystems_init);
#endif
static struct file_system_type *__get_fs_type(const char *name, int len)
--
2.6.1
The Kconfig currently controlling compilation of this code is:
config FILE_LOCKING
bool "Enable POSIX file locking API" if EXPERT
...meaning that it currently is not being built as a module by anyone.
Lets remove the couple traces of modularity 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 gets bumped to one level earlier when we
use the more appropriate fs_initcall here. However we've made similar
changes before without any fallout and none is expected here either.
Cc: Jeff Layton <[email protected]>
Acked-by: Jeff Layton <[email protected]>
Cc: "J. Bruce Fields" <[email protected]>
Cc: Alexander Viro <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
fs/locks.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c
index 0d2b3267e2a3..869b64418a5b 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -119,7 +119,6 @@
#include <linux/fdtable.h>
#include <linux/fs.h>
#include <linux/init.h>
-#include <linux/module.h>
#include <linux/security.h>
#include <linux/slab.h>
#include <linux/syscalls.h>
@@ -2706,7 +2705,7 @@ static int __init proc_locks_init(void)
proc_create("locks", 0, NULL, &proc_locks_operations);
return 0;
}
-module_init(proc_locks_init);
+fs_initcall(proc_locks_init);
#endif
static int __init filelock_init(void)
--
2.6.1
The Makefile currently controlling compilation of this code has:
ifeq ($(CONFIG_BLOCK),y)
obj-y += buffer.o block_dev.o direct-io.o mpage.o
and in addition to that the Kconfig is:
block/Kconfig:menuconfig BLOCK
block/Kconfig: bool "Enable the block layer" if EXPERT
...meaning that it currently is not being built as a module by anyone.
Lets remove the couple traces of modularity so that when reading the
code there is no doubt it is builtin-only.
Since module_init translates to device_initcall in the non-modular
case, the init ordering gets bumped to one level earlier when we
use the more appropriate fs_initcall here. However we've made similar
changes before without any fallout and none is expected here either.
Cc: Alexander Viro <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
fs/direct-io.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/direct-io.c b/fs/direct-io.c
index 602e8441bc0f..a7d7b466f00e 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -20,7 +20,7 @@
*/
#include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/init.h>
#include <linux/types.h>
#include <linux/fs.h>
#include <linux/mm.h>
@@ -1349,4 +1349,4 @@ static __init int dio_init(void)
dio_cache = KMEM_CACHE(dio, SLAB_PANIC);
return 0;
}
-module_init(dio_init)
+fs_initcall(dio_init)
--
2.6.1
The Kconfig currently controlling compilation of this code is:
config UNIX98_PTYS
bool "Unix98 PTY support" if EXPERT
...meaning that it currently is not being built as a module by anyone.
Lets remove the couple traces of modularity 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 gets bumped to one level earlier when we
use the more appropriate fs_initcall here. However we've made similar
changes before without any fallout and none is expected here either.
Cc: Al Viro <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Peter Hurley <[email protected]>
Cc: Josh Triplett <[email protected]>
Cc: David Howells <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
fs/devpts/inode.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index c35ffdc12bba..3c89b1f0804f 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -12,7 +12,6 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include <linux/module.h>
#include <linux/init.h>
#include <linux/fs.h>
#include <linux/sched.h>
@@ -703,4 +702,4 @@ static int __init init_devpts_fs(void)
}
return err;
}
-module_init(init_devpts_fs)
+fs_initcall(init_devpts_fs)
--
2.6.1
The Kconfig currently controlling compilation of this code is:
config BINFMT_ELF
bool "Kernel support for ELF binaries"
...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 this file was already using core_initcall and not module_init,
we have no changes in initcall ordering with this commit.
We don't replace module.h with init.h since the file already has that.
We delete the MODULE_LICENSE tag and capture that information at the
top of the file alongside author comments, etc.
Cc: Alexander Viro <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
fs/binfmt_elf.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 3a93755e880f..c26450ffbdc9 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -7,9 +7,9 @@
* Tools".
*
* Copyright 1993, 1994: Eric Youngdale ([email protected]).
+ * License: GPL
*/
-#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/fs.h>
#include <linux/mm.h>
@@ -2324,13 +2324,4 @@ static int __init init_elf_binfmt(void)
register_binfmt(&elf_format);
return 0;
}
-
-static void __exit exit_elf_binfmt(void)
-{
- /* Remove the COFF and ELF loaders. */
- unregister_binfmt(&elf_format);
-}
-
core_initcall(init_elf_binfmt);
-module_exit(exit_elf_binfmt);
-MODULE_LICENSE("GPL");
--
2.6.1
Hi Paul,
On 12/17/2015 11:11 AM, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
>
> config UNIX98_PTYS
> bool "Unix98 PTY support" if EXPERT
>
> ...meaning that it currently is not being built as a module by anyone.
>
> Lets remove the couple traces of modularity 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 gets bumped to one level earlier when we
> use the more appropriate fs_initcall here. However we've made similar
> changes before without any fallout and none is expected here either.
There's a slim possibility moving the devpts init up to fs level
(where it belongs) may impact certain broken userspace setups, since the
system devpts instance would now always be mounted before initramfs.
I'm still waiting to receive a reply back from Eric Biederman about
that userspace configuration, so I don't have a definitive answer on
whether this patch will break that setup.
Regards,
Peter Hurley
> Cc: Al Viro <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Peter Hurley <[email protected]>
> Cc: Josh Triplett <[email protected]>
> Cc: David Howells <[email protected]>
> Signed-off-by: Paul Gortmaker <[email protected]>
> ---
> fs/devpts/inode.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
> index c35ffdc12bba..3c89b1f0804f 100644
> --- a/fs/devpts/inode.c
> +++ b/fs/devpts/inode.c
> @@ -12,7 +12,6 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> -#include <linux/module.h>
> #include <linux/init.h>
> #include <linux/fs.h>
> #include <linux/sched.h>
> @@ -703,4 +702,4 @@ static int __init init_devpts_fs(void)
> }
> return err;
> }
> -module_init(init_devpts_fs)
> +fs_initcall(init_devpts_fs)
>
On 12/17/2015 11:10 AM, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
>
> config HUGETLBFS
> bool "HugeTLB file system support"
>
> ...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 gets moved to earlier levels when we use the
> more appropriate initcalls here.
>
> Originally I had the fs part and the mm part as separate commits,
> just by happenstance of the nature of how I detected these
> non-modular use cases. But that can possibly introduce regressions
> if the patch merge ordering puts the fs part 1st -- as the 0-day
> testing reported a splat at mount time.
>
> Investigating with "initcall_debug" showed that the delta was
> init_hugetlbfs_fs being called _before_ hugetlb_init instead of
> after. So both the fs change and the mm change are here together.
>
> In addition, it worked before due to luck of link order, since they
> were both in the same initcall category. So we now have the fs
> part using fs_initcall, and the mm part using subsys_initcall,
> which puts it one bucket earlier. It now passes the basic sanity
> test that failed in earlier 0-day testing.
>
> We delete the MODULE_LICENSE tag and capture that information at the
> top of the file alongside author comments, etc.
>
> We don't replace module.h with init.h since the file already has that.
> Also note that MODULE_ALIAS is a no-op for non-modular code.
>
> Cc: Nadia Yvette Chambers <[email protected]>
> Cc: Alexander Viro <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Naoya Horiguchi <[email protected]>
> Cc: Mike Kravetz <[email protected]>
> Cc: David Rientjes <[email protected]>
> Cc: Hillf Danton <[email protected]>
> Cc: Davidlohr Bueso <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Paul Gortmaker <[email protected]>
> ---
> fs/hugetlbfs/inode.c | 27 ++-------------------------
> mm/hugetlb.c | 39 +--------------------------------------
> 2 files changed, 3 insertions(+), 63 deletions(-)
>
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index de4bdfac0cec..dd04c2ad23b3 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -4,11 +4,11 @@
> * Nadia Yvette Chambers, 2002
> *
> * Copyright (C) 2002 Linus Torvalds.
> + * License: GPL
> */
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> -#include <linux/module.h>
> #include <linux/thread_info.h>
> #include <asm/current.h>
> #include <linux/sched.h> /* remove ASAP */
> @@ -1201,7 +1201,6 @@ static struct file_system_type hugetlbfs_fs_type = {
> .mount = hugetlbfs_mount,
> .kill_sb = kill_litter_super,
> };
> -MODULE_ALIAS_FS("hugetlbfs");
>
> static struct vfsmount *hugetlbfs_vfsmount[HUGE_MAX_HSTATE];
>
> @@ -1355,26 +1354,4 @@ static int __init init_hugetlbfs_fs(void)
> out2:
> return error;
> }
> -
> -static void __exit exit_hugetlbfs_fs(void)
> -{
> - struct hstate *h;
> - int i;
> -
> -
> - /*
> - * Make sure all delayed rcu free inodes are flushed before we
> - * destroy cache.
> - */
> - rcu_barrier();
> - kmem_cache_destroy(hugetlbfs_inode_cachep);
> - i = 0;
> - for_each_hstate(h)
> - kern_unmount(hugetlbfs_vfsmount[i++]);
> - unregister_filesystem(&hugetlbfs_fs_type);
> -}
> -
> -module_init(init_hugetlbfs_fs)
> -module_exit(exit_hugetlbfs_fs)
> -
> -MODULE_LICENSE("GPL");
> +fs_initcall(init_hugetlbfs_fs)
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index ef6963b577fd..be934df69b85 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4,7 +4,6 @@
> */
> #include <linux/list.h>
> #include <linux/init.h>
> -#include <linux/module.h>
> #include <linux/mm.h>
> #include <linux/seq_file.h>
> #include <linux/sysctl.h>
> @@ -2549,25 +2548,6 @@ static void hugetlb_unregister_node(struct node *node)
> nhs->hugepages_kobj = NULL;
> }
>
> -/*
> - * hugetlb module exit: unregister hstate attributes from node devices
> - * that have them.
> - */
> -static void hugetlb_unregister_all_nodes(void)
> -{
> - int nid;
> -
> - /*
> - * disable node device registrations.
> - */
> - register_hugetlbfs_with_node(NULL, NULL);
> -
> - /*
> - * remove hstate attributes from any nodes that have them.
> - */
> - for (nid = 0; nid < nr_node_ids; nid++)
> - hugetlb_unregister_node(node_devices[nid]);
> -}
>
> /*
> * Register hstate attributes for a single node device.
> @@ -2632,27 +2612,10 @@ static struct hstate *kobj_to_node_hstate(struct kobject *kobj, int *nidp)
> return NULL;
> }
>
> -static void hugetlb_unregister_all_nodes(void) { }
> -
> static void hugetlb_register_all_nodes(void) { }
>
> #endif
>
> -static void __exit hugetlb_exit(void)
> -{
> - struct hstate *h;
> -
> - hugetlb_unregister_all_nodes();
> -
> - for_each_hstate(h) {
> - kobject_put(hstate_kobjs[hstate_index(h)]);
> - }
> -
> - kobject_put(hugepages_kobj);
> - kfree(hugetlb_fault_mutex_table);
> -}
> -module_exit(hugetlb_exit);
> -
> static int __init hugetlb_init(void)
> {
> int i;
> @@ -2690,7 +2653,7 @@ static int __init hugetlb_init(void)
> mutex_init(&hugetlb_fault_mutex_table[i]);
> return 0;
> }
> -module_init(hugetlb_init);
> +subsys_initcall(hugetlb_init);
>
> /* Should be called on processing a hugepagesz=... option */
> void __init hugetlb_add_hstate(unsigned int order)
>
I like the removal of code.
Reviewed-By: Mike Kravetz <[email protected]>
--
Mike Kravetz
On Thu, 17 Dec 2015, Paul Gortmaker wrote:
>The Kconfig currently controlling compilation of this code is:
>
>config HUGETLBFS
> bool "HugeTLB file system support"
>
>...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 gets moved to earlier levels when we use the
>more appropriate initcalls here.
>
>Originally I had the fs part and the mm part as separate commits,
>just by happenstance of the nature of how I detected these
>non-modular use cases. But that can possibly introduce regressions
>if the patch merge ordering puts the fs part 1st -- as the 0-day
>testing reported a splat at mount time.
>
>Investigating with "initcall_debug" showed that the delta was
>init_hugetlbfs_fs being called _before_ hugetlb_init instead of
>after. So both the fs change and the mm change are here together.
>
>In addition, it worked before due to luck of link order, since they
>were both in the same initcall category. So we now have the fs
>part using fs_initcall, and the mm part using subsys_initcall,
>which puts it one bucket earlier. It now passes the basic sanity
>test that failed in earlier 0-day testing.
>
>We delete the MODULE_LICENSE tag and capture that information at the
>top of the file alongside author comments, etc.
>
>We don't replace module.h with init.h since the file already has that.
>Also note that MODULE_ALIAS is a no-op for non-modular code.
>
>Cc: Nadia Yvette Chambers <[email protected]>
>Cc: Alexander Viro <[email protected]>
>Cc: Andrew Morton <[email protected]>
>Cc: Naoya Horiguchi <[email protected]>
>Cc: Mike Kravetz <[email protected]>
>Cc: David Rientjes <[email protected]>
>Cc: Hillf Danton <[email protected]>
>Cc: Davidlohr Bueso <[email protected]>
>Cc: [email protected]
>Cc: [email protected]
>Reported-by: kernel test robot <[email protected]>
>Signed-off-by: Paul Gortmaker <[email protected]>
Acked-by: Davidlohr Bueso <[email protected]>
On Thu, 17 Dec 2015 14:11:03 -0500
Paul Gortmaker <[email protected]> wrote:
> The Kconfig currently controlling compilation of this code is:
>
> config FILE_LOCKING
> bool "Enable POSIX file locking API" if EXPERT
>
> ...meaning that it currently is not being built as a module by anyone.
>
> Lets remove the couple traces of modularity 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 gets bumped to one level earlier when we
> use the more appropriate fs_initcall here. However we've made similar
> changes before without any fallout and none is expected here either.
>
> Cc: Jeff Layton <[email protected]>
> Acked-by: Jeff Layton <[email protected]>
> Cc: "J. Bruce Fields" <[email protected]>
> Cc: Alexander Viro <[email protected]>
> Cc: [email protected]
> Signed-off-by: Paul Gortmaker <[email protected]>
> ---
> fs/locks.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/locks.c b/fs/locks.c
> index 0d2b3267e2a3..869b64418a5b 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -119,7 +119,6 @@
> #include <linux/fdtable.h>
> #include <linux/fs.h>
> #include <linux/init.h>
> -#include <linux/module.h>
> #include <linux/security.h>
> #include <linux/slab.h>
> #include <linux/syscalls.h>
> @@ -2706,7 +2705,7 @@ static int __init proc_locks_init(void)
> proc_create("locks", 0, NULL, &proc_locks_operations);
> return 0;
> }
> -module_init(proc_locks_init);
> +fs_initcall(proc_locks_init);
> #endif
>
> static int __init filelock_init(void)
Thanks Paul, I'll pick this one up for v4.5.
--
Jeff Layton <[email protected]>
[Re: [PATCH 7/8] fs: make devpts/inode.c explicitly non-modular] On 17/12/2015 (Thu 11:46) Peter Hurley wrote:
> Hi Paul,
>
> On 12/17/2015 11:11 AM, Paul Gortmaker wrote:
> > The Kconfig currently controlling compilation of this code is:
> >
> > config UNIX98_PTYS
> > bool "Unix98 PTY support" if EXPERT
> >
> > ...meaning that it currently is not being built as a module by anyone.
> >
> > Lets remove the couple traces of modularity 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 gets bumped to one level earlier when we
> > use the more appropriate fs_initcall here. However we've made similar
> > changes before without any fallout and none is expected here either.
>
> There's a slim possibility moving the devpts init up to fs level
> (where it belongs) may impact certain broken userspace setups, since the
> system devpts instance would now always be mounted before initramfs.
>
> I'm still waiting to receive a reply back from Eric Biederman about
> that userspace configuration, so I don't have a definitive answer on
> whether this patch will break that setup.
I did not see any problems when using what amounts to using a pretty
standard Ubuntu rootfs. Let me know if there is some other user space
situation I should be looking at. And thanks for the basic ack that
generally overall we should be using the initlevels we created that
obviously map to where certain things belong, like fs in this case.
Paul.
--
>
> Regards,
> Peter Hurley
>
> > Cc: Al Viro <[email protected]>
> > Cc: Andrew Morton <[email protected]>
> > Cc: Peter Hurley <[email protected]>
> > Cc: Josh Triplett <[email protected]>
> > Cc: David Howells <[email protected]>
> > Signed-off-by: Paul Gortmaker <[email protected]>
> > ---
> > fs/devpts/inode.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
> > index c35ffdc12bba..3c89b1f0804f 100644
> > --- a/fs/devpts/inode.c
> > +++ b/fs/devpts/inode.c
> > @@ -12,7 +12,6 @@
> >
> > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >
> > -#include <linux/module.h>
> > #include <linux/init.h>
> > #include <linux/fs.h>
> > #include <linux/sched.h>
> > @@ -703,4 +702,4 @@ static int __init init_devpts_fs(void)
> > }
> > return err;
> > }
> > -module_init(init_devpts_fs)
> > +fs_initcall(init_devpts_fs)
> >
>