lkml.org 
[lkml]   [2011]   [Dec]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: KSM: numa awareness sysfs knob
    On Wed, 30 Nov 2011, Andrew Morton wrote:

    > Date: Wed, 30 Nov 2011 15:47:19 -0800
    > From: Andrew Morton <akpm@linux-foundation.org>
    > To: Petr Holasek <pholasek@redhat.com>
    > Cc: Hugh Dickins <hughd@google.com>, Andrea Arcangeli
    > <aarcange@redhat.com>, linux-kernel@vger.kernel.org, linux-mm@kvack.org,
    > Anton Arapov <anton@redhat.com>
    > Subject: Re: [PATCH] [RFC] KSM: numa awareness sysfs knob
    >
    > On Wed, 30 Nov 2011 11:37:26 +0100
    > Petr Holasek <pholasek@redhat.com> wrote:
    >
    > > Introduce a new sysfs knob /sys/kernel/mm/ksm/max_node_dist, whose
    > > value will be used as the limitation for node distance of merged pages.
    > >
    >
    > The changelog doesn't really describe why you think Linux needs this
    > feature? What's the reasoning? Use cases? What value does it provide?

    Typical use-case could be a lot of KVM guests on NUMA machine and cpus from
    more distant nodes would have significant increase of access latency to the
    merged ksm page. I chose sysfs knob for higher scalability.

    >
    > > index b392e49..b882140 100644
    > > --- a/Documentation/vm/ksm.txt
    > > +++ b/Documentation/vm/ksm.txt
    > > @@ -58,6 +58,10 @@ sleep_millisecs - how many milliseconds ksmd should sleep before next scan
    > > e.g. "echo 20 > /sys/kernel/mm/ksm/sleep_millisecs"
    > > Default: 20 (chosen for demonstration purposes)
    > >
    > > +max_node_dist - maximum node distance between two pages which could be
    > > + merged.
    > > + Default: 255 (without any limitations)
    >
    > And this doesn't explain to our users why they might want to alter it,
    > and what effects they would see from doing so. Maybe that's obvious to
    > them...

    Now I can't figure out more extensive description of this feature, but we
    could explain it deeply, of course.

    >
    > > run - set 0 to stop ksmd from running but keep merged pages,
    > > set 1 to run ksmd e.g. "echo 1 > /sys/kernel/mm/ksm/run",
    > > set 2 to stop ksmd and unmerge all pages currently merged,
    > >
    > > ...
    > >
    > > +#ifdef CONFIG_NUMA
    > > +static inline int node_dist(int from, int to)
    > > +{
    > > + int dist = node_distance(from, to);
    > > +
    > > + return dist == -1 ? 0 : dist;
    > > +}
    >
    > So I spent some time grubbing around trying to work out what a return
    > value of -1 from node_distance() means, and wasn't successful. Perhaps
    > an explanatory comment here would have helped.

    Yes, apologize, my mistake. I've overlooked that it should never return value
    lower than LOCAL_DISTANCE even with CONFIG_NUMA unset. So those wrappers are
    pointless and I'll remove them.

    >
    > > +#else
    > > +static inline int node_dist(int from, int to)
    > > +{
    > > + return 0;
    > > +}
    > > +#endif
    > >
    > > ...
    > >
    > > +static ssize_t max_node_dist_store(struct kobject *kobj,
    > > + struct kobj_attribute *attr,
    > > + const char *buf, size_t count)
    > > +{
    > > + int err;
    > > + unsigned long node_dist;
    > > +
    > > + err = kstrtoul(buf, 10, &node_dist);
    > > + if (err || node_dist > 255)
    > > + return -EINVAL;
    >
    > If kstrtoul() returned an errno we should propagate that back rather than
    > overwriting it with -EINVAL.

    Ok, I'll fix it.

    >
    > > + ksm_node_distance = node_dist;
    > > +
    > > + return count;
    > > +}
    > >
    > > ...
    > >
    >


    \
     
     \ /
      Last update: 2011-12-01 11:19    [W:0.030 / U:0.904 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site