lkml.org 
[lkml]   [2017]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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).

    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

    \
     
     \ /
      Last update: 2017-09-15 13:50    [W:4.788 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site