Re: [PATCH V2 2/2] change shrinker API by passing shrink_control struct

From: Ying Han <yinghan@google.com>
To: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Nick Piggin <nickpiggin@yahoo.com.au>,
	Minchan Kim <minchan.kim@gmail.com>,
	Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>,
	Balbir Singh <balbir@linux.vnet.ibm.com>,
	Tejun Heo <tj@kernel.org>, Pavel Emelyanov <xemul@openvz.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Li Zefan <lizf@cn.fujitsu.com>, Mel Gorman <mel@csn.ul.ie>,
	Rik van Riel <riel@redhat.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Hugh Dickins <hughd@google.com>, Michal Hocko <mhocko@suse.cz>,
	Dave Hansen <dave@linux.vnet.ibm.com>,
	Zhu Yanhai <zhu.yanhai@gmail.com>,
	linux-mm@kvack.org
Subject: Re: [PATCH V2 2/2] change shrinker API by passing shrink_control struct
Date: Tue, 26 Apr 2011 18:15:52 -0700	[thread overview]
Message-ID: <BANLkTin+UxRdkPDdu322jqSv1FV6WSYtYg@mail.gmail.com> (raw)
In-Reply-To: <20110427094902.D170.A69D9226@jp.fujitsu.com>

On Tue, Apr 26, 2011 at 5:47 PM, KOSAKI Motohiro
<kosaki.motohiro@jp.fujitsu.com> wrote:
>> > >  {
>> > >       struct xfs_mount *mp;
>> > >       struct xfs_perag *pag;
>> > >       xfs_agnumber_t  ag;
>> > >       int             reclaimable;
>> > > +     int nr_to_scan = sc->nr_slab_to_reclaim;
>> > > +     gfp_t gfp_mask = sc->gfp_mask;
>> >
>> > And, this very near meaning field .nr_scanned and .nr_slab_to_reclaim
>> > poped up new question.
>> > Why don't we pass more clever slab shrinker target? Why do we need pass
>> > similar two argument?
>> >
>>
>> I renamed the nr_slab_to_reclaim and nr_scanned in shrink struct.
>
> Oh no. that's not naming issue. example, Nick's previous similar patch pass
> zone-total-pages and how-much-scanned-pages. (ie shrink_slab don't calculate
> current magical target scanning objects anymore)
>        ie,  "4 *  max_pass  * (scanned / nr- lru_pages-in-zones)"
>
> Instead, individual shrink_slab callback calculate this one.
> see git://git.kernel.org/pub/scm/linux/kernel/git/npiggin/linux-npiggin.git
>
> I'm curious why you change the design from another guy's previous very similar effort and
> We have to be convinced which is better.

Thank you for the pointer. My patch is intended to consolidate all
existing parameters passed from reclaim code
to the shrinker.

Talked w/ Nick and Andrew from last LSF,  we agree that this patch
will be useful for other extensions later which allows us easily
adding extensions to the shrinkers without shrinker files. Nick and I
talked about the effort later to pass the nodemask down to the
shrinker. He is cc-ed in the thread. Another thing I would like to
repost is to add the reclaim priority down to the shrinker, which we
won't throw tons of page caches pages by reclaiming one inode slab
object.

--Ying

>
>
>
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>