Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1732796 > unrolled thread

[PATCH 1/3] mm, sysctl: make VM stats configurable

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/3] mm, sysctl: make VM stats configurable Kemi Wang <kemi.wang@intel.com> - 2017-09-15 11:30 +0200
    Re: [PATCH 1/3] mm, sysctl: make VM stats configurable Michal Hocko <mhocko@kernel.org> - 2017-09-15 14:00 +0200
      Re: [PATCH 1/3] mm, sysctl: make VM stats configurable Dave Hansen <dave.hansen@linux.intel.com> - 2017-09-15 16:20 +0200
        Re: [PATCH 1/3] mm, sysctl: make VM stats configurable Michal Hocko <mhocko@kernel.org> - 2017-09-15 16:30 +0200
          Re: [PATCH 1/3] mm, sysctl: make VM stats configurable kemi <kemi.wang@intel.com> - 2017-09-18 04:50 +0200
            Re: [PATCH 1/3] mm, sysctl: make VM stats configurable Michal Hocko <mhocko@kernel.org> - 2017-09-18 08:00 +0200
      RE: [PATCH 1/3] mm, sysctl: make VM stats configurable "Wang, Kemi" <kemi.wang@intel.com> - 2017-09-16 04:20 +0200
      Re: [PATCH 1/3] mm, sysctl: make VM stats configurable kemi <kemi.wang@intel.com> - 2017-09-18 05:30 +0200
        Re: [PATCH 1/3] mm, sysctl: make VM stats configurable Michal Hocko <mhocko@kernel.org> - 2017-09-18 08:00 +0200

#1732796 — [PATCH 1/3] mm, sysctl: make VM stats configurable

This patch adds a tunable interface that allows VM stats configurable, as
suggested by Dave Hansen and Ying Huang.

When performance becomes a bottleneck and you can tolerate some possible
tool breakage and some decreased counter precision (e.g. numa counter), you
can do:
	echo [C|c]oarse > /proc/sys/vm/vmstat_mode

When performance is not a bottleneck and you want all tooling to work, you
can do:
	echo [S|s]trict > /proc/sys/vm/vmstat_mode

We recommend automatic detection of virtual memory statistics by system,
this is also system default configuration, you can do:
	echo [A|a]uto > /proc/sys/vm/vmstat_mode

The next patch handles numa statistics distinctively based-on different VM
stats mode.

Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Suggested-by: Dave Hansen <dave.hansen@intel.com>
Suggested-by: Ying Huang <ying.huang@intel.com>
Signed-off-by: Kemi Wang <kemi.wang@intel.com>
---
 include/linux/vmstat.h | 14 ++++++++++
 kernel/sysctl.c        |  7 +++++
 mm/vmstat.c            | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+)

diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index ade7cb5..c3634c7 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -9,6 +9,20 @@
 
 extern int sysctl_stat_interval;
 
