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    [from the cache]
©2003-2011 Jasper Spaans