Re: [PATCH v5 08/12] init: main: add KUnit to kernel init

2020-07-07 Thread Luis Chamberlain
On Fri, Jun 26, 2020 at 02:09:13PM -0700, Brendan Higgins wrote:
> Remove KUnit from init calls entirely, instead call directly from
> kernel_init().

The commit log does not explain *why*.

> Co-developed-by: Alan Maguire 
> Signed-off-by: Alan Maguire 
> Signed-off-by: Brendan Higgins 
> Reviewed-by: Stephen Boyd 

Other than that:

Reviewed-by: Luis Chamberlain 

  Luis


Re: [PATCH v5 01/12] vmlinux.lds.h: add linker section for KUnit test suites

2020-07-07 Thread Luis Chamberlain
On Fri, Jun 26, 2020 at 02:22:11PM -0700, Brendan Higgins wrote:
> On Fri, Jun 26, 2020 at 2:20 PM Kees Cook  wrote:
> >
> > On Fri, Jun 26, 2020 at 02:09:06PM -0700, Brendan Higgins wrote:
> > > Add a linker section where KUnit can put references to its test suites.
> > > This patch is the first step in transitioning to dispatching all KUnit
> > > tests from a centralized executor rather than having each as its own
> > > separate late_initcall.
> > >
> > > Co-developed-by: Iurii Zaikin 
> > > Signed-off-by: Iurii Zaikin 
> > > Signed-off-by: Brendan Higgins 
> > > Reviewed-by: Stephen Boyd 
> > > ---
> > >  include/asm-generic/vmlinux.lds.h | 8 
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/include/asm-generic/vmlinux.lds.h 
> > > b/include/asm-generic/vmlinux.lds.h
> > > index db600ef218d7d..4f9b036fc9616 100644
> > > --- a/include/asm-generic/vmlinux.lds.h
> > > +++ b/include/asm-generic/vmlinux.lds.h
> > > @@ -881,6 +881,13 @@
> > >   KEEP(*(.con_initcall.init)) \
> > >   __con_initcall_end = .;
> > >
> > > +/* Alignment must be consistent with (kunit_suite *) in 
> > > include/kunit/test.h */
> >
> > Nit on naming:
> >
> > > +#define KUNIT_TEST_SUITES\
> >
> > I would call this KUNIT_TABLE to maintain the same names as other things
> > of this nature.
> >
> > > + . = ALIGN(8);   \
> > > + __kunit_suites_start = .;   \
> > > + KEEP(*(.kunit_test_suites)) \
> > > + __kunit_suites_end = .;
> > > +
> > >  #ifdef CONFIG_BLK_DEV_INITRD
> > >  #define INIT_RAM_FS  \
> > >   . = ALIGN(4);   \
> > > @@ -1056,6 +1063,7 @@
> > >   INIT_CALLS  \
> > >   CON_INITCALL\
> > >   INIT_RAM_FS \
> > > + KUNIT_TEST_SUITES   \
> > >   }
> >
> > Nack: this must be in INIT_DATA, not in INIT_DATA_SECTION. Not all
> > architectures use the INIT_DATA_SECTION macro (e.g. arm64), but everything
> > uses INIT_DATA.
> 
> Oh, maybe that would eliminate the need for the other linkerscript
> patches? That would be nice.

Curious, did changing it as Kees suggest fix it for m68k?

  Luis


Re: [v2 PATCH] crypto: af_alg - Fix regression on empty requests

2020-07-03 Thread Luis Chamberlain
On Thu, Jul 02, 2020 at 01:32:21PM +1000, Herbert Xu wrote:
> On Tue, Jun 30, 2020 at 02:18:11PM +0530, Naresh Kamboju wrote:
> > 
> > Since we are on this subject,
> > LTP af_alg02  test case fails on stable 4.9 and stable 4.4
> > This is not a regression because the test case has been failing from
> > the beginning.
> > 
> > Is this test case expected to fail on stable 4.9 and 4.4 ?
> > or any chance to fix this on these older branches ?
> > 
> > Test output:
> > af_alg02.c:52: BROK: Timed out while reading from request socket.
> > 
> > ref:
> > https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.228-191-g082e807235d7/testrun/2884917/suite/ltp-crypto-tests/test/af_alg02/history/
> > https://qa-reports.linaro.org/lkft/linux-stable-rc-4.9-oe/build/v4.9.228-191-g082e807235d7/testrun/2884606/suite/ltp-crypto-tests/test/af_alg02/log
> 
> Actually this test really is broken.

FWIW the patch "umh: fix processed error when UMH_WAIT_PROC is used" was
dropped from linux-next for now as it was missing checking for signals.
I'll be open coding iall checks for each UMH_WAIT_PROC callers next. Its
not clear if this was the issue with this test case, but figured I'd let
you know.

  Luis


Re: [PATCH 6/6] kernel: add a kernel_wait helper