+/*
+ * vmstat_mode:
+ * 0 = auto mode of vmstat, automatic detection of VM statistics.
+ * 1 = strict mode of vmstat, keep all VM statistics.
+ * 2 = coarse mode of vmstat, ignore unimportant VM statistics.
+ */
+#define VMSTAT_AUTO_MODE 0
+#define VMSTAT_STRICT_MODE  1
+#define VMSTAT_COARSE_MODE  2
+#define VMSTAT_MODE_LEN 16
+extern char sysctl_vmstat_mode[];
+extern int sysctl_vmstat_mode_handler(struct ctl_table *table, int write,
+		void __user *buffer, size_t *length, loff_t *ppos);
+
 #ifdef CONFIG_VM_EVENT_COUNTERS
 /*
  * Light weight per cpu counter implementation.
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 6648fbb..f5b813b 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1234,6 +1234,13 @@ static struct ctl_table kern_table[] = {
 
 static struct ctl_table vm_table[] = {
 	{
+		.procname	= "vmstat_mode",
+		.data		= &sysctl_vmstat_mode,
+		.maxlen		= VMSTAT_MODE_LEN,
+		.mode		= 0644,
+		.proc_handler	= sysctl_vmstat_mode_handler,
+	},
+	{
 		.procname	= "overcommit_memory",
 		.data		= &sysctl_overcommit_memory,
 		.maxlen		= sizeof(sysctl_overcommit_memory),
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 4bb13e7..e675ad2 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -32,6 +32,76 @@
 
 #define NUMA_STATS_THRESHOLD (U16_MAX - 2)
 
+int vmstat_mode = VMSTAT_AUTO_MODE;
+char sysctl_vmstat_mode[VMSTAT_MODE_LEN] = "auto";
+static const char *vmstat_mode_name[3] = {"auto", "strict", "coarse"};
+static DEFINE_MUTEX(vmstat_mode_lock);
+
+
+static int __parse_vmstat_mode(char *s)
+{
+	const char *str = s;
+
+	if (strcmp(str, "auto") == 0 || strcmp(str, "Auto") == 0)
+		vmstat_mode = VMSTAT_AUTO_MODE;
+	else if (strcmp(str, "strict") == 0 || strcmp(str, "Strict") == 0)
+		vmstat_mode = VMSTAT_STRICT_MODE;
+	else if (strcmp(str, "coarse") == 0 || strcmp(str, "Coarse") == 0)
+		vmstat_mode = VMSTAT_COARSE_MODE;
+	else {
+		pr_warn("Ignoring invalid vmstat_mode value: %s\n", s);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+int sysctl_vmstat_mode_handler(struct ctl_table *table, int write,
+		void __user *buffer, size_t *length, loff_t *ppos)
+{
+	char old_string[VMSTAT_MODE_LEN];
+	int ret, oldval;
+
+	mutex_lock(&vmstat_mode_lock);
+	if (write)
+		strncpy(old_string, (char *)table->data, VMSTAT_MODE_LEN);
+	ret = proc_dostring(table, write, buffer, length, ppos);
+	if (ret || !write) {
+		mutex_unlock(&vmstat_mode_lock);
+		return ret;
+	}
+
+	oldval = vmstat_mode;
+	if (__parse_vmstat_mode((char *)table->data)) {
+		/*
+		 * invalid sysctl_vmstat_mode value, restore saved string
+		 */
+		strncpy((char *)table->data, old_string, VMSTAT_MODE_LEN);
+		vmstat_mode = oldval;
+	} else {
+		/*
+		 * check whether vmstat mode changes or not
+		 */
+		if (vmstat_mode == oldval) {
+			/* no change */
+			mutex_unlock(&vmstat_mode_lock);
+			return 0;
+		} else if (vmstat_mode == VMSTAT_AUTO_MODE)
+			pr_info("vmstat mode changes from %s to auto mode\n",
+					vmstat_mode_name[oldval]);
+		else if (vmstat_mode == VMSTAT_STRICT_MODE)
+			pr_info("vmstat mode changes from %s to strict mode\n",
+					vmstat_mode_name[oldval]);
+		else if (vmstat_mode == VMSTAT_COARSE_MODE)
+			pr_info("vmstat mode changes from %s to coarse mode\n",
+					vmstat_mode_name[oldval]);
+		else
+			pr_warn("invalid vmstat_mode:%d\n", vmstat_mode);
+	}
+
+	mutex_unlock(&vmstat_mode_lock);
+	return 0;
+}
+
 #ifdef CONFIG_VM_EVENT_COUNTERS
 DEFINE_PER_CPU(struct vm_event_state, vm_event_states) = {{0}};
 EXPORT_PER_CPU_SYMBOL(vm_event_states);
-- 
2.7.4

[toc] | [next] | [standalone]

#1732838

On Fri 15-09-17 17:23:24, Kemi Wang wrote:
> This patch adds a tunable interface that allows VM stats configurable, as
> suggested by Dave Hansen and Ying Huang.
> 
> When performance becomes a bottleneck and you can tolerate some possible
> tool breakage and some decreased counter precision (e.g. numa counter), you
> can do:
> 	echo [C|c]oarse > /proc/sys/vm/vmstat_mode
> 
> When performance is not a bottleneck and you want all tooling to work, you
> can do:
> 	echo [S|s]trict > /proc/sys/vm/vmstat_mode
> 
> We recommend automatic detection of virtual memory statistics by system,
> this is also system default configuration, you can do:
> 	echo [A|a]uto > /proc/sys/vm/vmstat_mode
> 
> The next patch handles numa statistics distinctively based-on different VM
> stats mode.

I would just merge this with the second patch so that it is clear how
those modes are implemented. I am also wondering why cannot we have a
much simpler interface and implementation to enable/disable numa stats
(btw. sysctl_vm_numa_stats would be more descriptive IMHO).

Why do we need an auto-mode? Is it safe to enforce by default. Is it
possible that userspace can get confused to see 0 NUMA stats in the
first read while other allocation stats are non-zero?
 
> Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Suggested-by: Dave Hansen <dave.hansen@intel.com>
> Suggested-by: Ying Huang <ying.huang@intel.com>
> Signed-off-by: Kemi Wang <kemi.wang@intel.com>
> ---
>  include/linux/vmstat.h | 14 ++++++++++
>  kernel/sysctl.c        |  7 +++++
>  mm/vmstat.c            | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 91 insertions(+)
> 
> diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
> index ade7cb5..c3634c7 100644
> --- a/include/linux/vmstat.h
> +++ b/include/linux/vmstat.h
> @@ -9,6 +9,20 @@
>  
>  extern int sysctl_stat_interval;
>  
> +/*
> + * vmstat_mode:
> + * 0 = auto mode of vmstat, automatic detection of VM statistics.
> + * 1 = strict mode of vmstat, keep all VM statistics.
> + * 2 = coarse mode of vmstat, ignore unimportant VM statistics.
> + */
> +#define VMSTAT_AUTO_MODE 0
> +#define VMSTAT_STRICT_MODE  1
> +#define VMSTAT_COARSE_MODE  2
> +#define VMSTAT_MODE_LEN 16
> +extern char sysctl_vmstat_mode[];
> +extern int sysctl_vmstat_mode_handler(struct ctl_table *table, int write,
> +		void __user *buffer, size_t *length, loff_t *ppos);
> +
>  #ifdef CONFIG_VM_EVENT_COUNTERS
>  /*
>   * Light weight per cpu counter implementation.
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 6648fbb..f5b813b 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1234,6 +1234,13 @@ static struct ctl_table kern_table[] = {
>  
>  static struct ctl_table vm_table[] = {
>  	{
> +		.procname	= "vmstat_mode",
> +		.data		= &sysctl_vmstat_mode,
> +		.maxlen		= VMSTAT_MODE_LEN,
> +		.mode		= 0644,
> +		.proc_handler	= sysctl_vmstat_mode_handler,
> +	},
> +	{
>  		.procname	= "overcommit_memory",
>  		.data		= &sysctl_overcommit_memory,
>  		.maxlen		= sizeof(sysctl_overcommit_memory),
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 4bb13e7..e675ad2 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -32,6 +32,76 @@
>  
>  #define NUMA_STATS_THRESHOLD (U16_MAX - 2)
>  
> +int vmstat_mode = VMSTAT_AUTO_MODE;
> +char sysctl_vmstat_mode[VMSTAT_MODE_LEN] = "auto";
> +static const char *vmstat_mode_name[3] = {"auto", "strict", "coarse"};
> +static DEFINE_MUTEX(vmstat_mode_lock);
> +
> +
> +static int __parse_vmstat_mode(char *s)
> +{
> +	const char *str = s;
> +
> +	if (strcmp(str, "auto") == 0 || strcmp(str, "Auto") == 0)
> +		vmstat_mode = VMSTAT_AUTO_MODE;
> +	else if (strcmp(str, "strict") == 0 || strcmp(str, "Strict") == 0)
> +		vmstat_mode = VMSTAT_STRICT_MODE;
> +	else if (strcmp(str, "coarse") == 0 || strcmp(str, "Coarse") == 0)
> +		vmstat_mode = VMSTAT_COARSE_MODE;
> +	else {
> +		pr_warn("Ignoring invalid vmstat_mode value: %s\n", s);
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +int sysctl_vmstat_mode_handler(struct ctl_table *table, int write,
> +		void __user *buffer, size_t *length, loff_t *ppos)
> +{
> +	char old_string[VMSTAT_MODE_LEN];
> +	int ret, oldval;
> +
> +	mutex_lock(&vmstat_mode_lock);
> +	if (write)
> +		strncpy(old_string, (char *)table->data, VMSTAT_MODE_LEN);
> +	ret = proc_dostring(table, write, buffer, length, ppos);
> +	if (ret || !write) {
> +		mutex_unlock(&vmstat_mode_lock);
> +		return ret;
> +	}
> +
> +	oldval = vmstat_mode;
> +	if (__parse_vmstat_mode((char *)table->data)) {
> +		/*
> +		 * invalid sysctl_vmstat_mode value, restore saved string
> +		 */
> +		strncpy((char *)table->data, old_string, VMSTAT_MODE_LEN);
> +		vmstat_mode = oldval;
> +	} else {
> +		/*
> +		 * check whether vmstat mode changes or not
> +		 */
> +		if (vmstat_mode == oldval) {
> +			/* no change */
> +			mutex_unlock(&vmstat_mode_lock);
> +			return 0;
> +		} else if (vmstat_mode == VMSTAT_AUTO_MODE)
> +			pr_info("vmstat mode changes from %s to auto mode\n",
> +					vmstat_mode_name[oldval]);
> +		else if (vmstat_mode == VMSTAT_STRICT_MODE)
> +			pr_info("vmstat mode changes from %s to strict mode\n",
> +					vmstat_mode_name[oldval]);
> +		else if (vmstat_mode == VMSTAT_COARSE_MODE)
> +			pr_info("vmstat mode changes from %s to coarse mode\n",
> +					vmstat_mode_name[oldval]);
> +		else
> +			pr_warn("invalid vmstat_mode:%d\n", vmstat_mode);
> +	}
> +
> +	mutex_unlock(&vmstat_mode_lock);
> +	return 0;
> +}
> +
>  #ifdef CONFIG_VM_EVENT_COUNTERS
>  DEFINE_PER_CPU(struct vm_event_state, vm_event_states) = {{0}};
>  EXPORT_PER_CPU_SYMBOL(vm_event_states);
> -- 
> 2.7.4
> 

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1732906

