2023-03-09 04:39:03

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH] mm: memory-failure: Move memory failure sysctls to its own file

The sysctl_memory_failure_early_kill and memory_failure_recovery
are only used in memory-failure.c, move them to its own file.

Signed-off-by: Kefeng Wang <[email protected]>
---
include/linux/mm.h | 2 --
kernel/sysctl.c | 20 --------------------
mm/memory-failure.c | 36 ++++++++++++++++++++++++++++++++++--
3 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 573db4266885..3ed739c79026 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3583,8 +3583,6 @@ int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
extern int memory_failure(unsigned long pfn, int flags);
extern void memory_failure_queue_kick(int cpu);
extern int unpoison_memory(unsigned long pfn);
-extern int sysctl_memory_failure_early_kill;
-extern int sysctl_memory_failure_recovery;
extern void shake_page(struct page *p);
extern atomic_long_t num_poisoned_pages __read_mostly;
extern int soft_offline_page(unsigned long pfn, int flags);
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 932a5c196ab2..e6d73ee5917b 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2240,26 +2240,6 @@ static struct ctl_table vm_table[] = {
.proc_handler = proc_dointvec,
.extra1 = SYSCTL_ZERO,
},
-#endif
-#ifdef CONFIG_MEMORY_FAILURE
- {
- .procname = "memory_failure_early_kill",
- .data = &sysctl_memory_failure_early_kill,
- .maxlen = sizeof(sysctl_memory_failure_early_kill),
- .mode = 0644,
- .proc_handler = proc_dointvec_minmax,
- .extra1 = SYSCTL_ZERO,
- .extra2 = SYSCTL_ONE,
- },
- {
- .procname = "memory_failure_recovery",
- .data = &sysctl_memory_failure_recovery,
- .maxlen = sizeof(sysctl_memory_failure_recovery),
- .mode = 0644,
- .proc_handler = proc_dointvec_minmax,
- .extra1 = SYSCTL_ZERO,
- .extra2 = SYSCTL_ONE,
- },
#endif
{
.procname = "user_reserve_kbytes",
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index fae9baf3be16..d3edfffb3f07 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -62,13 +62,14 @@
#include <linux/page-isolation.h>
#include <linux/pagewalk.h>
#include <linux/shmem_fs.h>
+#include <linux/sysctl.h>
#include "swap.h"
#include "internal.h"
#include "ras/ras_event.h"

-int sysctl_memory_failure_early_kill __read_mostly = 0;
+static int sysctl_memory_failure_early_kill __read_mostly = 0;

-int sysctl_memory_failure_recovery __read_mostly = 1;
+static int sysctl_memory_failure_recovery __read_mostly = 1;

atomic_long_t num_poisoned_pages __read_mostly = ATOMIC_LONG_INIT(0);

@@ -122,6 +123,37 @@ const struct attribute_group memory_failure_attr_group = {
.attrs = memory_failure_attr,
};

+#ifdef CONFIG_SYSCTL
+static struct ctl_table memory_failure_table[] = {
+ {
+ .procname = "memory_failure_early_kill",
+ .data = &sysctl_memory_failure_early_kill,
+ .maxlen = sizeof(sysctl_memory_failure_early_kill),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
+ {
+ .procname = "memory_failure_recovery",
+ .data = &sysctl_memory_failure_recovery,
+ .maxlen = sizeof(sysctl_memory_failure_recovery),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
+ { }
+};
+
+static int __init memory_failure_sysctl_init(void)
+{
+ register_sysctl_init("vm", memory_failure_table);
+ return 0;
+}
+late_initcall(memory_failure_sysctl_init);
+#endif /* CONFIG_SYSCTL */
+
/*
* Return values:
* 1: the page is dissolved (if needed) and taken off from buddy,
--
2.35.3



Subject: Re: [PATCH] mm: memory-failure: Move memory failure sysctls to its own file

On Thu, Mar 09, 2023 at 12:59:24PM +0800, Kefeng Wang wrote:
> The sysctl_memory_failure_early_kill and memory_failure_recovery
> are only used in memory-failure.c, move them to its own file.

Thank you for the patch.

Could you explain the benefit to move them?
We seem to have many other parameters in kernel/sysctl.c which are used
only in single places, so why do we handle these two differently?

Thanks,
Naoya Horiguchi

2023-03-09 06:03:49

by Kefeng Wang

[permalink] [raw]
Subject: Re: [PATCH] mm: memory-failure: Move memory failure sysctls to its own file



On 2023/3/9 13:14, HORIGUCHI NAOYA(堀口 直也) wrote:
> On Thu, Mar 09, 2023 at 12:59:24PM +0800, Kefeng Wang wrote:
>> The sysctl_memory_failure_early_kill and memory_failure_recovery
>> are only used in memory-failure.c, move them to its own file.
>
> Thank you for the patch.
>
> Could you explain the benefit to move them?
> We seem to have many other parameters in kernel/sysctl.c which are used
> only in single places, so why do we handle these two differently?
>

Actually, all of them need to be moved into theirs own file as required
by proc sysctl maintainer, see [1]

> Thanks,
> Naoya Horiguchi

[1] https://lore.kernel.org/all/[email protected]/

Subject: Re: [PATCH] mm: memory-failure: Move memory failure sysctls to its own file

On Thu, Mar 09, 2023 at 02:03:39PM +0800, Kefeng Wang wrote:
>
>
> On 2023/3/9 13:14, HORIGUCHI NAOYA(堀口 直也) wrote:
> > On Thu, Mar 09, 2023 at 12:59:24PM +0800, Kefeng Wang wrote:
> > > The sysctl_memory_failure_early_kill and memory_failure_recovery
> > > are only used in memory-failure.c, move them to its own file.
> >
> > Thank you for the patch.
> >
> > Could you explain the benefit to move them?
> > We seem to have many other parameters in kernel/sysctl.c which are used
> > only in single places, so why do we handle these two differently?
> >
>
> Actually, all of them need to be moved into theirs own file as required
> by proc sysctl maintainer, see [1]

Thank you for clarification, so now I agree with the change.
It seems that checkpatch.pl shows the following error, so could
you resolve this?

ERROR: do not initialise statics to 0
#300: FILE: mm/memory-failure.c:70:
+static int sysctl_memory_failure_early_kill __read_mostly = 0;

With this change, ...

Acked-by: Naoya Horiguchi <[email protected]>

Thank you very much.

2023-03-10 03:37:04

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH v2] mm: memory-failure: Move memory failure sysctls to its own file

The sysctl_memory_failure_early_kill and memory_failure_recovery
are only used in memory-failure.c, move them to its own file.

Acked-by: Naoya Horiguchi <[email protected]>
Signed-off-by: Kefeng Wang <[email protected]>
---
v2: add Ack and do not initialise statics to 0
include/linux/mm.h | 2 --
kernel/sysctl.c | 20 --------------------
mm/memory-failure.c | 35 +++++++++++++++++++++++++++++++++--
3 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index ce1590933995..4765aedee1d4 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3587,8 +3587,6 @@ int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
extern int memory_failure(unsigned long pfn, int flags);
extern void memory_failure_queue_kick(int cpu);
extern int unpoison_memory(unsigned long pfn);
-extern int sysctl_memory_failure_early_kill;
-extern int sysctl_memory_failure_recovery;
extern void shake_page(struct page *p);
extern atomic_long_t num_poisoned_pages __read_mostly;
extern int soft_offline_page(unsigned long pfn, int flags);
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index ce0297acf97c..0a8a3c9c82e4 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2350,26 +2350,6 @@ static struct ctl_table vm_table[] = {
.proc_handler = proc_dointvec,
.extra1 = SYSCTL_ZERO,
},
-#endif
-#ifdef CONFIG_MEMORY_FAILURE
- {
- .procname = "memory_failure_early_kill",
- .data = &sysctl_memory_failure_early_kill,
- .maxlen = sizeof(sysctl_memory_failure_early_kill),
- .mode = 0644,
- .proc_handler = proc_dointvec_minmax,
- .extra1 = SYSCTL_ZERO,
- .extra2 = SYSCTL_ONE,
- },
- {
- .procname = "memory_failure_recovery",
- .data = &sysctl_memory_failure_recovery,
- .maxlen = sizeof(sysctl_memory_failure_recovery),
- .mode = 0644,
- .proc_handler = proc_dointvec_minmax,
- .extra1 = SYSCTL_ZERO,
- .extra2 = SYSCTL_ONE,
- },
#endif
{
.procname = "user_reserve_kbytes",
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index fae9baf3be16..6c007b7730fe 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -62,13 +62,14 @@
#include <linux/page-isolation.h>
#include <linux/pagewalk.h>
#include <linux/shmem_fs.h>
+#include <linux/sysctl.h>
#include "swap.h"
#include "internal.h"
#include "ras/ras_event.h"

-int sysctl_memory_failure_early_kill __read_mostly = 0;
+static int sysctl_memory_failure_early_kill __read_mostly;

-int sysctl_memory_failure_recovery __read_mostly = 1;
+static int sysctl_memory_failure_recovery __read_mostly = 1;

atomic_long_t num_poisoned_pages __read_mostly = ATOMIC_LONG_INIT(0);

@@ -122,6 +123,36 @@ const struct attribute_group memory_failure_attr_group = {
.attrs = memory_failure_attr,
};

+#ifdef CONFIG_SYSCTL
+static struct ctl_table memory_failure_table[] = {
+ {
+ .procname = "memory_failure_early_kill",
+ .data = &sysctl_memory_failure_early_kill,
+ .maxlen = sizeof(sysctl_memory_failure_early_kill),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
+ {
+ .procname = "memory_failure_recovery",
+ .data = &sysctl_memory_failure_recovery,
+ .maxlen = sizeof(sysctl_memory_failure_recovery),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
+};
+
+static int __init memory_failure_sysctl_init(void)
+{
+ register_sysctl_init("vm", memory_failure_table);
+ return 0;
+}
+late_initcall(memory_failure_sysctl_init);
+#endif /* CONFIG_SYSCTL */
+
/*
* Return values:
* 1: the page is dissolved (if needed) and taken off from buddy,
--
2.35.3