There are several reasons to not use module_init 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.
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, but using that in the fs/ dir just seems wrong, and we have
done similar minor init ordering shuffles in the past w/o fallout.
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 the
concerns here with respect to limiting unbinding that we have when
doing similar cleanups over there in the drivers/* dir.
These commits were generated and tested on linux-next but I can put
them on a v4.4-rc4 based branch if merge processing vs mail processing
is desired. Given that they are largely trivial, I don't think the
underlying baseline is all that important here.
Cc: Al Viro <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: David Howells <[email protected]>
Cc: Eric Paris <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: "J. Bruce Fields" <[email protected]>
Cc: Jeff Layton <[email protected]>
Cc: Josh Triplett <[email protected]>
Cc: [email protected]
Cc: Nadia Yvette Chambers <[email protected]>
Cc: Peter Hurley <[email protected]>
Paul Gortmaker (10):
fs: make notify dnotify.c explicitly non-modular
fs: make quota/netlink.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: make hugetlbfs/inode.c explicitly non-modular
fs: make quota/dquot.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 +---
fs/quota/dquot.c | 2 +-
fs/quota/netlink.c | 5 +----
10 files changed, 12 insertions(+), 53 deletions(-)
--
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 Kconfig currently controlling compilation of this code is:
config QUOTA_NETLINK_INTERFACE
bool "Report quota messages through netlink interface"
...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: Jan Kara <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
fs/quota/netlink.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/quota/netlink.c b/fs/quota/netlink.c
index bb2869f5dfd8..d07a2f91d858 100644
--- a/fs/quota/netlink.c
+++ b/fs/quota/netlink.c
@@ -1,7 +1,5 @@
-
#include <linux/cred.h>
#include <linux/init.h>
-#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/quotaops.h>
#include <linux/sched.h>
@@ -105,5 +103,4 @@ static int __init quota_init(void)
"VFS: Failed to create quota netlink interface.\n");
return 0;
};
-
-module_init(quota_init);
+fs_initcall(quota_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]>
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 fa76eb2910a9..15e2b60aa2d1 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>
@@ -2702,7 +2701,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
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 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 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: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
fs/hugetlbfs/inode.c | 27 ++-------------------------
1 file changed, 2 insertions(+), 25 deletions(-)
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index a1cb8fd2289b..47789292a582 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 */
@@ -1202,7 +1202,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];
@@ -1356,26 +1355,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)
--
2.6.1
The Kconfig currently controlling compilation of this code is:
config QUOTA
bool "Quota 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
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.
We don't delete module.h because the code in turn tries to load other
modules as appropriate and so it still needs that header.
Cc: Jan Kara <[email protected]>
Cc: Alexander Viro <[email protected]>
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
---
fs/quota/dquot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index ef0d64b2a6d9..fbd70af98820 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -2924,4 +2924,4 @@ static int __init dquot_init(void)
return 0;
}
-module_init(dquot_init);
+fs_initcall(dquot_init);
--
2.6.1
On Sat 12-12-15 16:30:04, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
>
> config QUOTA_NETLINK_INTERFACE
> bool "Report quota messages through netlink interface"
>
> ...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: Jan Kara <[email protected]>
> Signed-off-by: Paul Gortmaker <[email protected]>
Looks good. I have taken the patch into my tree.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Sat 12-12-15 16:30:12, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
>
> config QUOTA
> bool "Quota 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
> 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.
>
> We don't delete module.h because the code in turn tries to load other
> modules as appropriate and so it still needs that header.
Looks good. I have taken the patch to my tree.
Honza
>
> Cc: Jan Kara <[email protected]>
> Cc: Alexander Viro <[email protected]>
> Cc: [email protected]
> Signed-off-by: Paul Gortmaker <[email protected]>
> ---
> fs/quota/dquot.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index ef0d64b2a6d9..fbd70af98820 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -2924,4 +2924,4 @@ static int __init dquot_init(void)
>
> return 0;
> }
> -module_init(dquot_init);
> +fs_initcall(dquot_init);
> --
> 2.6.1
>
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Sat, 12 Dec 2015 16:30:07 -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]>
> 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 fa76eb2910a9..15e2b60aa2d1 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>
> @@ -2702,7 +2701,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)
Looks fine to me and I doubt we'll see any merge conflicts with
anything I have queued so far. Do you need any of us to pick any of
these up or are you going to be merging them as a set?
Acked-by: Jeff Layton <[email protected]>
[Re: [PATCH 05/10] fs: make locks.c explicitly non-modular] On 14/12/2015 (Mon 06:31) Jeff Layton wrote:
> On Sat, 12 Dec 2015 16:30:07 -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.
> >
[...]
>
> Looks fine to me and I doubt we'll see any merge conflicts with
> anything I have queued so far. Do you need any of us to pick any of
> these up or are you going to be merging them as a set?
I was hoping to spread as many of these around as possible so I don't
end up with a giant pull request to Linus. There is code out there
without a clear maintainership path, so eventually I'll have to send
some his way (or via akpm) but the less that end up in that pile, the
better IMHO.
It looks like the hugetlb init level change upset one of the automated
qemu 0-day boot tests, so I need to investigate that next.
Paul.
--
>
> Acked-by: Jeff Layton <[email protected]>
[[PATCH 09/10] fs: make hugetlbfs/inode.c explicitly non-modular] On 12/12/2015 (Sat 16:30) 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 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.
Famous last words. Turns out the 0-day builder found some fallout from
what is presumably the initlevel change in one of its qemu boot tests,
as per the snippet below that didn't show up in my testing.
So we can't use this one as-is. Currently investigating the implicit
initlevel dependency here to understand why it happens...
Paul.
--
BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
IP: [<ffffffff928af8d3>] __lock_acquire+0x160/0x1a3f
PGD 1b6a7067 PUD 1b711067 PMD 0
Oops: 0000 [#1] PREEMPT DEBUG_PAGEALLOC
Modules linked in:
CPU: 0 PID: 139 Comm: mount Not tainted 4.4.0-rc4-00009-g2e9d30b #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014
task: ffff88001bcc0000 ti: ffff88001b720000 task.ti: ffff88001b720000
RIP: 0010:[<ffffffff928af8d3>] [<ffffffff928af8d3>] __lock_acquire+0x160/0x1a3f
RSP: 0018:ffff88001b723890 EFLAGS: 00010002
RAX: 0000000000000046 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000018
RBP: ffff88001b723960 R08: 0000000000000001 R09: 0000000000000001
R10: ffff88001b723980 R11: 0000000000001e27 R12: 0000000000000000
R13: ffff88001bcc0000 R14: 0000000000000018 R15: 0000000000000001
FS: 00007f77a46bb840(0000) GS:ffffffff93220000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000018 CR3: 000000001b703000 CR4: 00000000000006f0
Stack:
0000000000000400 0000000000000002 ffff88001bcc0000 0000000000000002
ffffffff9297f5b7 0000000100000000 ffff880000000000 0000000000000000
ffff88001bcc0000 ffff880000000000 ffffffff928ae160 ffff88001bcc0000
Call Trace:
[<ffffffff9297f5b7>] ? deactivate_slab+0x41c/0x919
[<ffffffff928ae160>] ? mark_held_locks+0x5e/0x74
[<ffffffff9293b230>] ? get_page_from_freelist+0x819/0xd4f
[<ffffffff928b16f8>] lock_acquire+0x121/0x1c2
[<ffffffff928b16f8>] ? lock_acquire+0x121/0x1c2
[<ffffffff9297f5b7>] ? deactivate_slab+0x41c/0x919
[<ffffffff92de0cf2>] _raw_spin_lock+0x32/0x41
[<ffffffff9297f5b7>] ? deactivate_slab+0x41c/0x919
[<ffffffff9297f5b7>] deactivate_slab+0x41c/0x919
[<ffffffff92806479>] ? print_context_stack+0x6a/0xb6
[<ffffffff928a1b5e>] ? preempt_count_sub+0x34/0x3f
[<ffffffff92805e94>] ? dump_trace+0x28a/0x2a2
[<ffffffff929ffe1f>] ? hugetlbfs_alloc_inode+0x99/0xcc
>
> 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: [email protected]
> Signed-off-by: Paul Gortmaker <[email protected]>
> ---
> fs/hugetlbfs/inode.c | 27 ++-------------------------
> 1 file changed, 2 insertions(+), 25 deletions(-)
>
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index a1cb8fd2289b..47789292a582 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 */
> @@ -1202,7 +1202,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];
>
> @@ -1356,26 +1355,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)
> --
> 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]>
---
[v2: combine the mm patch into the fs patch; bump the mm part to use
subsys_initcall so we are guaranteed proper ordering; retest after
reproducing the 0-day fail locally; update log to reflect all this.]
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 a1cb8fd2289b..47789292a582 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 */
@@ -1202,7 +1202,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];
@@ -1356,26 +1355,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 cc4c8789b394..12908dcf5831 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