On 09/15/2017 04:49 AM, Michal Hocko wrote:
> Why do we need an auto-mode? Is it safe to enforce by default.

Do we *need* it?  Not really.

But, it does offer the best of both worlds: The vast majority of users
see virtually no impact from the counters.  The minority that do need
them pay the cost *and* don't have to change their tooling at all.

> Is it> possible that userspace can get confused to see 0 NUMA stats in
the
> first read while other allocation stats are non-zero?

I doubt it.  Those counters are pretty worthless by themselves.  I have
tooling that goes and reads them, but it aways displays deltas.  Read
stats, sleep one second, read again, print the difference.

The only scenario I can see mattering is someone who is seeing a
performance issue due to NUMA allocation misses (or whatever) and wants
to go look *back* in the past.

A single-time printk could also go a long way to keeping folks from
getting confused.

[toc] | [prev] | [next] | [standalone]

#1732909

On Fri 15-09-17 07:16:23, Dave Hansen wrote:
> On 09/15/2017 04:49 AM, Michal Hocko wrote:
> > Why do we need an auto-mode? Is it safe to enforce by default.
> 
> Do we *need* it?  Not really.
> 
> But, it does offer the best of both worlds: The vast majority of users
> see virtually no impact from the counters.  The minority that do need
> them pay the cost *and* don't have to change their tooling at all.

Just to make it clear, I am not really opposing. It just adds some code
which we can safe... It is also rather chatty for something that can be
true/false.
 
> > Is it> possible that userspace can get confused to see 0 NUMA stats in
> the
> > first read while other allocation stats are non-zero?
> 
> I doubt it.  Those counters are pretty worthless by themselves.  I have
> tooling that goes and reads them, but it aways displays deltas.  Read
> stats, sleep one second, read again, print the difference.

This is how I use them as well.
 
> The only scenario I can see mattering is someone who is seeing a
> performance issue due to NUMA allocation misses (or whatever) and wants
> to go look *back* in the past.

yes

> A single-time printk could also go a long way to keeping folks from
> getting confused.

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1733621

On 2017年09月15日 22:28, Michal Hocko wrote:
> On Fri 15-09-17 07:16:23, Dave Hansen wrote:
>> On 09/15/2017 04:49 AM, Michal Hocko wrote:
>>> Why do we need an auto-mode? Is it safe to enforce by default.
>>
>> Do we *need* it?  Not really.
>>
>> But, it does offer the best of both worlds: The vast majority of users
>> see virtually no impact from the counters.  The minority that do need
>> them pay the cost *and* don't have to change their tooling at all.
> 
> Just to make it clear, I am not really opposing. It just adds some code
> which we can safe... It is also rather chatty for something that can be
> true/false.
> 

It has benefit, as Dave mentioned above.
Actually, it adds some coding complexity to provide a tuning interface with
on/off/auto mode. Using human-readable string instead of magic number makes
it easier to use, people probably don't need to review the ABI doc again
before using it. So, I don't think that should be a problem 
 
