Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1294153 > unrolled thread

[PATCH v2 0/8] fs: don't use module helpers in non-modular code

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/8] fs: don't use module helpers in non-modular code Paul Gortmaker <paul.gortmaker@windriver.com> - 2015-12-17 20:20 +0100
    [PATCH 7/8] fs: make devpts/inode.c explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2015-12-17 20:20 +0100
      Re: [PATCH 7/8] fs: make devpts/inode.c explicitly non-modular Peter Hurley <peter@hurleysoftware.com> - 2015-12-17 20:50 +0100
        Re: [PATCH 7/8] fs: make devpts/inode.c explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2015-12-20 04:00 +0100
    [PATCH 8/8] fs: make binfmt_elf.c explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2015-12-17 20:20 +0100
    [PATCH 2/8] fs: make notify dnotify.c explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2015-12-17 20:20 +0100
    [PATCH 3/8] fs: make fcntl.c explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2015-12-17 20:20 +0100

#1294153 — [PATCH v2 0/8] fs: don't use module helpers in non-modular code

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 <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: David Rientjes <rientjes@google.com>
Cc: Eric Paris <eparis@parisplace.org>
Cc: Hillf Danton <hillf.zj@alibaba-inc.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: Jeff Layton <jlayton@poochiereds.net>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Nadia Yvette Chambers <nyc@holomorphy.com>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-mm@kvack.org

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1294155 — [PATCH 7/8] fs: make devpts/inode.c explicitly non-modular

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 <viro@zeniv.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1294184 — Re: [PATCH 7/8] fs: make devpts/inode.c explicitly non-modular

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 <viro@zeniv.linux.org.uk>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Peter Hurley <peter@hurleysoftware.com>
> Cc: Josh Triplett <josh@joshtriplett.org>
> Cc: David Howells <dhowells@redhat.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  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)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1295504 — Re: [PATCH 7/8] fs: make devpts/inode.c explicitly non-modular

[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 <viro@zeniv.linux.org.uk>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Peter Hurley <peter@hurleysoftware.com>
> > Cc: Josh Triplett <josh@joshtriplett.org>
> > Cc: David Howells <dhowells@redhat.com>
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> > ---
> >  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)
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1294157 — [PATCH 8/8] fs: make binfmt_elf.c explicitly non-modular

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 <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 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 (ericy@cais.com).
+ * 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1294158 — [PATCH 2/8] fs: make notify dnotify.c explicitly non-modular

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 <eparis@parisplace.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1294159 — [PATCH 3/8] fs: make fcntl.c explicitly non-modular

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 <viro@zeniv.linux.org.uk>
Cc: Jeff Layton <jlayton@poochiereds.net>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web