Re: [PATCH 2/4] proc/sysctl: add shared variables -1
From: Xiaoming Ni
Date: Fri May 15 2020 - 22:32:38 EST
On 2020/5/16 0:05, Kees Cook wrote:
On Fri, May 15, 2020 at 05:06:28PM +0800, Xiaoming Ni wrote:
On 2020/5/15 16:06, Kees Cook wrote:
On Fri, May 15, 2020 at 12:33:42PM +0800, Xiaoming Ni wrote:
Add the shared variable SYSCTL_NEG_ONE to replace the variable neg_one
used in both sysctl_writes_strict and hung_task_warnings.
Signed-off-by: Xiaoming Ni <nixiaoming@xxxxxxxxxx>
---
fs/proc/proc_sysctl.c | 2 +-
include/linux/sysctl.h | 1 +
kernel/hung_task_sysctl.c | 3 +--
kernel/sysctl.c | 3 +--
How about doing this refactoring in advance of the extraction patch?
Before advance of the extraction patch, neg_one is only used in one file,
does it seem to have no value for refactoring?
I guess it doesn't matter much, but I think it's easier to review in the
sense that neg_one is first extracted and then later everything else is
moved.
Later, when more features sysctl interface is moved to the code file,
there will be more variables that need to be extracted.
So should I only extract the neg_one variable here, or should I extract
all the variables used by multiple features?
fs/proc/proc_sysctl.c | 2 +-
include/linux/sysctl.h | 11 ++++++++---
kernel/sysctl.c | 37 ++++++++++++++++---------------------
3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index b6f5d45..3f77e64 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -23,7 +23,7 @@
static const struct inode_operations proc_sys_dir_operations;
/* shared constants to be used in various sysctls */
-const int sysctl_vals[] = { 0, 1, INT_MAX };
+const int sysctl_vals[] = { -1, 0, 1, 2, 4, 100, 1000, INT_MAX };
EXPORT_SYMBOL(sysctl_vals);
/* Support for permanently empty directories */
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 43f8ef9..bf97c30 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -38,9 +38,14 @@
struct ctl_dir;
/* Keep the same order as in fs/proc/proc_sysctl.c */
-#define SYSCTL_ZERO ((void *)&sysctl_vals[0])
-#define SYSCTL_ONE ((void *)&sysctl_vals[1])
-#define SYSCTL_INT_MAX ((void *)&sysctl_vals[2])
+#define SYSCTL_NEG_ONE ((void *)&sysctl_vals[0])
+#define SYSCTL_ZERO ((void *)&sysctl_vals[1])
+#define SYSCTL_ONE ((void *)&sysctl_vals[2])
+#define SYSCTL_TWO ((void *)&sysctl_vals[3])
+#define SYSCTL_FOUR ((void *)&sysctl_vals[4])
+#define SYSCTL_ONE_HUNDRED ((void *)&sysctl_vals[5])
+#define SYSCTL_ONE_THOUSAND ((void *)&sysctl_vals[6])
+#define SYSCTL_INT_MAX ((void *)&sysctl_vals[7])
extern const int sysctl_vals[];
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 5dd6d01..efe6172 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -118,14 +118,9 @@
/* Constants used for minimum and maximum */
-static int __maybe_unused neg_one = -1;
-static int __maybe_unused two = 2;
-static int __maybe_unused four = 4;
static unsigned long zero_ul;
static unsigned long one_ul = 1;
static unsigned long long_max = LONG_MAX;
-static int one_hundred = 100;
-static int one_thousand = 1000;
#ifdef CONFIG_PRINTK
static int ten_thousand = 10000;
#endif
@@ -534,7 +529,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.maxlen = sizeof(int),
.mode = 0644,
.proc_handler = proc_dointvec_minmax,
- .extra1 = &neg_one,
+ .extra1 = SYSCTL_NEG_ONE,
.extra2 = SYSCTL_ONE,
},
#endif
@@ -865,7 +860,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0644,
.proc_handler = proc_dointvec_minmax_sysadmin,
.extra1 = SYSCTL_ZERO,
- .extra2 = &two,
+ .extra2 = SYSCTL_TWO,
},
#endif
{
@@ -1043,7 +1038,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0644,
.proc_handler = perf_cpu_time_max_percent_handler,
.extra1 = SYSCTL_ZERO,
- .extra2 = &one_hundred,
+ .extra2 = SYSCTL_ONE_HUNDRED,
},
{
.procname = "perf_event_max_stack",
@@ -1061,7 +1056,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0644,
.proc_handler = perf_event_max_stack_handler,
.extra1 = SYSCTL_ZERO,
- .extra2 = &one_thousand,
+ .extra2 = SYSCTL_ONE_THOUSAND,
},
#endif
{
@@ -1136,7 +1131,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0644,
.proc_handler = proc_dointvec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &two,
+ .extra2 = SYSCTL_TWO,
},
{
.procname = "panic_on_oom",
@@ -1145,7 +1140,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0644,
.proc_handler = proc_dointvec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &two,
+ .extra2 = SYSCTL_TWO,
},
{
.procname = "oom_kill_allocating_task",
@@ -1190,7 +1185,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0644,
.proc_handler = dirty_background_ratio_handler,
.extra1 = SYSCTL_ZERO,
- .extra2 = &one_hundred,
+ .extra2 = SYSCTL_ONE_HUNDRED,
},
{
.procname = "dirty_background_bytes",
@@ -1207,7 +1202,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0644,
.proc_handler = dirty_ratio_handler,
.extra1 = SYSCTL_ZERO,
- .extra2 = &one_hundred,
+ .extra2 = SYSCTL_ONE_HUNDRED,
},
{
.procname = "dirty_bytes",
@@ -1247,7 +1242,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0644,
.proc_handler = proc_dointvec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &one_hundred,
+ .extra2 = SYSCTL_ONE_HUNDRED,
},
#ifdef CONFIG_HUGETLB_PAGE
{
@@ -1304,7 +1299,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0200,
.proc_handler = drop_caches_sysctl_handler,
.extra1 = SYSCTL_ONE,
- .extra2 = &four,
+ .extra2 = SYSCTL_FOUR,
},
#ifdef CONFIG_COMPACTION
{
@@ -1357,7 +1352,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0644,
.proc_handler = watermark_scale_factor_sysctl_handler,
.extra1 = SYSCTL_ONE,
- .extra2 = &one_thousand,
+ .extra2 = SYSCTL_ONE_THOUSAND,
},
{
.procname = "percpu_pagelist_fraction",
@@ -1436,7 +1431,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0644,
.proc_handler = sysctl_min_unmapped_ratio_sysctl_handler,
.extra1 = SYSCTL_ZERO,
- .extra2 = &one_hundred,
+ .extra2 = SYSCTL_ONE_HUNDRED,
},
{
.procname = "min_slab_ratio",
@@ -1445,7 +1440,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0644,
.proc_handler = sysctl_min_slab_ratio_sysctl_handler,
.extra1 = SYSCTL_ZERO,
- .extra2 = &one_hundred,
+ .extra2 = SYSCTL_ONE_HUNDRED,
},
#endif
#ifdef CONFIG_SMP
@@ -1728,7 +1723,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0600,
.proc_handler = proc_dointvec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &two,
+ .extra2 = SYSCTL_TWO,
},
{
.procname = "protected_regular",
@@ -1737,7 +1732,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0600,
.proc_handler = proc_dointvec_minmax,
.extra1 = SYSCTL_ZERO,
- .extra2 = &two,
+ .extra2 = SYSCTL_TWO,
},
{
.procname = "suid_dumpable",
@@ -1746,7 +1741,7 @@ static int sysrq_sysctl_handler(struct ctl_table
*table, int write,
.mode = 0644,
.proc_handler = proc_dointvec_minmax_coredump,
.extra1 = SYSCTL_ZERO,
- .extra2 = &two,
+ .extra2 = SYSCTL_TWO,
},
#if defined(CONFIG_BINFMT_MISC) || defined(CONFIG_BINFMT_MISC_MODULE)
{