"numa_stat" should not be included in the scope of CONFIG_HUGETLB_PAGE,
if CONFIG_HUGETLB_PAGE is not configured even if CONFIG_NUMA is configured,
"numa_stat" is missed form /proc. Remove it out of CONFIG_HUGETLB_PAGE
and move numa_stat sysctl handling to mm/vmstat.c.
Fixes: 4518085e127d ("mm, sysctl: make NUMA stats configurable")
Signed-off-by: Muchun Song <[email protected]>
Cc: <[email protected]>
---
include/linux/vmstat.h | 5 -----
kernel/sysctl.c | 9 ---------
mm/vmstat.c | 52 +++++++++++++++++++++++++-------------------------
3 files changed, 26 insertions(+), 40 deletions(-)
diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index bfe38869498d..1297a6b8ba23 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -13,12 +13,7 @@
extern int sysctl_stat_interval;
#ifdef CONFIG_NUMA
-#define ENABLE_NUMA_STAT 1
-#define DISABLE_NUMA_STAT 0
-extern int sysctl_vm_numa_stat;
DECLARE_STATIC_KEY_TRUE(vm_numa_stat_key);
-int sysctl_vm_numa_stat_handler(struct ctl_table *table, int write,
- void *buffer, size_t *length, loff_t *ppos);
#endif
struct reclaim_stat {
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index e52b6e372c60..3d6f36f230bb 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2107,15 +2107,6 @@ static struct ctl_table vm_table[] = {
.mode = 0644,
.proc_handler = &hugetlb_mempolicy_sysctl_handler,
},
- {
- .procname = "numa_stat",
- .data = &sysctl_vm_numa_stat,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = sysctl_vm_numa_stat_handler,
- .extra1 = SYSCTL_ZERO,
- .extra2 = SYSCTL_ONE,
- },
#endif
{
.procname = "hugetlb_shm_group",
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 373d2730fcf2..e10afbee888e 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -33,8 +33,6 @@
#include "internal.h"
#ifdef CONFIG_NUMA
-int sysctl_vm_numa_stat = ENABLE_NUMA_STAT;
-
/* zero numa counters within a zone */
static void zero_zone_numa_counters(struct zone *zone)
{
@@ -73,35 +71,37 @@ static void invalid_numa_statistics(void)
zero_global_numa_counters();
}
-static DEFINE_MUTEX(vm_numa_stat_lock);
-
-int sysctl_vm_numa_stat_handler(struct ctl_table *table, int write,
- void *buffer, size_t *length, loff_t *ppos)
+static int sysctl_numa_stat_handler(struct ctl_table *table, int write,
+ void *buffer, size_t *length, loff_t *ppos)
{
- int ret, oldval;
+ int ret;
+ struct static_key *key = table->data;
+ static DEFINE_MUTEX(lock);
- mutex_lock(&vm_numa_stat_lock);
- if (write)
- oldval = sysctl_vm_numa_stat;
- ret = proc_dointvec_minmax(table, write, buffer, length, ppos);
- if (ret || !write)
- goto out;
-
- if (oldval == sysctl_vm_numa_stat)
- goto out;
- else if (sysctl_vm_numa_stat == ENABLE_NUMA_STAT) {
- static_branch_enable(&vm_numa_stat_key);
- pr_info("enable numa statistics\n");
- } else {
- static_branch_disable(&vm_numa_stat_key);
+ mutex_lock(&lock);
+ ret = proc_do_static_key(table, write, buffer, length, ppos);
+ if (!ret && write && !static_key_enabled(key))
invalid_numa_statistics();
- pr_info("disable numa statistics, and clear numa counters\n");
- }
-
-out:
- mutex_unlock(&vm_numa_stat_lock);
+ mutex_unlock(&lock);
return ret;
}
+
+static struct ctl_table numa_stat_sysctl[] = {
+ {
+ .procname = "numa_stat",
+ .data = &vm_numa_stat_key.key,
+ .mode = 0644,
+ .proc_handler = sysctl_numa_stat_handler,
+ },
+ { }
+};
+
+static int __init numa_stat_sysctl_init(void)
+{
+ register_sysctl_init("vm", numa_stat_sysctl);
+ return 0;
+}
+late_initcall(numa_stat_sysctl_init);
#endif
#ifdef CONFIG_VM_EVENT_COUNTERS
--
2.11.0
On Thu 09-06-22 12:03:42, Muchun Song wrote:
> "numa_stat" should not be included in the scope of CONFIG_HUGETLB_PAGE,
> if CONFIG_HUGETLB_PAGE is not configured even if CONFIG_NUMA is configured,
> "numa_stat" is missed form /proc. Remove it out of CONFIG_HUGETLB_PAGE
> and move numa_stat sysctl handling to mm/vmstat.c.
>
> Fixes: 4518085e127d ("mm, sysctl: make NUMA stats configurable")
> Signed-off-by: Muchun Song <[email protected]>
> Cc: <[email protected]>
It really looks like numa_stat is misplaced but I am wondering why the
fix cannot be as simple as
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 830aaf8ca08e..b9c2cd9ed3a2 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2463,6 +2463,17 @@ static struct ctl_table vm_table[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_TWO_HUNDRED,
},
+#ifdef CONFIG_NUMA
+ {
+ .procname = "numa_stat",
+ .data = &sysctl_vm_numa_stat,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = sysctl_vm_numa_stat_handler,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
+#endif
#ifdef CONFIG_HUGETLB_PAGE
{
.procname = "nr_hugepages",
@@ -2479,15 +2490,6 @@ static struct ctl_table vm_table[] = {
.mode = 0644,
.proc_handler = &hugetlb_mempolicy_sysctl_handler,
},
- {
- .procname = "numa_stat",
- .data = &sysctl_vm_numa_stat,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = sysctl_vm_numa_stat_handler,
- .extra1 = SYSCTL_ZERO,
- .extra2 = SYSCTL_ONE,
- },
#endif
{
.procname = "hugetlb_shm_group",
--
Michal Hocko
SUSE Labs
On Thu, Jun 09, 2022 at 10:45:56AM +0200, Michal Hocko wrote:
> On Thu 09-06-22 12:03:42, Muchun Song wrote:
> > "numa_stat" should not be included in the scope of CONFIG_HUGETLB_PAGE,
> > if CONFIG_HUGETLB_PAGE is not configured even if CONFIG_NUMA is configured,
> > "numa_stat" is missed form /proc. Remove it out of CONFIG_HUGETLB_PAGE
> > and move numa_stat sysctl handling to mm/vmstat.c.
> >
> > Fixes: 4518085e127d ("mm, sysctl: make NUMA stats configurable")
> > Signed-off-by: Muchun Song <[email protected]>
> > Cc: <[email protected]>
>
> It really looks like numa_stat is misplaced but I am wondering why the
> fix cannot be as simple as
>
Your changes LGTM. I think I have overdone it. I really should seprate
this into two separate patches, one is bug fix (just like your changes),
another is simplify the code further. I'll rework this patch.
Thanks.