>>> Is it> possible that userspace can get confused to see 0 NUMA stats in
>> the
>>> first read while other allocation stats are non-zero?
>>
>> I doubt it.  Those counters are pretty worthless by themselves.  I have
>> tooling that goes and reads them, but it aways displays deltas.  Read
>> stats, sleep one second, read again, print the difference.
> 
> This is how I use them as well.
>  
>> The only scenario I can see mattering is someone who is seeing a
>> performance issue due to NUMA allocation misses (or whatever) and wants
>> to go look *back* in the past.
> 
> yes
> 

If it really matters, setting vmstat_mode=strict as a default option is a simple 
way to fix it. What's your idea? thanks

>> A single-time printk could also go a long way to keeping folks from
>> getting confused.
> 

[toc] | [prev] | [next] | [standalone]

#1733652

On Mon 18-09-17 10:44:52, kemi wrote:
> 
> 
> On 2017年09月15日 22:28, Michal Hocko wrote:
> > On Fri 15-09-17 07:16:23, Dave Hansen wrote:
> >> On 09/15/2017 04:49 AM, Michal Hocko wrote:
> >>> Why do we need an auto-mode? Is it safe to enforce by default.
> >>
> >> Do we *need* it?  Not really.
> >>
> >> But, it does offer the best of both worlds: The vast majority of users
> >> see virtually no impact from the counters.  The minority that do need
> >> them pay the cost *and* don't have to change their tooling at all.
> > 
> > Just to make it clear, I am not really opposing. It just adds some code
> > which we can safe... It is also rather chatty for something that can be
> > true/false.
> > 
> 
> It has benefit, as Dave mentioned above.
> Actually, it adds some coding complexity to provide a tuning interface with
> on/off/auto mode. Using human-readable string instead of magic number makes
> it easier to use, people probably don't need to review the ABI doc again
> before using it. So, I don't think that should be a problem 

Is this a thing that would be changed very often. I suspect that once
needed it will be set in a startup sysctl configuration and there will
be no further need to touch it again.

> >>> Is it> possible that userspace can get confused to see 0 NUMA stats in
> >> the
> >>> first read while other allocation stats are non-zero?
> >>
> >> I doubt it.  Those counters are pretty worthless by themselves.  I have
> >> tooling that goes and reads them, but it aways displays deltas.  Read
> >> stats, sleep one second, read again, print the difference.
> > 
> > This is how I use them as well.
> >  
> >> The only scenario I can see mattering is someone who is seeing a
> >> performance issue due to NUMA allocation misses (or whatever) and wants
> >> to go look *back* in the past.
> > 
> > yes
> > 
> 
> If it really matters, setting vmstat_mode=strict as a default option is a simple 
> way to fix it. What's your idea? thanks

Well, we are usually very conservative when changing the default
behavior. The primary reason why I was asking is that the auto mode
doesn't make much sense unless it is the default. I fully realize that
such an hypothetical breakage is really hard to envision but considering
it is more code to allow auto mode than a simple on/off (we have parsing
helpers for that AFAIR) then I would rather go with the simpler option.

This is up to you of course.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1733192

-----Original Message-----
From: Michal Hocko [mailto:mhocko@kernel.org] 
Sent: Friday, September 15, 2017 7:50 PM
To: Wang, Kemi <kemi.wang@intel.com>
Cc: Luis R . Rodriguez <mcgrof@kernel.org>; Kees Cook <keescook@chromium.org>; Andrew Morton <akpm@linux-foundation.org>; Jonathan Corbet <corbet@lwn.net>; Mel Gorman <mgorman@techsingularity.net>; Johannes Weiner <hannes@cmpxchg.org>; Christopher Lameter <cl@linux.com>; Sebastian Andrzej Siewior <bigeasy@linutronix.de>; Vlastimil Babka <vbabka@suse.cz>; Hillf Danton <hillf.zj@alibaba-inc.com>; Dave <dave.hansen@linux.intel.com>; Chen, Tim C <tim.c.chen@intel.com>; Kleen, Andi <andi.kleen@intel.com>; Jesper Dangaard Brouer <brouer@redhat.com>; Huang, Ying <ying.huang@intel.com>; Lu, Aaron <aaron.lu@intel.com>; Proc sysctl <linux-fsdevel@vger.kernel.org>; Linux MM <linux-mm@kvack.org>; Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/3] mm, sysctl: make VM stats configurable

