From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752555Ab1GGV3f (ORCPT ); Thu, 7 Jul 2011 17:29:35 -0400 Received: from smtp1.linux-foundation.org ([140.211.169.13]:51125 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752076Ab1GGV3e (ORCPT ); Thu, 7 Jul 2011 17:29:34 -0400 Date: Thu, 7 Jul 2011 14:29:22 -0700 From: Andrew Morton To: KAMEZAWA Hiroyuki Cc: "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , "nishimura@mxp.nes.nec.co.jp" , "bsingharora@gmail.com" , Michal Hocko , Ying Han Subject: Re: [PATCH][Cleanup] memcg: consolidates memory cgroup lru stat functions Message-Id: <20110707142922.c9657ec4.akpm@linux-foundation.org> In-Reply-To: <20110707155217.909c429a.kamezawa.hiroyu@jp.fujitsu.com> References: <20110707155217.909c429a.kamezawa.hiroyu@jp.fujitsu.com> X-Mailer: Sylpheed 3.0.2 (GTK+ 2.20.1; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 7 Jul 2011 15:52:17 +0900 KAMEZAWA Hiroyuki wrote: > In mm/memcontrol.c, there are many lru stat functions as.. > > mem_cgroup_zone_nr_lru_pages > mem_cgroup_node_nr_file_lru_pages > mem_cgroup_nr_file_lru_pages > mem_cgroup_node_nr_anon_lru_pages > mem_cgroup_nr_anon_lru_pages > mem_cgroup_node_nr_unevictable_lru_pages > mem_cgroup_nr_unevictable_lru_pages > mem_cgroup_node_nr_lru_pages > mem_cgroup_nr_lru_pages > mem_cgroup_get_local_zonestat > > Some of them are under #ifdef MAX_NUMNODES >1 and others are not. > This seems bad. This patch consolidates all functions into > > mem_cgroup_zone_nr_lru_pages() > mem_cgroup_node_nr_lru_pages() > mem_cgroup_nr_lru_pages() > > For these functions, "which LRU?" information is passed by a mask. > > example) > mem_cgroup_nr_lru_pages(mem, BIT(LRU_ACTIVE_ANON)) > > And I added some macro as ALL_LRU, ALL_LRU_FILE, ALL_LRU_ANON. > example) > mem_cgroup_nr_lru_pages(mem, ALL_LRU) > > BTW, considering layout of NUMA memory placement of counters, this patch seems > to be better. > > Now, when we gather all LRU information, we scan in following orer > for_each_lru -> for_each_node -> for_each_zone. > > This means we'll touch cache lines in different node in turn. > > After patch, we'll scan > for_each_node -> for_each_zone -> for_each_lru(mask) > > Then, we'll gather information in the same cacheline at once. mm/vmscan.c: In function 'zone_nr_lru_pages': mm/vmscan.c:175: warning: passing argument 2 of 'mem_cgroup_zone_nr_lru_pages' makes pointer from integer without a cast include/linux/memcontrol.h:307: note: expected 'struct zone *' but argument is of type 'int' mm/vmscan.c:175: error: too many arguments to function 'mem_cgroup_zone_nr_lru_pages' --- a/include/linux/memcontrol.h~memcg-consolidates-memory-cgroup-lru-stat-functions-fix +++ a/include/linux/memcontrol.h @@ -304,8 +304,8 @@ mem_cgroup_inactive_file_is_low(struct m } static inline unsigned long -mem_cgroup_zone_nr_lru_pages(struct mem_cgroup *memcg, struct zone *zone, - enum lru_list lru) +mem_cgroup_zone_nr_lru_pages(struct mem_cgroup *memcg, int nid, int zid, + unsigned int lru_mask) { return 0; } > +unsigned long > +mem_cgroup_zone_nr_lru_pages(struct mem_cgroup *mem, int nid, int zid, > + unsigned int lru_mask) The memcg code sometimes uses "struct mem_cgroup *mem" and sometimes uses "struct mem_cgroup *memcg". That's irritating. I think "memcg" is better.