2020-06-20 Thread Luis Chamberlain
On Sat, Jun 20, 2020 at 08:35:38AM +0200, Christoph Hellwig wrote:
> On Fri, Jun 19, 2020 at 09:17:00PM +0000, Luis Chamberlain wrote:
> > On Thu, Jun 18, 2020 at 04:46:27PM +0200, Christoph Hellwig wrote:
> > > --- a/kernel/exit.c
> > > +++ b/kernel/exit.c
> > > @@ -1626,6 +1626,22 @@ long kernel_wait4(pid_t upid, int __user 
> > > *stat_addr, int options,
> > >   return ret;
> > >  }
> > >  
> > > +int kernel_wait(pid_t pid, int *stat)
> > > +{
> > > + struct wait_opts wo = {
> > > + .wo_type= PIDTYPE_PID,
> > > + .wo_pid = find_get_pid(pid),
> > > + .wo_flags   = WEXITED,
> > > + };
> > > + int ret;
> > > +
> > > + ret = do_wait();
> > > + if (ret > 0 && wo.wo_stat)
> > > + *stat = wo.wo_stat;
> > 
> > Since all we care about is WEXITED, that could be simplified
> > to something like this:
> > 
> > if (ret > 0 && KWIFEXITED(wo.wo_stat)
> > *stat = KWEXITSTATUS(wo.wo_stat)
> > 
> > Otherwise callers have to use W*() wrappers.
> > 
> > > + put_pid(wo.wo_pid);
> > > + return ret;
> > > +}
> > 
> > Then we don't get *any* in-kernel code dealing with the W*() crap.
> > I just unwrapped this for the umh [0], given that otherwise we'd
> > have to use KW*() callers elsewhere. Doing it upshot one level
> > further would be even better.
> > 
> > [0] https://lkml.kernel.org/r/20200610154923.27510-1-mcg...@kernel.org  
> > 
> Do you just want to pick this patch up, add your suggested bits and
> add it to the beginning of your series?  That should clean the whole
> thing up a bit.  Nothing else in this series depends on the patch.

Sure but let's wait to hear from the NFS folks.

I'm waiting to hear from NFS folks if the one place where the UMH is
fixed for the error code (on fs/nfsd/nfs4recover.c we never were
disabling the upcall as the error code of -ENOENT or -EACCES was *never*
properly checked for) to see how critical that was. If it can help
stable kernels the fix can go in as I proposed, followed by this patch
to further take the KWEXITSTATUS() up further, and ensure we *never*
deal with this in-kernel. If its not a fix stable kernels should care
for what you suggest of taking this patch first would be best and I'd be
happy to do that.

  Luis


Re: [PATCH 6/6] kernel: add a kernel_wait helper

2020-06-19 Thread Luis Chamberlain
On Thu, Jun 18, 2020 at 04:46:27PM +0200, Christoph Hellwig wrote:
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1626,6 +1626,22 @@ long kernel_wait4(pid_t upid, int __user *stat_addr, 
> int options,
>   return ret;
>  }
>  
> +int kernel_wait(pid_t pid, int *stat)
> +{
> + struct wait_opts wo = {
> + .wo_type= PIDTYPE_PID,
> + .wo_pid = find_get_pid(pid),
> + .wo_flags   = WEXITED,
> + };
> + int ret;
> +
> + ret = do_wait();
> + if (ret > 0 && wo.wo_stat)
> + *stat = wo.wo_stat;

Since all we care about is WEXITED, that could be simplified
to something like this:

if (ret > 0 && KWIFEXITED(wo.wo_stat)
*stat = KWEXITSTATUS(wo.wo_stat)

Otherwise callers have to use W*() wrappers.

> + put_pid(wo.wo_pid);
> + return ret;
> +}

Then we don't get *any* in-kernel code dealing with the W*() crap.
I just unwrapped this for the umh [0], given that otherwise we'd
have to use KW*() callers elsewhere. Doing it upshot one level
further would be even better.

[0] https://lkml.kernel.org/r/20200610154923.27510-1-mcg...@kernel.org  


  Luis


Re: [PATCH 01/13] sysctl: add new register_sysctl_subdir() helper

2020-05-29 Thread Luis Chamberlain
On Fri, May 29, 2020 at 11:13:21AM +0300, Jani Nikula wrote:
> On Fri, 29 May 2020, Luis Chamberlain  wrote:
> > Often enough all we need to do is create a subdirectory so that
> > we can stuff sysctls underneath it. However, *if* that directory
> > was already created early on the boot sequence we really have no
> > need to use the full boiler plate code for it, we can just use
> > local variables to help us guide sysctl to place the new leaf files.
> 
> I find it hard to figure out the lifetime requirements for the tables
> passed in; when it's okay to use local variables and when you need
> longer lifetimes. It's not documented, everyone appears to be using
> static tables for this. It's far from obvious.

I agree 2000% that it is not obvious. What made me consider it was that
I *knew* that the base directory would already exist, so it wouldn't
make sense for the code to rely on earlier parts of a table if part
of the hierarchy already existed.

In fact, a *huge* part of the due dilligence on this and futre series
on this cleanup will be to be 100% sure that the base path is already
created. And so this use is obviously dangerous, you just *need* to
know that the base path is created before.

Non-posted changes also deal with link order to help address this
in other places, given that link order controls how *initcalls()
(early_initcall(), late_initcall(), etc) are ordered if you have
multiple of these.

I had a link order series long ago which augmented our ability to make
things clearer at a link order. Eventually I believe this will become
more important, specially as we end up wanting to async more code.

For now, we can only rely on manual code inspection for ensuring
proper ordering. Part of the implicit aspects of this cleanup is
to slowly make these things clearer for each base path.

So... the "fs" base path will actually end up being created in
fs/sysctl.c after we are *fully* done with the fs sysctl cleanups.

  Luis


Re: [PATCH 09/13] firmware_loader: simplify sysctl declaration with register_sysctl_subdir()

2020-05-29 Thread Luis Chamberlain
On Fri, May 29, 2020 at 12:26:13PM +0200, Greg KH wrote:
> On Fri, May 29, 2020 at 07:41:04AM +0000, Luis Chamberlain wrote:
> > From: Xiaoming Ni 
> > 
> > Move the firmware config sysctl table to fallback_table.c and use the
> > new register_sysctl_subdir() helper. This removes the clutter from
> > kernel/sysctl.c.
> > 
> > Signed-off-by: Xiaoming Ni 
> > Signed-off-by: Luis Chamberlain 
> > ---
> >  drivers/base/firmware_loader/fallback.c   |  4 
> >  drivers/base/firmware_loader/fallback.h   | 11 ++
> >  drivers/base/firmware_loader/fallback_table.c | 22 +--
> >  include/linux/sysctl.h|  1 -
> >  kernel/sysctl.c   |  7 --
> >  5 files changed, 35 insertions(+), 10 deletions(-)
> 
> So it now takes more lines than the old stuff?  :(

Pretty much agreed with the other changes, thanks for the review!

But this diff-stat change, indeed, it is unfortunate that we end up
with more code here than before. We'll try to reduce it instead
somehow, however in some cases during this spring-cleaning, since
the goal is to move code from one file to another, it *may* require
more code. So it won't always be negative. But we'll try!

  Luis


Re: [Intel-gfx] [PATCH 06/13] ocfs2: use new sysctl subdir helper register_sysctl_subdir()

2020-05-29 Thread Luis Chamberlain
On Fri, May 29, 2020 at 01:23:19AM -0700, Kees Cook wrote:
> On Fri, May 29, 2020 at 07:41:01AM +0000, Luis Chamberlain wrote:
> > This simplifies the code considerably. The following coccinelle
> > SmPL grammar rule was used to transform this code.
> > 
> > // pycocci sysctl-subdir.cocci fs/ocfs2/stackglue.c
> > 
> > @c1@
> > expression E1;
> > identifier subdir, sysctls;
> > @@
> > 
> > static struct ctl_table subdir[] = {
> > {
> > .procname = E1,
> > .maxlen = 0,
> > .mode = 0555,
> > .child = sysctls,
> > },
> > { }
> > };
> > 
> > @c2@
> > identifier c1.subdir;
> > 
> > expression E2;
> > identifier base;
> > @@
> > 
> > static struct ctl_table base[] = {
> > {
> > .procname = E2,
> > .maxlen = 0,
> > .mode = 0555,
> > .child = subdir,
> > },
> > { }
> > };
> > 
> > @c3@
> > identifier c2.base;
> > identifier header;
> > @@
> > 
> > header = register_sysctl_table(base);
> > 
> > @r1 depends on c1 && c2 && c3@
> > expression c1.E1;
> > identifier c1.subdir, c1.sysctls;
> > @@
> > 
> > -static struct ctl_table subdir[] = {
> > -   {
> > -   .procname = E1,
> > -   .maxlen = 0,
> > -   .mode = 0555,
> > -   .child = sysctls,
> > -   },
> > -   { }
> > -};
> > 
> > @r2 depends on c1 && c2 && c3@
> > identifier c1.subdir;
> > 
> > expression c2.E2;
> > identifier c2.base;
> > @@
> > -static struct ctl_table base[] = {
> > -   {
> > -   .procname = E2,
> > -   .maxlen = 0,
> > -   .mode = 0555,
> > -   .child = subdir,
> > -   },
> > -   { }
> > -};
> > 
> > @r3 depends on c1 && c2 && c3@
> > expression c1.E1;
> > identifier c1.sysctls;
> > expression c2.E2;
> > identifier c2.base;
> > identifier c3.header;
> > @@
> > 
> > header =
> > -register_sysctl_table(base);
> > +register_sysctl_subdir(E2, E1, sysctls);
> > 
> > Generated-by: Coccinelle SmPL
> > 
> > Signed-off-by: Luis Chamberlain 
> > ---
> >  fs/ocfs2/stackglue.c | 27 ---
> >  1 file changed, 4 insertions(+), 23 deletions(-)
> > 
> > diff --git a/fs/ocfs2/stackglue.c b/fs/ocfs2/stackglue.c
> > index a191094694c6..addafced7f59 100644
> > --- a/fs/ocfs2/stackglue.c
> > +++ b/fs/ocfs2/stackglue.c
> > @@ -677,28 +677,8 @@ static struct ctl_table ocfs2_mod_table[] = {
> > },
> > { }
> >  };
> > -
> > -static struct ctl_table ocfs2_kern_table[] = {
> > -   {
> > -   .procname   = "ocfs2",
> > -   .data   = NULL,
> > -   .maxlen = 0,
> > -   .mode   = 0555,
> > -   .child  = ocfs2_mod_table
> > -   },
> > -   { }
> > -};
> > -
> > -static struct ctl_table ocfs2_root_table[] = {
> > -   {
> > -   .procname   = "fs",
> > -   .data   = NULL,
> > -   .maxlen = 0,
> > -   .mode   = 0555,
> > -   .child  = ocfs2_kern_table
> > -   },
> > -   { }
> > -};
> > +   .data   = NULL,
> > +   .data   = NULL,
> 
> The conversion script doesn't like the .data field assignments. ;)
> 
> Was this series built with allmodconfig? I would have expected this to
> blow up very badly. :)

Yikes, sense, you're right. Nope, I left the random config tests to
0day. Will fix, thanks!

  Luis


[PATCH 13/13] fs: move binfmt_misc sysctl to its own file

2020-05-29 Thread Luis Chamberlain
This moves the binfmt_misc sysctl to its own file to help remove
clutter from kernel/sysctl.c.

Signed-off-by: Luis Chamberlain 
---
 fs/binfmt_misc.c | 1 +
 kernel/sysctl.c  | 7 ---
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index f69a043f562b..656b3f5f3bbf 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -821,6 +821,7 @@ static int __init init_misc_binfmt(void)
int err = register_filesystem(_fs_type);
if (!err)
insert_binfmt(_format);
+   register_sysctl_empty_subdir("fs", "binfmt_misc");
return err;
 }
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 460532cd5ac8..7714e7b476c2 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -3042,13 +3042,6 @@ static struct ctl_table fs_table[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_TWO,
},
-#if defined(CONFIG_BINFMT_MISC) || defined(CONFIG_BINFMT_MISC_MODULE)
-   {
-   .procname   = "binfmt_misc",
-   .mode   = 0555,
-   .child  = sysctl_mount_point,
-   },
-#endif
{
.procname   = "pipe-max-size",
.data   = _max_size,
-- 
2.26.2



[PATCH 12/13] sysctl: add helper to register empty subdir

2020-05-29 Thread Luis Chamberlain
The way to create a subdirectory from the base set of directories
is a bit obscure, so provide a helper which makes this clear, and
also helps remove boiler plate code required to do this work.

Signed-off-by: Luis Chamberlain 
---
 include/linux/sysctl.h |  7 +++
 kernel/sysctl.c| 16 +---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 33a471b56345..89c92390e6de 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -208,6 +208,8 @@ extern void register_sysctl_init(const char *path, struct 
ctl_table *table,
 extern struct ctl_table_header *register_sysctl_subdir(const char *base,
   const char *subdir,
   struct ctl_table *table);
+extern void register_sysctl_empty_subdir(const char *base, const char *subdir);
+
 void do_sysctl_args(void);
 
 extern int pwrsw_enabled;
@@ -231,6 +233,11 @@ inline struct ctl_table_header 
*register_sysctl_subdir(const char *base,
return NULL;
 }
 
+static inline void register_sysctl_empty_subdir(const char *base,
+   const char *subdir)
+{
+}
+
 static inline struct ctl_table_header *register_sysctl_paths(
const struct ctl_path *path, struct ctl_table *table)
 {
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index f9a35325d5d5..460532cd5ac8 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -3188,13 +3188,17 @@ struct ctl_table_header *register_sysctl_subdir(const 
char *base,
{ }
};
 
-   if (!table->procname)
+   if (table != sysctl_mount_point && !table->procname)
goto out;
 
hdr = register_sysctl_table(base_table);
if (unlikely(!hdr)) {
-   pr_err("failed when creating subdirectory sysctl %s/%s/%s\n",
-  base, subdir, table->procname);
+   if (table != sysctl_mount_point)
+   pr_err("failed when creating subdirectory sysctl 
%s/%s/%s\n",
+  base, subdir, table->procname);
+   else
+   pr_err("failed when creating empty subddirectory 
%s/%s\n",
+  base, subdir);
goto out;
}
kmemleak_not_leak(hdr);
@@ -3202,6 +3206,12 @@ struct ctl_table_header *register_sysctl_subdir(const 
char *base,
return hdr;
 }
 EXPORT_SYMBOL_GPL(register_sysctl_subdir);
+
+void register_sysctl_empty_subdir(const char *base,
+ const char *subdir)
+{
+   register_sysctl_subdir(base, subdir, sysctl_mount_point);
+}
 #endif /* CONFIG_SYSCTL */
 /*
  * No sense putting this after each symbol definition, twice,
-- 
2.26.2



[PATCH 09/13] firmware_loader: simplify sysctl declaration with register_sysctl_subdir()

2020-05-29 Thread Luis Chamberlain
From: Xiaoming Ni 

Move the firmware config sysctl table to fallback_table.c and use the
new register_sysctl_subdir() helper. This removes the clutter from
kernel/sysctl.c.

Signed-off-by: Xiaoming Ni 
Signed-off-by: Luis Chamberlain 
---
 drivers/base/firmware_loader/fallback.c   |  4 
 drivers/base/firmware_loader/fallback.h   | 11 ++
 drivers/base/firmware_loader/fallback_table.c | 22 +--
 include/linux/sysctl.h|  1 -
 kernel/sysctl.c   |  7 --
 5 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c 
b/drivers/base/firmware_loader/fallback.c
index d9ac7296205e..8190653ae9a3 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -200,12 +200,16 @@ static struct class firmware_class = {
 
 int register_sysfs_loader(void)
 {
+   int ret = register_firmware_config_sysctl();
+   if (ret != 0)
+   return ret;
return class_register(_class);
 }
 
 void unregister_sysfs_loader(void)
 {
class_unregister(_class);
+   unregister_firmware_config_sysctl();
 }
 
 static ssize_t firmware_loading_show(struct device *dev,
diff --git a/drivers/base/firmware_loader/fallback.h 
b/drivers/base/firmware_loader/fallback.h
index 06f4577733a8..7d2cb5f6ceb8 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -42,6 +42,17 @@ void fw_fallback_set_default_timeout(void);
 
 int register_sysfs_loader(void);
 void unregister_sysfs_loader(void);
+#ifdef CONFIG_SYSCTL
+extern int register_firmware_config_sysctl(void);
+extern void unregister_firmware_config_sysctl(void);
+#else
+static inline int register_firmware_config_sysctl(void)
+{
+   return 0;
+}
+static inline void unregister_firmware_config_sysctl(void) { }
+#endif /* CONFIG_SYSCTL */
+
 #else /* CONFIG_FW_LOADER_USER_HELPER */
 static inline int firmware_fallback_sysfs(struct firmware *fw, const char 
*name,
  struct device *device,
diff --git a/drivers/base/firmware_loader/fallback_table.c 
b/drivers/base/firmware_loader/fallback_table.c
index 46a731dede6f..4234aa5ee5df 100644
--- a/drivers/base/firmware_loader/fallback_table.c
+++ b/drivers/base/firmware_loader/fallback_table.c
@@ -24,7 +24,7 @@ struct firmware_fallback_config fw_fallback_config = {
 EXPORT_SYMBOL_NS_GPL(fw_fallback_config, FIRMWARE_LOADER_PRIVATE);
 
 #ifdef CONFIG_SYSCTL
-struct ctl_table firmware_config_table[] = {
+static struct ctl_table firmware_config_table[] = {
{
.procname   = "force_sysfs_fallback",
.data   = _fallback_config.force_sysfs_fallback,
@@ -45,4 +45,22 @@ struct ctl_table firmware_config_table[] = {
},
{ }
 };
-#endif
+
+static struct ctl_table_header *hdr;
+int register_firmware_config_sysctl(void)
+{
+   if (hdr)
+   return -EEXIST;
+   hdr = register_sysctl_subdir("kernel", "firmware_config",
+firmware_config_table);
+   if (!hdr)
+   return -ENOMEM;
+   return 0;
+}
+
+void unregister_firmware_config_sysctl(void)
+{
+   if (hdr)
+   unregister_sysctl_table(hdr);
+}
+#endif /* CONFIG_SYSCTL */
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 58bc978d4f03..aa01f54d0442 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -217,7 +217,6 @@ extern int no_unaligned_warning;
 
 extern struct ctl_table sysctl_mount_point[];
 extern struct ctl_table random_table[];
-extern struct ctl_table firmware_config_table[];
 extern struct ctl_table epoll_table[];
 
 #else /* CONFIG_SYSCTL */
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 30c2d521502a..e007375c8a11 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2088,13 +2088,6 @@ static struct ctl_table kern_table[] = {
.mode   = 0555,
.child  = usermodehelper_table,
},
-#ifdef CONFIG_FW_LOADER_USER_HELPER
-   {
-   .procname   = "firmware_config",
-   .mode   = 0555,
-   .child  = firmware_config_table,
-   },
-#endif
{
.procname   = "overflowuid",
.data   = ,
-- 
2.26.2



[PATCH 11/13] random: simplify sysctl declaration with register_sysctl_subdir()

2020-05-29 Thread Luis Chamberlain
From: Xiaoming Ni 

Move random_table sysctl from kernel/sysctl.c to drivers/char/random.c
and use register_sysctl_subdir() to help remove the clutter out of
kernel/sysctl.c.

Signed-off-by: Xiaoming Ni 
Signed-off-by: Luis Chamberlain 
---
 drivers/char/random.c  | 14 --
 include/linux/sysctl.h |  1 -
 kernel/sysctl.c|  5 -
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index a7cf6aa65908..73fd4b6e9c18 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -2101,8 +2101,7 @@ static int proc_do_entropy(struct ctl_table *table, int 
write,
 }
 
 static int sysctl_poolsize = INPUT_POOL_WORDS * 32;
-extern struct ctl_table random_table[];
-struct ctl_table random_table[] = {
+static struct ctl_table random_table[] = {
{
.procname   = "poolsize",
.data   = _poolsize,
@@ -2164,6 +2163,17 @@ struct ctl_table random_table[] = {
 #endif
{ }
 };
+
+/*
+ * rand_initialize() is called before sysctl_init(),
+ * so we cannot call register_sysctl_init() in rand_initialize()
+ */
+static int __init random_sysctls_init(void)
+{
+   register_sysctl_subdir("kernel", "random", random_table);
+   return 0;
+}
+device_initcall(random_sysctls_init);
 #endif /* CONFIG_SYSCTL */
 
 struct batched_entropy {
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index e5364b69dd95..33a471b56345 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -216,7 +216,6 @@ extern int unaligned_dump_stack;
 extern int no_unaligned_warning;
 
 extern struct ctl_table sysctl_mount_point[];
-extern struct ctl_table random_table[];
 
 #else /* CONFIG_SYSCTL */
 static inline struct ctl_table_header *register_sysctl_table(struct ctl_table 
* table)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 5c116904feb7..f9a35325d5d5 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2078,11 +2078,6 @@ static struct ctl_table kern_table[] = {
.mode   = 0644,
.proc_handler   = sysctl_max_threads,
},
-   {
-   .procname   = "random",
-   .mode   = 0555,
-   .child  = random_table,
-   },
{
.procname   = "usermodehelper",
.mode   = 0555,
-- 
2.26.2



[PATCH 10/13] eventpoll: simplify sysctl declaration with register_sysctl_subdir()

2020-05-29 Thread Luis Chamberlain
From: Xiaoming Ni 

Move epoll_table sysctl to fs/eventpoll.c and remove the
clutter out of kernel/sysctl.c by using register_sysctl_subdir()..

Signed-off-by: Xiaoming Ni 
Signed-off-by: Luis Chamberlain 
---
 fs/eventpoll.c | 10 +-
 include/linux/poll.h   |  2 --
 include/linux/sysctl.h |  1 -
 kernel/sysctl.c|  7 ---
 4 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 12eebcdea9c8..957ebc9700e3 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -299,7 +299,7 @@ static LIST_HEAD(tfile_check_list);
 static long long_zero;
 static long long_max = LONG_MAX;
 
-struct ctl_table epoll_table[] = {
+static struct ctl_table epoll_table[] = {
{
.procname   = "max_user_watches",
.data   = _user_watches,
@@ -311,6 +311,13 @@ struct ctl_table epoll_table[] = {
},
{ }
 };
+
+static void __init epoll_sysctls_init(void)
+{
+   register_sysctl_subdir("fs", "epoll", epoll_table);
+}
+#else
+#define epoll_sysctls_init() do { } while (0)
 #endif /* CONFIG_SYSCTL */
 
 static const struct file_operations eventpoll_fops;
@@ -2422,6 +2429,7 @@ static int __init eventpoll_init(void)
/* Allocates slab cache used to allocate "struct eppoll_entry" */
pwq_cache = kmem_cache_create("eventpoll_pwq",
sizeof(struct eppoll_entry), 0, SLAB_PANIC|SLAB_ACCOUNT, NULL);
+   epoll_sysctls_init();
 
return 0;
 }
diff --git a/include/linux/poll.h b/include/linux/poll.h
index 1cdc32b1f1b0..a9e0e1c2d1f2 100644
--- a/include/linux/poll.h
+++ b/include/linux/poll.h
@@ -8,12 +8,10 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
 
-extern struct ctl_table epoll_table[]; /* for sysctl */
 /* ~832 bytes of stack space used max in sys_select/sys_poll before allocating
additional memory. */
 #ifdef __clang__
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index aa01f54d0442..e5364b69dd95 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -217,7 +217,6 @@ extern int no_unaligned_warning;
 
 extern struct ctl_table sysctl_mount_point[];
 extern struct ctl_table random_table[];
-extern struct ctl_table epoll_table[];
 
 #else /* CONFIG_SYSCTL */
 static inline struct ctl_table_header *register_sysctl_table(struct ctl_table 
* table)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index e007375c8a11..5c116904feb7 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -3001,13 +3001,6 @@ static struct ctl_table fs_table[] = {
.proc_handler   = proc_dointvec,
},
 #endif
-#ifdef CONFIG_EPOLL
-   {
-   .procname   = "epoll",
-   .mode   = 0555,
-   .child  = epoll_table,
-   },
-#endif
 #endif
{
.procname   = "protected_symlinks",
-- 
2.26.2



[PATCH 07/13] test_sysctl: use new sysctl subdir helper register_sysctl_subdir()

2020-05-29 Thread Luis Chamberlain
This simplifies the code considerably. The following coccinelle
SmPL grammar rule was used to transform this code.

// pycocci sysctl-subdir.cocci lib/test_sysctl.c

@c1@
expression E1;
identifier subdir, sysctls;
@@

static struct ctl_table subdir[] = {
{
.procname = E1,
.maxlen = 0,
.mode = 0555,
.child = sysctls,
},
{ }
};

@c2@
identifier c1.subdir;

expression E2;
identifier base;
@@

static struct ctl_table base[] = {
{
.procname = E2,
.maxlen = 0,
.mode = 0555,
.child = subdir,
},
{ }
};

@c3@
identifier c2.base;
identifier header;
@@

header = register_sysctl_table(base);

@r1 depends on c1 && c2 && c3@
expression c1.E1;
identifier c1.subdir, c1.sysctls;
@@

-static struct ctl_table subdir[] = {
-   {
-   .procname = E1,
-   .maxlen = 0,
-   .mode = 0555,
-   .child = sysctls,
-   },
-   { }
-};

@r2 depends on c1 && c2 && c3@
identifier c1.subdir;

expression c2.E2;
identifier c2.base;
@@
-static struct ctl_table base[] = {
-   {
-   .procname = E2,
-   .maxlen = 0,
-   .mode = 0555,
-   .child = subdir,
-   },
-   { }
-};

@r3 depends on c1 && c2 && c3@
expression c1.E1;
identifier c1.sysctls;
expression c2.E2;
identifier c2.base;
identifier c3.header;
@@

header =
-register_sysctl_table(base);
+register_sysctl_subdir(E2, E1, sysctls);

Generated-by: Coccinelle SmPL
Signed-off-by: Luis Chamberlain 
---
 lib/test_sysctl.c | 23 ++-
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/lib/test_sysctl.c b/lib/test_sysctl.c
index 84eaae22d3a6..b17581307756 100644
--- a/lib/test_sysctl.c
+++ b/lib/test_sysctl.c
@@ -128,26 +128,6 @@ static struct ctl_table test_table[] = {
{ }
 };
 
-static struct ctl_table test_sysctl_table[] = {
-   {
-   .procname   = "test_sysctl",
-   .maxlen = 0,
-   .mode   = 0555,
-   .child  = test_table,
-   },
-   { }
-};
-
-static struct ctl_table test_sysctl_root_table[] = {
-   {
-   .procname   = "debug",
-   .maxlen = 0,
-   .mode   = 0555,
-   .child  = test_sysctl_table,
-   },
-   { }
-};
-
 static struct ctl_table_header *test_sysctl_header;
 
 static int __init test_sysctl_init(void)
@@ -155,7 +135,8 @@ static int __init test_sysctl_init(void)
test_data.bitmap_0001 = kzalloc(SYSCTL_TEST_BITMAP_SIZE/8, GFP_KERNEL);
if (!test_data.bitmap_0001)
return -ENOMEM;
-   test_sysctl_header = register_sysctl_table(test_sysctl_root_table);
+   test_sysctl_header = register_sysctl_subdir("debug", "test_sysctl",
+   test_table);
if (!test_sysctl_header) {
kfree(test_data.bitmap_0001);
return -ENOMEM;
-- 
2.26.2



[PATCH 08/13] inotify: simplify sysctl declaration with register_sysctl_subdir()

2020-05-29 Thread Luis Chamberlain
From: Xiaoming Ni 

move inotify_user sysctl to inotify_user.c and use the new
register_sysctl_subdir() helper.

Signed-off-by: Xiaoming Ni 
Signed-off-by: Luis Chamberlain 
---
 fs/notify/inotify/inotify_user.c | 11 ++-
 include/linux/inotify.h  |  3 ---
 kernel/sysctl.c  | 11 ---
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index f88bbcc9efeb..64859fbf8463 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -46,7 +46,7 @@ struct kmem_cache *inotify_inode_mark_cachep __read_mostly;
 
 #include 
 
-struct ctl_table inotify_table[] = {
+static struct ctl_table inotify_table[] = {
{
.procname   = "max_user_instances",
.data   = 
_user_ns.ucount_max[UCOUNT_INOTIFY_INSTANCES],
@@ -73,6 +73,14 @@ struct ctl_table inotify_table[] = {
},
{ }
 };
+
+static void __init inotify_sysctls_init(void)
+{
+   register_sysctl_subdir("fs", "inotify", inotify_table);
+}
+
+#else
+#define inotify_sysctls_init() do { } while (0)
 #endif /* CONFIG_SYSCTL */
 
 static inline __u32 inotify_arg_to_mask(u32 arg)
@@ -826,6 +834,7 @@ static int __init inotify_user_setup(void)
inotify_max_queued_events = 16384;
init_user_ns.ucount_max[UCOUNT_INOTIFY_INSTANCES] = 128;
init_user_ns.ucount_max[UCOUNT_INOTIFY_WATCHES] = 8192;
+   inotify_sysctls_init();
 
return 0;
 }
diff --git a/include/linux/inotify.h b/include/linux/inotify.h
index 6a24905f6e1e..8d20caa1b268 100644
--- a/include/linux/inotify.h
+++ b/include/linux/inotify.h
@@ -7,11 +7,8 @@
 #ifndef _LINUX_INOTIFY_H
 #define _LINUX_INOTIFY_H
 
-#include 
 #include 
 
-extern struct ctl_table inotify_table[]; /* for sysctl */
-
 #define ALL_INOTIFY_BITS (IN_ACCESS | IN_MODIFY | IN_ATTRIB | IN_CLOSE_WRITE | 
\
  IN_CLOSE_NOWRITE | IN_OPEN | IN_MOVED_FROM | \
  IN_MOVED_TO | IN_CREATE | IN_DELETE | \
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 04ff032f2863..30c2d521502a 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -123,10 +123,6 @@ static const int maxolduid = 65535;
 static int ngroups_max = NGROUPS_MAX;
 static const int cap_last_cap = CAP_LAST_CAP;
 
-#ifdef CONFIG_INOTIFY_USER
-#include 
-#endif
-
 #ifdef CONFIG_PROC_SYSCTL
 
 /**
@@ -3012,13 +3008,6 @@ static struct ctl_table fs_table[] = {
.proc_handler   = proc_dointvec,
},
 #endif
-#ifdef CONFIG_INOTIFY_USER
-   {
-   .procname   = "inotify",
-   .mode   = 0555,
-   .child  = inotify_table,
-   },
-#endif 
 #ifdef CONFIG_EPOLL
{
.procname   = "epoll",
-- 
2.26.2



[PATCH 06/13] ocfs2: use new sysctl subdir helper register_sysctl_subdir()

2020-05-29 Thread Luis Chamberlain
This simplifies the code considerably. The following coccinelle
SmPL grammar rule was used to transform this code.

// pycocci sysctl-subdir.cocci fs/ocfs2/stackglue.c

@c1@
expression E1;
identifier subdir, sysctls;
@@

static struct ctl_table subdir[] = {
{
.procname = E1,
.maxlen = 0,
.mode = 0555,
.child = sysctls,
},
{ }
};

@c2@
identifier c1.subdir;

expression E2;
identifier base;
@@

static struct ctl_table base[] = {
{
.procname = E2,
.maxlen = 0,
.mode = 0555,
.child = subdir,
},
{ }
};

@c3@
identifier c2.base;
identifier header;
@@

header = register_sysctl_table(base);

@r1 depends on c1 && c2 && c3@
expression c1.E1;
identifier c1.subdir, c1.sysctls;
@@

-static struct ctl_table subdir[] = {
-   {
-   .procname = E1,
-   .maxlen = 0,
-   .mode = 0555,
-   .child = sysctls,
-   },
-   { }
-};

@r2 depends on c1 && c2 && c3@
identifier c1.subdir;

expression c2.E2;
identifier c2.base;
@@
-static struct ctl_table base[] = {
-   {
-   .procname = E2,
-   .maxlen = 0,
-   .mode = 0555,
-   .child = subdir,
-   },
-   { }
-};

@r3 depends on c1 && c2 && c3@
expression c1.E1;
identifier c1.sysctls;
expression c2.E2;
identifier c2.base;
identifier c3.header;
@@

header =
-register_sysctl_table(base);
+register_sysctl_subdir(E2, E1, sysctls);

Generated-by: Coccinelle SmPL

Signed-off-by: Luis Chamberlain 
---
 fs/ocfs2/stackglue.c | 27 ---
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/fs/ocfs2/stackglue.c b/fs/ocfs2/stackglue.c
index a191094694c6..addafced7f59 100644
--- a/fs/ocfs2/stackglue.c
+++ b/fs/ocfs2/stackglue.c
@@ -677,28 +677,8 @@ static struct ctl_table ocfs2_mod_table[] = {
},
{ }
 };
-
-static struct ctl_table ocfs2_kern_table[] = {
-   {
-   .procname   = "ocfs2",
-   .data   = NULL,
-   .maxlen = 0,
-   .mode   = 0555,
-   .child  = ocfs2_mod_table
-   },
-   { }
-};
-
-static struct ctl_table ocfs2_root_table[] = {
-   {
-   .procname   = "fs",
-   .data   = NULL,
-   .maxlen = 0,
-   .mode   = 0555,
-   .child  = ocfs2_kern_table
-   },
-   { }
-};
+   .data   = NULL,
+   .data   = NULL,
 
 static struct ctl_table_header *ocfs2_table_header;
 
@@ -711,7 +691,8 @@ static int __init ocfs2_stack_glue_init(void)
 {
strcpy(cluster_stack_name, OCFS2_STACK_PLUGIN_O2CB);
 
-   ocfs2_table_header = register_sysctl_table(ocfs2_root_table);
+   ocfs2_table_header = register_sysctl_subdir("fs", "ocfs2",
+   ocfs2_mod_table);
if (!ocfs2_table_header) {
printk(KERN_ERR
   "ocfs2 stack glue: unable to register sysctl\n");
-- 
2.26.2



[PATCH 01/13] sysctl: add new register_sysctl_subdir() helper

2020-05-29 Thread Luis Chamberlain
Often enough all we need to do is create a subdirectory so that
we can stuff sysctls underneath it. However, *if* that directory
was already created early on the boot sequence we really have no
need to use the full boiler plate code for it, we can just use
local variables to help us guide sysctl to place the new leaf files.

So use a helper to do precisely this.

Signed-off-by: Luis Chamberlain 
---
 include/linux/sysctl.h | 11 +++
 kernel/sysctl.c| 37 +
 2 files changed, 48 insertions(+)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index ddaa06ddd852..58bc978d4f03 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -205,6 +205,9 @@ void unregister_sysctl_table(struct ctl_table_header * 
table);
 extern int sysctl_init(void);
 extern void register_sysctl_init(const char *path, struct ctl_table *table,
 const char *table_name);
+extern struct ctl_table_header *register_sysctl_subdir(const char *base,
+  const char *subdir,
+  struct ctl_table *table);
 void do_sysctl_args(void);
 
 extern int pwrsw_enabled;
@@ -223,6 +226,14 @@ static inline struct ctl_table_header 
*register_sysctl_table(struct ctl_table *
return NULL;
 }
 
+static
+inline struct ctl_table_header *register_sysctl_subdir(const char *base,
+  const char *subdir,
+  struct ctl_table *table)
+{
+   return NULL;
+}
+
 static inline struct ctl_table_header *register_sysctl_paths(
const struct ctl_path *path, struct ctl_table *table)
 {
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 008ac0576ae5..04ff032f2863 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -3195,6 +3195,43 @@ void __init register_sysctl_init(const char *path, 
struct ctl_table *table,
}
kmemleak_not_leak(hdr);
 }
+
+struct ctl_table_header *register_sysctl_subdir(const char *base,
+   const char *subdir,
+   struct ctl_table *table)
+{
+   struct ctl_table_header *hdr = NULL;
+   struct ctl_table subdir_table[] = {
+   {
+   .procname   = subdir,
+   .mode   = 0555,
+   .child  = table,
+   },
+   { }
+   };
+   struct ctl_table base_table[] = {
+   {
+   .procname   = base,
+   .mode   = 0555,
+   .child  = subdir_table,
+   },
+   { }
+   };
+
+   if (!table->procname)
+   goto out;
+
+   hdr = register_sysctl_table(base_table);
+   if (unlikely(!hdr)) {
+   pr_err("failed when creating subdirectory sysctl %s/%s/%s\n",
+  base, subdir, table->procname);
+   goto out;
+   }
+   kmemleak_not_leak(hdr);
+out:
+   return hdr;
+}
+EXPORT_SYMBOL_GPL(register_sysctl_subdir);
 #endif /* CONFIG_SYSCTL */
 /*
  * No sense putting this after each symbol definition, twice,
-- 
2.26.2



[PATCH 05/13] macintosh/mac_hid.c: use new sysctl subdir helper register_sysctl_subdir()

2020-05-29 Thread Luis Chamberlain
This simplifies the code considerably. The following coccinelle
SmPL grammar rule was used to transform this code.

// pycocci sysctl-subdir.cocci drivers/macintosh/mac_hid.c

@c1@
expression E1;
identifier subdir, sysctls;
@@

static struct ctl_table subdir[] = {
{
.procname = E1,
.maxlen = 0,
.mode = 0555,
.child = sysctls,
},
{ }
};

@c2@
identifier c1.subdir;

expression E2;
identifier base;
@@

static struct ctl_table base[] = {
{
.procname = E2,
.maxlen = 0,
.mode = 0555,
.child = subdir,
},
{ }
};

@c3@
identifier c2.base;
identifier header;
@@

header = register_sysctl_table(base);

@r1 depends on c1 && c2 && c3@
expression c1.E1;
identifier c1.subdir, c1.sysctls;
@@

-static struct ctl_table subdir[] = {
-   {
-   .procname = E1,
-   .maxlen = 0,
-   .mode = 0555,
-   .child = sysctls,
-   },
-   { }
-};

@r2 depends on c1 && c2 && c3@
identifier c1.subdir;

expression c2.E2;
identifier c2.base;
@@
-static struct ctl_table base[] = {
-   {
-   .procname = E2,
-   .maxlen = 0,
-   .mode = 0555,
-   .child = subdir,
-   },
-   { }
-};

@r3 depends on c1 && c2 && c3@
expression c1.E1;
identifier c1.sysctls;
expression c2.E2;
identifier c2.base;
identifier c3.header;
@@

header =
-register_sysctl_table(base);
+register_sysctl_subdir(E2, E1, sysctls);

Generated-by: Coccinelle SmPL

Signed-off-by: Luis Chamberlain 
---
 drivers/macintosh/mac_hid.c | 25 ++---
 1 file changed, 2 insertions(+), 23 deletions(-)

diff --git a/drivers/macintosh/mac_hid.c b/drivers/macintosh/mac_hid.c
index 28b8581b44dd..736d0e151716 100644
--- a/drivers/macintosh/mac_hid.c
+++ b/drivers/macintosh/mac_hid.c
@@ -239,33 +239,12 @@ static struct ctl_table mac_hid_files[] = {
{ }
 };
 
-/* dir in /proc/sys/dev */
-static struct ctl_table mac_hid_dir[] = {
-   {
-   .procname   = "mac_hid",
-   .maxlen = 0,
-   .mode   = 0555,
-   .child  = mac_hid_files,
-   },
-   { }
-};
-
-/* /proc/sys/dev itself, in case that is not there yet */
-static struct ctl_table mac_hid_root_dir[] = {
-   {
-   .procname   = "dev",
-   .maxlen = 0,
-   .mode   = 0555,
-   .child  = mac_hid_dir,
-   },
-   { }
-};
-
 static struct ctl_table_header *mac_hid_sysctl_header;
 
 static int __init mac_hid_init(void)
 {
-   mac_hid_sysctl_header = register_sysctl_table(mac_hid_root_dir);
+   mac_hid_sysctl_header = register_sysctl_subdir("dev", "mac_hid",
+  mac_hid_files);
if (!mac_hid_sysctl_header)
return -ENOMEM;
 
-- 
2.26.2



[PATCH 04/13] i915: use new sysctl subdir helper register_sysctl_subdir()

2020-05-29 Thread Luis Chamberlain
This simplifies the code considerably. The following coccinelle
SmPL grammar rule was used to transform this code.

// pycocci sysctl-subdir.cocci drivers/gpu/drm/i915/i915_perf.c

@c1@
expression E1;
identifier subdir, sysctls;
@@

static struct ctl_table subdir[] = {
{
.procname = E1,
.maxlen = 0,
.mode = 0555,
.child = sysctls,
},
{ }
};

@c2@
identifier c1.subdir;

expression E2;
identifier base;
@@

static struct ctl_table base[] = {
{
.procname = E2,
.maxlen = 0,
.mode = 0555,
.child = subdir,
},
{ }
};

@c3@
identifier c2.base;
identifier header;
@@

header = register_sysctl_table(base);

@r1 depends on c1 && c2 && c3@
expression c1.E1;
identifier c1.subdir, c1.sysctls;
@@

-static struct ctl_table subdir[] = {
-   {
-   .procname = E1,
-   .maxlen = 0,
-   .mode = 0555,
-   .child = sysctls,
-   },
-   { }
-};

@r2 depends on c1 && c2 && c3@
identifier c1.subdir;

expression c2.E2;
identifier c2.base;
@@
-static struct ctl_table base[] = {
-   {
-   .procname = E2,
-   .maxlen = 0,
-   .mode = 0555,
-   .child = subdir,
-   },
-   { }
-};

@r3 depends on c1 && c2 && c3@
expression c1.E1;
identifier c1.sysctls;
expression c2.E2;
identifier c2.base;
identifier c3.header;
@@

header =
-register_sysctl_table(base);
+register_sysctl_subdir(E2, E1, sysctls);

Generated-by: Coccinelle SmPL
Signed-off-by: Luis Chamberlain 
---
 drivers/gpu/drm/i915/i915_perf.c | 22 +-
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 665bb076e84d..52509b573794 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -4203,26 +4203,6 @@ static struct ctl_table oa_table[] = {
{}
 };
 
-static struct ctl_table i915_root[] = {
-   {
-.procname = "i915",
-.maxlen = 0,
-.mode = 0555,
-.child = oa_table,
-},
-   {}
-};
-
-static struct ctl_table dev_root[] = {
-   {
-.procname = "dev",
-.maxlen = 0,
-.mode = 0555,
-.child = i915_root,
-},
-   {}
-};
-
 /**
  * i915_perf_init - initialize i915-perf state on module bind
  * @i915: i915 device instance
@@ -4383,7 +4363,7 @@ static int destroy_config(int id, void *p, void *data)
 
 void i915_perf_sysctl_register(void)
 {
-   sysctl_header = register_sysctl_table(dev_root);
+   sysctl_header = register_sysctl_subdir("dev", "i915", oa_table);
 }
 
 void i915_perf_sysctl_unregister(void)
-- 
2.26.2



[PATCH 02/13] cdrom: use new sysctl subdir helper register_sysctl_subdir()

2020-05-29 Thread Luis Chamberlain
This simplifies the code considerably. The following coccinelle
SmPL grammar rule was used to transform this code.

// pycocci sysctl-subdir.cocci drivers/cdrom/cdrom.c

@c1@
expression E1;
identifier subdir, sysctls;
@@

static struct ctl_table subdir[] = {
{
.procname = E1,
.maxlen = 0,
.mode = 0555,
.child = sysctls,
},
{ }
};

@c2@
identifier c1.subdir;

expression E2;
identifier base;
@@

static struct ctl_table base[] = {
{
.procname = E2,
.maxlen = 0,
.mode = 0555,
.child = subdir,
},
{ }
};

@c3@
identifier c2.base;
identifier header;
@@

header = register_sysctl_table(base);

@r1 depends on c1 && c2 && c3@
expression c1.E1;
identifier c1.subdir, c1.sysctls;
@@

-static struct ctl_table subdir[] = {
-   {
-   .procname = E1,
-   .maxlen = 0,
-   .mode = 0555,
-   .child = sysctls,
-   },
-   { }
-};

@r2 depends on c1 && c2 && c3@
identifier c1.subdir;

expression c2.E2;
identifier c2.base;
@@
-static struct ctl_table base[] = {
-   {
-   .procname = E2,
-   .maxlen = 0,
-   .mode = 0555,
-   .child = subdir,
-   },
-   { }
-};

@r3 depends on c1 && c2 && c3@
expression c1.E1;
identifier c1.sysctls;
expression c2.E2;
identifier c2.base;
identifier c3.header;
@@

header =
-register_sysctl_table(base);
+register_sysctl_subdir(E2, E1, sysctls);

Generated-by: Coccinelle SmPL
Signed-off-by: Luis Chamberlain 
---
 drivers/cdrom/cdrom.c | 23 ++-
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index a0a7ae705de8..3c638f464cef 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -3719,26 +3719,6 @@ static struct ctl_table cdrom_table[] = {
{ }
 };
 
-static struct ctl_table cdrom_cdrom_table[] = {
-   {
-   .procname   = "cdrom",
-   .maxlen = 0,
-   .mode   = 0555,
-   .child  = cdrom_table,
-   },
-   { }
-};
-
-/* Make sure that /proc/sys/dev is there */
-static struct ctl_table cdrom_root_table[] = {
-   {
-   .procname   = "dev",
-   .maxlen = 0,
-   .mode   = 0555,
-   .child  = cdrom_cdrom_table,
-   },
-   { }
-};
 static struct ctl_table_header *cdrom_sysctl_header;
 
 static void cdrom_sysctl_register(void)
@@ -3748,7 +3728,8 @@ static void cdrom_sysctl_register(void)
if (!atomic_add_unless(, 1, 1))
return;
 
-   cdrom_sysctl_header = register_sysctl_table(cdrom_root_table);
+   cdrom_sysctl_header = register_sysctl_subdir("dev", "cdrom",
+cdrom_table);
 
/* set the defaults */
cdrom_sysctl_settings.autoclose = autoclose;
-- 
2.26.2



[PATCH 03/13] hpet: use new sysctl subdir helper register_sysctl_subdir()

2020-05-29 Thread Luis Chamberlain
This simplifies the code considerably. The following coccinelle
SmPL grammar rule was used to transform this code.

// pycocci sysctl-subdir.cocci drivers/char/hpet.c

@c1@
expression E1;
identifier subdir, sysctls;
@@

static struct ctl_table subdir[] = {
{
.procname = E1,
.maxlen = 0,
.mode = 0555,
.child = sysctls,
},
{ }
};

@c2@
identifier c1.subdir;

expression E2;
identifier base;
@@

static struct ctl_table base[] = {
{
.procname = E2,
.maxlen = 0,
.mode = 0555,
.child = subdir,
},
{ }
};

@c3@
identifier c2.base;
identifier header;
@@

header = register_sysctl_table(base);

@r1 depends on c1 && c2 && c3@
expression c1.E1;
identifier c1.subdir, c1.sysctls;
@@

-static struct ctl_table subdir[] = {
-   {
-   .procname = E1,
-   .maxlen = 0,
-   .mode = 0555,
-   .child = sysctls,
-   },
-   { }
-};

@r2 depends on c1 && c2 && c3@
identifier c1.subdir;

expression c2.E2;
identifier c2.base;
@@
-static struct ctl_table base[] = {
-   {
-   .procname = E2,
-   .maxlen = 0,
-   .mode = 0555,
-   .child = subdir,
-   },
-   { }
-};

@r3 depends on c1 && c2 && c3@
expression c1.E1;
identifier c1.sysctls;
expression c2.E2;
identifier c2.base;
identifier c3.header;
@@

header =
-register_sysctl_table(base);
+register_sysctl_subdir(E2, E1, sysctls);

Generated-by: Coccinelle SmPL

Signed-off-by: Luis Chamberlain 
---
 drivers/char/hpet.c | 22 +-
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index ed3b7dab678d..169c970d5ff8 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -746,26 +746,6 @@ static struct ctl_table hpet_table[] = {
{}
 };
 
-static struct ctl_table hpet_root[] = {
-   {
-.procname = "hpet",
-.maxlen = 0,
-.mode = 0555,
-.child = hpet_table,
-},
-   {}
-};
-
-static struct ctl_table dev_root[] = {
-   {
-.procname = "dev",
-.maxlen = 0,
-.mode = 0555,
-.child = hpet_root,
-},
-   {}
-};
-
 static struct ctl_table_header *sysctl_header;
 
 /*
@@ -1059,7 +1039,7 @@ static int __init hpet_init(void)
if (result < 0)
return -ENODEV;
 
-   sysctl_header = register_sysctl_table(dev_root);
+   sysctl_header = register_sysctl_subdir("dev", "hpet", hpet_table);
 
result = acpi_bus_register_driver(_acpi_driver);
if (result < 0) {
-- 
2.26.2



[PATCH 00/13] sysctl: spring cleaning

2020-05-29 Thread Luis Chamberlain
Me and Xiaoming are working on some kernel/sysctl.c spring cleaning.
During a recent linux-next merge conflict it became clear that
the kitchen sink on kernel/sysctl.c creates too many conflicts,
and so we need to do away with stuffing everyone's knobs on this
one file.

This is part of that work. This is not expected to get merged yet, but
since our delta is pretty considerable at this point, we need to piece
meal this and collect reviews for what we have so far. This follows up
on some of his recent work.

This series focuses on a new helper to deal with subdirectories and
empty subdirectories. The terminology that we will embrace will be
that things like "fs", "kernel", "debug" are based directories, and
directories underneath this are subdirectories.

In this case, the cleanup ends up also trimming the amount of
code we have for sysctls.

If this seems reasonable we'll kdocify this a bit too.

This code has been boot tested without issues, and I'm letting 0day do
its thing to test against many kconfig builds. If you however spot
any issues please let us know.

Luis Chamberlain (9):
  sysctl: add new register_sysctl_subdir() helper
  cdrom: use new sysctl subdir helper register_sysctl_subdir()
  hpet: use new sysctl subdir helper register_sysctl_subdir()
  i915: use new sysctl subdir helper register_sysctl_subdir()
  macintosh/mac_hid.c: use new sysctl subdir helper
register_sysctl_subdir()
  ocfs2: use new sysctl subdir helper register_sysctl_subdir()
  test_sysctl: use new sysctl subdir helper register_sysctl_subdir()
  sysctl: add helper to register empty subdir
  fs: move binfmt_misc sysctl to its own file

Xiaoming Ni (4):
  inotify: simplify sysctl declaration with register_sysctl_subdir()
  firmware_loader: simplify sysctl declaration with
register_sysctl_subdir()
  eventpoll: simplify sysctl declaration with register_sysctl_subdir()
  random: simplify sysctl declaration with register_sysctl_subdir()

 drivers/base/firmware_loader/fallback.c   |  4 +
 drivers/base/firmware_loader/fallback.h   | 11 +++
 drivers/base/firmware_loader/fallback_table.c | 22 -
 drivers/cdrom/cdrom.c | 23 +
 drivers/char/hpet.c   | 22 +
 drivers/char/random.c | 14 +++-
 drivers/gpu/drm/i915/i915_perf.c  | 22 +
 drivers/macintosh/mac_hid.c   | 25 +-
 fs/binfmt_misc.c  |  1 +
 fs/eventpoll.c| 10 ++-
 fs/notify/inotify/inotify_user.c  | 11 ++-
 fs/ocfs2/stackglue.c  | 27 +-
 include/linux/inotify.h   |  3 -
 include/linux/poll.h  |  2 -
 include/linux/sysctl.h| 21 -
 kernel/sysctl.c   | 84 +++
 lib/test_sysctl.c | 23 +
 17 files changed, 144 insertions(+), 181 deletions(-)

-- 
2.26.2



Re: [PATCH 0/8] Simplefs: group and simplify linux fs code

2020-04-16 Thread Luis Chamberlain
On Tue, Apr 14, 2020 at 02:42:54PM +0200, Emanuele Giuseppe Esposito wrote:
> This series of patches introduce wrappers for functions,
> arguments simplification in functions calls and most importantly
> groups duplicated code in a single header, simplefs, to avoid redundancy
> in the linux fs, especially debugfs and tracefs.

The general goal seems worthy, but here I don't see explained why hasn't
this gone through libfs, and what the intention was long term. For
instance, you added some other generalizations which you have found. It
was not clear that this was the goal here, to expand on these paths.

What if common code on fs is found which are not part of debugfs and
tracefs, how does one decide if to move to libfs or simplefs?

  Luis


Re: [PATCH 2/8] fs: extract simple_pin/release_fs to separate files

2020-04-16 Thread Luis Chamberlain
On Tue, Apr 14, 2020 at 02:42:56PM +0200, Emanuele Giuseppe Esposito wrote:
> We will augment this family of functions with inode management.  To avoid
> littering include/linux/fs.h and fs/libfs.c, move them to a separate header,
> with a Kconfig symbol to enable them.

If there are no functional changes, indicating that on the commit log
will make the review much easier.

> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index d1398cef3b18..fc38a6f0fc11 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -288,12 +288,16 @@ config STRIP_ASM_SYMS
>  
>  config READABLE_ASM
>   bool "Generate readable assembler code"
> - depends on DEBUG_KERNEL
> - help
> -   Disable some compiler optimizations that tend to generate human 
> unreadable
> -   assembler output. This may make the kernel slightly slower, but it 
> helps
> -   to keep kernel developers who have to stare a lot at assembler 
> listings
> -   sane.
> +depends on DEBUG_KERNEL
> +help
> +  Disable some compiler optimizations that tend to generate human 
> unreadable
> +  assembler output. This may make the kernel slightly slower, but it 
> helps
> +  to keep kernel developers who have to stare a lot at assembler listings
> +  sane.
> +   

This minor change above should just be a separate patch. Its just noise
otherwise.

> +config DEBUG_FS
> + bool "Debug Filesystem"
> + select SIMPLEFS

I'm at a loss reviewing this,  my lib/Kconfig.debug already has a config
DEBUG_FS.  But above I see it is being added for the very first time.
I'm sure there is some odd conditional which is obscuring this, can
this be explained in the commit log?

  Luis


Re: [PATCH 1/8] apparmor: just use vfs_kern_mount to make .null

2020-04-16 Thread Luis Chamberlain
On Tue, Apr 14, 2020 at 02:42:55PM +0200, Emanuele Giuseppe Esposito wrote:
> aa_mk_null_file is using simple_pin_fs/simple_release_fs with local
> variables as arguments, for what would amount to a simple
> vfs_kern_mount/mntput pair if everything was inlined.  Just use
> the normal filesystem API since the reference counting is not needed
> here.

*Why* is refcounting not needed here?

   Luis

> 
> Signed-off-by: Emanuele Giuseppe Esposito 
> ---
>  security/apparmor/apparmorfs.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 280741fc0f5f..828bb1eb77ea 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -2525,14 +2525,14 @@ struct path aa_null;
>  
>  static int aa_mk_null_file(struct dentry *parent)
>  {
> - struct vfsmount *mount = NULL;
> + struct file_system_type *type = parent->d_sb->s_type;
> + struct vfsmount *mount;
>   struct dentry *dentry;
>   struct inode *inode;
> - int count = 0;
> - int error = simple_pin_fs(parent->d_sb->s_type, , );
>  
> - if (error)
> - return error;
> + mount = vfs_kern_mount(type, SB_KERNMOUNT, type->name, NULL);
> + if (IS_ERR(mount))
> + return PTR_ERR(mount);
>  
>   inode_lock(d_inode(parent));
>   dentry = lookup_one_len(NULL_FILE_NAME, parent, strlen(NULL_FILE_NAME));
> @@ -2561,7 +2561,7 @@ static int aa_mk_null_file(struct dentry *parent)
>   dput(dentry);
>  out:
>   inode_unlock(d_inode(parent));
> - simple_release_fs(, );
> + mntput(mount);
>   return error;
>  }
>  
> -- 
> 2.25.2
>