Skip to contentSkip to conversation

sysctl: minor CONFIG_SYSCTL cleanups

PATCH In review Oleg Nesterov 2 participants 5 messages Aug 30 – Sep 14

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover sysctl: minor CONFIG_SYSCTL cleanups

Hi Joel,

I tried to address your comments to V1
https://lore.kernel.org/all/[email protected]/

Changes:

	2/3 - new

	3/3 - don't remove the extra whitespace in drivers/parport/procfs.c

Oleg.
--

 drivers/parport/procfs.c |   8 +--
 fs/dcache.c              |   2 +-
 fs/file_table.c          |   2 +-
 kernel/sysctl.c          | 174 ++++++++++++++++++++++-------------------------
 4 files changed, 89 insertions(+), 97 deletions(-)
#1 lore raw

PATCH v2 1/3 sysctl: collapse redundant CONFIG_SYSCTL nesting in kernel/sysctl.c

After the removal of CONFIG_PROC_SYSCTL, several #ifdef CONFIG_SYSCTL blocks ended up nested inside the outer (now the same) CONFIG_SYSCTL guards.

Collapse them and remove a now-stale "/proc/sys support" comment which referred to CONFIG_PROC_SYSCTL.

---
 kernel/sysctl.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)
kernel/sysctl.c +2 -14
index f7b75985d542..d0a612c65b08 100644
@@ -29,14 +29,12 @@ EXPORT_SYMBOL(sysctl_vals);
 const unsigned long sysctl_long_vals[] = { 0, 1, LONG_MAX };
 EXPORT_SYMBOL_GPL(sysctl_long_vals);
 
-#if defined(CONFIG_SYSCTL)
+#ifdef CONFIG_SYSCTL
 
 /* Constants used for minimum and maximum */
 static const int ngroups_max = NGROUPS_MAX;
 static const int cap_last_cap = CAP_LAST_CAP;
 
-#ifdef CONFIG_SYSCTL
-
 /**
  * enum sysctl_writes_mode - supported sysctl write modes
  *
@@ -64,14 +62,6 @@ enum sysctl_writes_mode {
 };
 
 static enum sysctl_writes_mode sysctl_writes_strict = SYSCTL_WRITES_STRICT;
-#endif /* CONFIG_SYSCTL */
-#endif /* CONFIG_SYSCTL */
-
-/*
- * /proc/sys support
- */
-
-#ifdef CONFIG_SYSCTL
 
 static int _proc_do_string(char *data, int maxlen, int dir,
 		char *buffer, size_t *lenp, loff_t *ppos)
