From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752755AbcBEJ6g (ORCPT ); Fri, 5 Feb 2016 04:58:36 -0500 Received: from mx2.parallels.com ([199.115.105.18]:45491 "EHLO mx2.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752735AbcBEJ6d (ORCPT ); Fri, 5 Feb 2016 04:58:33 -0500 Date: Fri, 5 Feb 2016 12:58:21 +0300 From: Vladimir Davydov To: Johannes Weiner CC: Andrew Morton , Michal Hocko , , Subject: Re: [PATCH 1/3] mm: memcontrol: make tree_{stat,events} fetch all stats Message-ID: <20160205095821.GA29522@esperanza> References: <57ff0330b597738127ae0f9ca331016719bea7d8.1454589800.git.vdavydov@virtuozzo.com> <20160204204540.GD8208@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20160204204540.GD8208@cmpxchg.org> X-ClientProxiedBy: US-EXCH2.sw.swsoft.com (10.255.249.46) To US-EXCH.sw.swsoft.com (10.255.249.47) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 04, 2016 at 03:45:40PM -0500, Johannes Weiner wrote: > On Thu, Feb 04, 2016 at 04:03:37PM +0300, Vladimir Davydov wrote: > > Currently, tree_{stat,events} helpers can only get one stat index at a > > time, so when there are a lot of stats to be reported one has to call it > > over and over again (see memory_stat_show). This is neither effective, > > nor does it look good. Instead, let's make these helpers take a snapshot > > of all available counters. > > > > Signed-off-by: Vladimir Davydov > > This looks much better, and most of the callstacks involved here are > very flat, so the increased stack consumption should be alright. > > The only exception there is the threshold code, which can happen from > the direct reclaim path and thus with a fairly deep stack already. Yeah, I missed this case. Thought mem_cgroup_usage is only used for reporting to userspace. Thanks for catching this. > > Would it be better to leave mem_cgroup_usage() alone, open-code it, > and then use tree_stat() and tree_events() only for v2 memory.stat? > Definitely. diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 59f74074c04c..4f2afb9a2d67 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2745,14 +2745,20 @@ static void tree_events(struct mem_cgroup *memcg, unsigned long *events) static unsigned long mem_cgroup_usage(struct mem_cgroup *memcg, bool swap) { - unsigned long stat[MEMCG_NR_STAT]; - unsigned long val; + unsigned long val = 0; if (mem_cgroup_is_root(memcg)) { - tree_stat(memcg, stat); - val = stat[MEM_CGROUP_STAT_CACHE] + stat[MEM_CGROUP_STAT_RSS]; - if (swap) - val += stat[MEM_CGROUP_STAT_SWAP]; + struct mem_cgroup *iter; + + for_each_mem_cgroup_tree(iter, memcg) { + val += mem_cgroup_read_stat(iter, + MEM_CGROUP_STAT_CACHE); + val += mem_cgroup_read_stat(iter, + MEM_CGROUP_STAT_RSS); + if (swap) + val += mem_cgroup_read_stat(iter, + MEM_CGROUP_STAT_SWAP); + } } else { if (!swap) val = page_counter_read(&memcg->memory);