Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754988Ab2F3M3i (ORCPT ); Sat, 30 Jun 2012 08:29:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36080 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754249Ab2F3M3h (ORCPT ); Sat, 30 Jun 2012 08:29:37 -0400 Date: Sat, 30 Jun 2012 14:29:23 +0200 From: Petr Holasek To: Andrew Morton Cc: Hugh Dickins , Andrea Arcangeli , Chris Wright , Izik Eidus , Rik van Riel , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Anton Arapov Subject: Re: [PATCH v2] KSM: numa awareness sysfs knob Message-ID: <20120630122919.GB3036@stainedmachine.redhat.com> References: <1340970592-25001-1-git-send-email-pholasek@redhat.com> <20120629141759.3312b49e.akpm@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120629141759.3312b49e.akpm@linux-foundation.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3022 Lines: 89 On Fri, 29 Jun 2012, Andrew Morton wrote: > On Fri, 29 Jun 2012 13:49:52 +0200 > Petr Holasek wrote: > > > Introduces new sysfs boolean knob /sys/kernel/mm/ksm/merge_nodes > > which control merging pages across different numa nodes. > > When it is set to zero only pages from the same node are merged, > > otherwise pages from all nodes can be merged together (default behavior). > > > > 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. Sysfs knob was choosen > > for higher scalability. > > > > Every numa node has its own stable & unstable trees because > > of faster searching and inserting. Changing of merge_nodes > > value is possible only when there are not any ksm shared pages in system. > > It would be neat to have a knob which enables KSM for all anon > mappings. ie: pretend that MADV_MERGEABLE is always set. For testing > coverage purposes. Interesting idea, I'll try to add it in next release if /sys/kernel/mm/ksm directory is the right place for such debug knob. > > --- a/Documentation/vm/ksm.txt > > +++ b/Documentation/vm/ksm.txt > > @@ -58,6 +58,12 @@ 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) > > > > +merge_nodes - specifies if pages from different numa nodes can be merged. > > + When set to 0, ksm merges only pages which physically > > + resides in the memory area of same NUMA node. It brings > > + lower latency to access to shared page. > > + Default: 1 > > s/resides/reside/. > > This doc should mention that /sys/kernel/mm/ksm/run should be zeroed to > alter merge_nodes. Otherwise confusion will reign. > Oh, forgot to mention it. I'll fix it. > > > > ... > > > > +static ssize_t merge_nodes_store(struct kobject *kobj, > > + struct kobj_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + int err; > > + unsigned long knob; > > + > > + err = kstrtoul(buf, 10, &knob); > > + if (err) > > + return err; > > + if (knob > 1) > > + return -EINVAL; > > + > > + if (ksm_run & KSM_RUN_MERGE) > > + return -EBUSY; > > + > > + mutex_lock(&ksm_thread_mutex); > > + if (ksm_merge_nodes != knob) { > > + if (ksm_pages_shared > 0) > > + return -EBUSY; > > + else > > + ksm_merge_nodes = knob; > > + } > > + mutex_unlock(&ksm_thread_mutex); > > + > > + return count; > > +} > > Seems a bit racy. Shouldn't the test of ksm_run be inside the locked > region? > Agreed. Thanks for your review! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/