@@ -1366,7 +1356,7 @@ int proc_do_large_bitmap(const struct ctl_table *table, int dir,
 
 #endif /* CONFIG_SYSCTL */
 
-#if defined(CONFIG_SYSCTL)
+#ifdef CONFIG_SYSCTL
 int proc_do_static_key(const struct ctl_table *table, int dir,
 		       void *buffer, size_t *lenp, loff_t *ppos)
 {
@@ -1398,7 +1388,6 @@ int proc_do_static_key(const struct ctl_table *table, int dir,
 }
 
 static const struct ctl_table sysctl_subsys_table[] = {
-#ifdef CONFIG_SYSCTL
 	{
 		.procname	= "sysctl_writes_strict",
 		.data		= &sysctl_writes_strict,
@@ -1408,7 +1397,6 @@ static const struct ctl_table sysctl_subsys_table[] = {
 		.extra1		= SYSCTL_NEG_ONE,
 		.extra2		= SYSCTL_ONE,
 	},
-#endif
 	{
 		.procname	= "ngroups_max",
 		.data		= (void *)&ngroups_max,
Signature · 1 line
2.52.0
#2 lore raw

PATCH v2 2/3 sysctl: consolidate CONFIG_SYSCTL into a single block in kernel/sysctl.c

In reply to Oleg Nesterov

After the previous cleanup there are two CONFIG_SYSCTL blocks. Move the second block up into the first one and eliminate the redundant

Also add a proc_do_static_key() stub to the #else block. Not strictly necessary today, but consistent with the other stubs declared in include/linux/sysctl.h.

---
 kernel/sysctl.c | 160 +++++++++++++++++++++++++-----------------------
 1 file changed, 82 insertions(+), 78 deletions(-)
kernel/sysctl.c +82 -78
index d0a612c65b08..f6ea3eeeef65 100644
@@ -1255,6 +1255,87 @@ int proc_do_large_bitmap(const struct ctl_table *table, int dir,
 	return err;
 }
 
+int proc_do_static_key(const struct ctl_table *table, int dir,
+		       void *buffer, size_t *lenp, loff_t *ppos)
+{
+	struct static_key *key = (struct static_key *)table->data;
+	static DEFINE_MUTEX(static_key_mutex);
+	int val, ret;
+	struct ctl_table tmp = {
+		.data   = &val,
+		.maxlen = sizeof(val),
+		.mode   = table->mode,
+		.extra1 = SYSCTL_ZERO,
+		.extra2 = SYSCTL_ONE,
+	};
+
+	if (SYSCTL_USER_TO_KERN(dir) && !capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	mutex_lock(&static_key_mutex);
+	val = static_key_enabled(key);
+	ret = proc_dointvec_minmax(&tmp, dir, buffer, lenp, ppos);
+	if (SYSCTL_USER_TO_KERN(dir) && !ret) {
+		if (val)
+			static_key_enable(key);
+		else
+			static_key_disable(key);
+	}
+	mutex_unlock(&static_key_mutex);
+	return ret;
+}
+
+static const struct ctl_table sysctl_subsys_table[] = {
+	{
+		.procname	= "sysctl_writes_strict",
+		.data		= &sysctl_writes_strict,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_NEG_ONE,
+		.extra2		= SYSCTL_ONE,
+	},
+	{
+		.procname	= "ngroups_max",
+		.data		= (void *)&ngroups_max,
+		.maxlen		= sizeof (int),
+		.mode		= 0444,
+		.proc_handler	= proc_dointvec,
+	},
+	{
+		.procname	= "cap_last_cap",
+		.data		= (void *)&cap_last_cap,
+		.maxlen		= sizeof(int),
+		.mode		= 0444,
+		.proc_handler	= proc_dointvec,
+	},
+#ifdef CONFIG_SYSCTL_ARCH_UNALIGN_ALLOW
+	{
+		.procname	= "unaligned-trap",
+		.data		= &unaligned_enabled,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+	},
+#endif
+#ifdef CONFIG_SYSCTL_ARCH_UNALIGN_NO_WARN
+	{
+		.procname	= "ignore-unaligned-usertrap",
+		.data		= &no_unaligned_warning,
+		.maxlen		= sizeof (int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+	},
+#endif
+};
+
+int __init sysctl_init_bases(void)
+{
+	register_sysctl_init("kernel", sysctl_subsys_table);
+
+	return 0;
+}
+
 #else /* CONFIG_SYSCTL */
 
 int proc_dostring(const struct ctl_table *table, int dir,
@@ -1354,89 +1435,12 @@ int proc_do_large_bitmap(const struct ctl_table *table, int dir,
 	return -ENOSYS;
 }
 
-#endif /* CONFIG_SYSCTL */
-
-#ifdef CONFIG_SYSCTL
 int proc_do_static_key(const struct ctl_table *table, int dir,
 		       void *buffer, size_t *lenp, loff_t *ppos)
 {
-	struct static_key *key = (struct static_key *)table->data;
-	static DEFINE_MUTEX(static_key_mutex);
-	int val, ret;
-	struct ctl_table tmp = {
-		.data   = &val,
-		.maxlen = sizeof(val),
-		.mode   = table->mode,
-		.extra1 = SYSCTL_ZERO,
-		.extra2 = SYSCTL_ONE,
-	};
-
-	if (SYSCTL_USER_TO_KERN(dir) && !capable(CAP_SYS_ADMIN))
-		return -EPERM;
-
-	mutex_lock(&static_key_mutex);
-	val = static_key_enabled(key);
-	ret = proc_dointvec_minmax(&tmp, dir, buffer, lenp, ppos);
-	if (SYSCTL_USER_TO_KERN(dir) && !ret) {
-		if (val)
-			static_key_enable(key);
-		else
-			static_key_disable(key);
-	}
-	mutex_unlock(&static_key_mutex);
-	return ret;
+	return -ENOSYS;
 }
 
-static const struct ctl_table sysctl_subsys_table[] = {
-	{
-		.procname	= "sysctl_writes_strict",
-		.data		= &sysctl_writes_strict,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_minmax,
-		.extra1		= SYSCTL_NEG_ONE,
-		.extra2		= SYSCTL_ONE,
-	},
-	{
-		.procname	= "ngroups_max",
-		.data		= (void *)&ngroups_max,
-		.maxlen		= sizeof (int),
-		.mode		= 0444,
-		.proc_handler	= proc_dointvec,
-	},
-	{
-		.procname	= "cap_last_cap",
-		.data		= (void *)&cap_last_cap,
-		.maxlen		= sizeof(int),
-		.mode		= 0444,
-		.proc_handler	= proc_dointvec,
-	},
-#ifdef CONFIG_SYSCTL_ARCH_UNALIGN_ALLOW
-	{
-		.procname	= "unaligned-trap",
-		.data		= &unaligned_enabled,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
-	},
-#endif
-#ifdef CONFIG_SYSCTL_ARCH_UNALIGN_NO_WARN
-	{
-		.procname	= "ignore-unaligned-usertrap",
-		.data		= &no_unaligned_warning,
-		.maxlen		= sizeof (int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
-	},
-#endif
-};
-
-int __init sysctl_init_bases(void)
-{
-	register_sysctl_init("kernel", sysctl_subsys_table);
-
-	return 0;
-}
 #endif /* CONFIG_SYSCTL */
 /*
  * No sense putting this after each symbol definition, twice,
Signature · 1 line
2.52.0
#3 lore raw

PATCH v2 3/3 sysctl: remove redundant CONFIG_PROC_FS checks

In reply to Oleg Nesterov

Now that CONFIG_PROC_SYSCTL is gone, CONFIG_SYSCTL depends on CONFIG_PROC_FS directly in fs/proc/Kconfig, so the double guard defined(CONFIG_SYSCTL) && defined(CONFIG_PROC_FS) is redundant. Simplify to "#ifdef CONFIG_SYSCTL".

---
 drivers/parport/procfs.c | 6 +++---
 fs/dcache.c              | 2 +-
 fs/file_table.c          | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/parport/procfs.c +3 -3
index 3880460e67f2..1bfafc2a5724 100644
@@ -26,7 +26,7 @@
 
 #include <linux/uaccess.h>
 
-#if defined(CONFIG_SYSCTL) && defined(CONFIG_PROC_FS)
+#ifdef CONFIG_SYSCTL
 
 #define PARPORT_MIN_TIMESLICE_VALUE 1ul 
 #define PARPORT_MAX_TIMESLICE_VALUE ((unsigned long) HZ)
@@ -560,7 +560,7 @@ static void __exit parport_default_proc_unregister(void)
 	parport_bus_exit();
 }
 
-#else /* no sysctl or no procfs*/
+#else /* CONFIG_SYSCTL */
 
 int parport_proc_register(struct parport *pp)
 {
@@ -591,7 +591,7 @@ static void __exit parport_default_proc_unregister (void)
 {
 	parport_bus_exit();
 }
-#endif
+#endif /* CONFIG_SYSCTL */
 
 subsys_initcall(parport_default_proc_register)
 module_exit(parport_default_proc_unregister)
fs/dcache.c +1 -1
index 1b1a81f10da6..7ce421e9191f 100644
@@ -144,7 +144,7 @@ static DEFINE_PER_CPU(long, nr_dentry_unused);
 static DEFINE_PER_CPU(long, nr_dentry_negative);
 static int dentry_negative_policy;
 
-#if defined(CONFIG_SYSCTL) && defined(CONFIG_PROC_FS)
+#ifdef CONFIG_SYSCTL
 /* Statistics gathering. */
 static struct dentry_stat_t dentry_stat = {
 	.age_limit = 45,
fs/file_table.c +1 -1
index c68b8c0a4097..08ca3631b726 100644
@@ -122,7 +122,7 @@ unsigned long get_max_files(void)
 }
 EXPORT_SYMBOL_GPL(get_max_files);
 
-#if defined(CONFIG_SYSCTL) && defined(CONFIG_PROC_FS)
+#ifdef CONFIG_SYSCTL
 
 /*
  * Handle nr_files sysctl
Signature · 1 line
2.52.0
maintainer
#4 lore raw
In reply to Oleg Nesterov
Quoted 11 lines · Oleg Nesterov

Hi Joel,

I tried to address your comments to V1
https://lore.kernel.org/all/[email protected]/

Changes:

	2/3 - new

	3/3 - don't remove the extra whitespace in drivers/parport/procfs.c
Quoted 9 lines

Oleg.
--

 drivers/parport/procfs.c |   8 +--
 fs/dcache.c              |   2 +-
 fs/file_table.c          |   2 +-
 kernel/sysctl.c          | 174 ++++++++++++++++++++++-------------------------
 4 files changed, 89 insertions(+), 97 deletions(-)

This All looks good to me. Will put it in sysctl-next to see if it breaks something.

Thx