linux-kernel - Re: [RFC] memcg: move definitions to .h and inline some functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090819142705.GN22626@balbir.in.ibm.com>
Date:	Wed, 19 Aug 2009 19:57:05 +0530
From:	Balbir Singh <balbir@...ux.vnet.ibm.com>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Cc:	Wu Fengguang <fengguang.wu@...el.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Rik van Riel <riel@...hat.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Avi Kivity <avi@...hat.com>,
	Andrea Arcangeli <aarcange@...hat.com>,
	"Dike, Jeffrey G" <jeffrey.g.dike@...el.com>,
	"Yu, Wilfred" <wilfred.yu@...el.com>,
	"Kleen, Andi" <andi.kleen@...el.com>,
	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christoph Lameter <cl@...ux-foundation.org>,
	Mel Gorman <mel@....ul.ie>,
	LKML <linux-kernel@...r.kernel.org>,
	linux-mm <linux-mm@...ck.org>,
	"nishimura@....nes.nec.co.jp" <nishimura@....nes.nec.co.jp>,
	"lizf@...fujitsu.com" <lizf@...fujitsu.com>,
	"menage@...gle.com" <menage@...gle.com>
Subject: Re: [RFC] memcg: move definitions to .h and inline some functions

* KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com> [2009-08-19 23:18:01]:

> Wu Fengguang ?$B$5$s$O=q$-$^$7$?!'
> > On Tue, Aug 18, 2009 at 11:57:52PM +0800, KOSAKI Motohiro wrote:
> >>
> >> > > This one of the reasons why we unconditionally deactivate
> >> > > the active anon pages, and do background scanning of the
> >> > > active anon list when reclaiming page cache pages.
> >> > >
> >> > > We want to always move some pages to the inactive anon
> >> > > list, so it does not get too small.
> >> >
> >> > Right, the current code tries to pull inactive list out of
> >> > smallish-size state as long as there are vmscan activities.
> >> >
> >> > However there is a possible (and tricky) hole: mem cgroups
> >> > don't do batched vmscan. shrink_zone() may call shrink_list()
> >> > with nr_to_scan=1, in which case shrink_list() _still_ calls
> >> > isolate_pages() with the much larger SWAP_CLUSTER_MAX.
> >> >
> >> > It effectively scales up the inactive list scan rate by 10 times when
> >> > it is still small, and may thus prevent it from growing up for ever.
> >> >
> >> > In that case, LRU becomes FIFO.
> >> >
> >> > Jeff, can you confirm if the mem cgroup's inactive list is small?
> >> > If so, this patch should help.
> >>
> >> This patch does right thing.
> >> However, I would explain why I and memcg folks didn't do that in past
> >> days.
> >>
> >> Strangely, some memcg struct declaration is hide in *.c. Thus we can't
> >> make inline function and we hesitated to introduce many function calling
> >> overhead.
> >>
> >> So, Can we move some memcg structure declaration to *.h and make
> >> mem_cgroup_get_saved_scan() inlined function?
> >
> > OK here it is. I have to move big chunks to make it compile, and it
> > does reduced a dozen lines of code :)
> >
> > Is this big copy&paste acceptable? (memcg developers CCed).
> >
> > Thanks,
> > Fengguang
> 
> I don't like this. plz add hooks to necessary places, at this stage.
> This will be too big for inlined function, anyway.
> plz move this after you find overhead is too big.

Me too.. I want to abstract the implementation within memcontrol.c to
be honest (I am concerned that someone might include memcontrol.h and
access its structure members, which scares me). Hiding it within
memcontrol.c provides the right level of abstraction.

Could you please explain your motivation for this change? I got cc'ed
on to a few emails, is this for the patch that export nr_save_scanned
approach?

-- 
	Balbir
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/