On Fri 15-09-17 17:23:24, Kemi Wang wrote:
> This patch adds a tunable interface that allows VM stats configurable, as
> suggested by Dave Hansen and Ying Huang.
> 
> When performance becomes a bottleneck and you can tolerate some possible
> tool breakage and some decreased counter precision (e.g. numa counter), you
> can do:
> 	echo [C|c]oarse > /proc/sys/vm/vmstat_mode
> 
> When performance is not a bottleneck and you want all tooling to work, you
> can do:
> 	echo [S|s]trict > /proc/sys/vm/vmstat_mode
> 
> We recommend automatic detection of virtual memory statistics by system,
> this is also system default configuration, you can do:
> 	echo [A|a]uto > /proc/sys/vm/vmstat_mode
> 
> The next patch handles numa statistics distinctively based-on different VM
> stats mode.

I would just merge this with the second patch so that it is clear how
those modes are implemented. I am also wondering why cannot we have a
much simpler interface and implementation to enable/disable numa stats
(btw. sysctl_vm_numa_stats would be more descriptive IMHO).

The motivation is that we propose a general tunable  interface for VM stats.
This would be more scalable, since we don't have to add an individual
Interface for each type of counter that can be configurable.
In the second patch, NUMA stats, as an example, can benefit for that.

If you still hold your idea, I don't mind to merge them together.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1733629

On 2017年09月15日 19:49, Michal Hocko wrote:
> On Fri 15-09-17 17:23:24, Kemi Wang wrote:
>> This patch adds a tunable interface that allows VM stats configurable, as
>> suggested by Dave Hansen and Ying Huang.
>>
>> When performance becomes a bottleneck and you can tolerate some possible
>> tool breakage and some decreased counter precision (e.g. numa counter), you
>> can do:
>> 	echo [C|c]oarse > /proc/sys/vm/vmstat_mode
>>
>> When performance is not a bottleneck and you want all tooling to work, you
>> can do:
>> 	echo [S|s]trict > /proc/sys/vm/vmstat_mode
>>
>> We recommend automatic detection of virtual memory statistics by system,
>> this is also system default configuration, you can do:
>> 	echo [A|a]uto > /proc/sys/vm/vmstat_mode
>>
>> The next patch handles numa statistics distinctively based-on different VM
>> stats mode.
> 
> I would just merge this with the second patch so that it is clear how
> those modes are implemented. I am also wondering why cannot we have a
> much simpler interface and implementation to enable/disable numa stats
> (btw. sysctl_vm_numa_stats would be more descriptive IMHO).
> 

Apologize for resending it, because I found my previous reply mixed with
Michal's in many email client.

The motivation is that we propose a general tunable  interface for VM stats.
This would be more scalable, since we don't have to add an individual
Interface for each type of counter that can be configurable.
In the second patch, NUMA stats, as an example, can benefit for that.
If you still hold your idea, I don't mind to merge them together.

[toc] | [prev] | [next] | [standalone]

#1733655

On Mon 18-09-17 11:22:37, kemi wrote:
> 
> 
> On 2017年09月15日 19:49, Michal Hocko wrote:
> > On Fri 15-09-17 17:23:24, Kemi Wang wrote:
> >> This patch adds a tunable interface that allows VM stats configurable, as
> >> suggested by Dave Hansen and Ying Huang.
> >>
> >> When performance becomes a bottleneck and you can tolerate some possible
> >> tool breakage and some decreased counter precision (e.g. numa counter), you
> >> can do:
> >> 	echo [C|c]oarse > /proc/sys/vm/vmstat_mode
> >>
> >> When performance is not a bottleneck and you want all tooling to work, you
> >> can do:
> >> 	echo [S|s]trict > /proc/sys/vm/vmstat_mode
> >>
> >> We recommend automatic detection of virtual memory statistics by system,
> >> this is also system default configuration, you can do:
> >> 	echo [A|a]uto > /proc/sys/vm/vmstat_mode
> >>
> >> The next patch handles numa statistics distinctively based-on different VM
> >> stats mode.
> > 
> > I would just merge this with the second patch so that it is clear how
> > those modes are implemented. I am also wondering why cannot we have a
> > much simpler interface and implementation to enable/disable numa stats
> > (btw. sysctl_vm_numa_stats would be more descriptive IMHO).
> > 
> 
> Apologize for resending it, because I found my previous reply mixed with
> Michal's in many email client.
> 
> The motivation is that we propose a general tunable  interface for VM stats.
> This would be more scalable, since we don't have to add an individual
> Interface for each type of counter that can be configurable.

Can you envision which other counters would fall into the same category?

> In the second patch, NUMA stats, as an example, can benefit for that.
> If you still hold your idea, I don't mind to merge them together.

Well, I would prefer simplicy in the first place.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web