From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail144.messagelabs.com (mail144.messagelabs.com [216.82.254.51]) by kanga.kvack.org (Postfix) with ESMTP id BCB016B0071 for ; Fri, 19 Nov 2010 22:23:35 -0500 (EST) Received: from kpbe14.cbf.corp.google.com (kpbe14.cbf.corp.google.com [172.25.105.78]) by smtp-out.google.com with ESMTP id oAK3NU9h005660 for ; Fri, 19 Nov 2010 19:23:31 -0800 Received: from qyk33 (qyk33.prod.google.com [10.241.83.161]) by kpbe14.cbf.corp.google.com with ESMTP id oAK3NOSv002815 for ; Fri, 19 Nov 2010 19:23:29 -0800 Received: by qyk33 with SMTP id 33so230896qyk.2 for ; Fri, 19 Nov 2010 19:23:24 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20101119142552.df0e351c.akpm@linux-foundation.org> References: <1290054891-6097-1-git-send-email-yinghan@google.com> <20101118085921.GA11314@amd> <20101119142552.df0e351c.akpm@linux-foundation.org> Date: Fri, 19 Nov 2010 19:23:22 -0800 Message-ID: Subject: Re: [PATCH] Pass priority to shrink_slab From: Ying Han Content-Type: multipart/alternative; boundary=0016362839fa9e34730495738b4f Sender: owner-linux-mm@kvack.org To: Andrew Morton Cc: Nick Piggin , Mel Gorman , Minchan Kim , Rik van Riel , Hugh Dickins , Nick Piggin , linux-mm@kvack.org, Wu Fengguang List-ID: --0016362839fa9e34730495738b4f Content-Type: text/plain; charset=ISO-8859-1 On Fri, Nov 19, 2010 at 2:25 PM, Andrew Morton wrote: > On Thu, 18 Nov 2010 19:59:21 +1100 > Nick Piggin wrote: > > > On Wed, Nov 17, 2010 at 08:34:51PM -0800, Ying Han wrote: > > > Pass the reclaim priority down to the shrink_slab() which passes to the > > > shrink_icache_memory() for inode cache. It helps the situation when > > > shrink_slab() is being too agressive, it removes the inode as well as > all > > > the pages associated with the inode. Especially when single inode has > lots > > > of pages points to it. The application encounters performance hit when > > > that happens. > > > > > > The problem was observed on some workload we run, where it has small > number > > > of large files. Page reclaim won't blow away the inode which is pinned > by > > > dentry which in turn is pinned by open file descriptor. But if the > application > > > is openning and closing the fds, it has the chance to trigger the > issue. > > > > > > I have a script which reproduce the issue. The test is creating 1500 > empty > > > files and one big file in a cgroup. Then it starts adding memory > pressure > > > in the cgroup. Both before/after the patch we see the slab drops > (inode) in > > > slabinfo but the big file clean pages being preserves only after the > change. > > > > I was going to do this as a flag when nearing OOM. Is there a reason > > to have it priority based? That seems a little arbitrary to me... > > > > There are subtleties here. > > Take the case of a machine with 1MB lowmem and 8GB highmem. It has a > million cached inodes, each one with a single attached pagecache page. > The fairly common lots-of-small-files workload. > > The inodes are all in lowmem. Most of their pagecache is in highmem. > > To satisfy a GFP_KERNEL or GFP_USER allocation request, we need to free > up some of that lowmem. But none of those inodes are reclaimable, > because of their attached highmem pagecache. So in this case we very > much want to shoot down those inodes' pagecache within the icache > shrinker, so we can get those inodes reclaimed. > With the proposed change, that reclaim won't be happening until vmscan > has reached a higher priority. Which means that the VM will instead go > nuts reclaiming *other* lowmem objects. That means all the other slabs > which have shrinkers. It also means lowmem pagecache: those inodes > will cause all your filesystem metadata to get evicted. It also means > that anonymous memory which happened to land in lowmem will get swapped > out, and program text which is in lowmem will be unmapped and evicted. > > Thanks Andrew for your comments. The example makes sense to me although it seems to little bit rare. On the page reclaim path, we always try the page lru first and then the shrink slab since the latter one has no guarantee of freeing page. If the lowmem has user pages on the lru which could be reclaimed, preserving the slabs might not be a bed idea? And if the page lru has hard time to reclaim those pages, it will raise up the priority and in turn will affect the shrinker after the change. There may be other undesirable interactions as well - I'm not thinking > too hard at present ;) Thinking caps on, please. > > > I think the patch needs to be smarter. It should at least take > into account the *amount* of memory attached to the inode - > address_space.nr_pages. > Agree. The check of (priority > 0 && inode->i_data.nrpages) could potentially be improved. > > Where "amount" is a fuzzy concept - the shrinkers try to account for > seek cost and not just number-of-bytes, so that needs thinking about as > well. > > So what to do? I don't immediately see any alternative to implementing > reasonably comprehensive aging for inodes. Each time around the LRU > the inode gets aged. Each time it or its pages get touched, it gets > unaged. When considering an inode for eviction we look to see if > > fn(inode age) > fn(number of seeks to reestablish inode and its pagecache) > > Which is an interesting project ;) > > Interesting idea, but this also has the highmem and lowmem problem. > > And yes, we need a struct shrinker_control so we can fiddle with the > argument passing without having to edit lots of files each time. > Yes, and it would be much easier later to add a small feature (like this one) w/o touching so many files of the shrinkers. I am thinking if we can extend the scan_control from page reclaim and pass it down to the shrinker ? --Ying --0016362839fa9e34730495738b4f Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

On Fri, Nov 19, 2010 at 2:25 PM, Andrew = Morton <a= kpm@linux-foundation.org> wrote:
On Thu, 18 Nov 2010 19:59:21 +1100
Nick Piggin <npig= gin@kernel.dk> wrote:

> On Wed, Nov 17, 2010 at 08:34:51PM -0800, Ying= Han wrote:
> > Pass the reclaim priority down to the shrink_slab() which passes = to the
> > shrink_icache_memory() for inode cache. It helps the situation wh= en
> > shrink_slab() is being too agressive, it removes the inode as wel= l as all
> > the pages associated with the inode. Especially when single inode= has lots
> > of pages points to it. The application encounters performance hit= when
> > that happens.
> >
> > The problem was observed on some workload we run, where it has sm= all number
> > of large files. Page reclaim won't blow away the inode which = is pinned by
> > dentry which in turn is pinned by open file descriptor. But if th= e application
> > is openning and closing the fds, it has the chance to trigger the= issue.
> >
> > I have a script which reproduce the issue. The test is creating 1= 500 empty
> > files and one big file in a cgroup. Then it starts adding memory = pressure
> > in the cgroup. Both before/after the patch we see the slab drops = (inode) in
> > slabinfo but the big file clean pages being preserves only after = the change.
>
> I was going to do this as a flag when nearing OOM. Is there a reason > to have it priority based? That seems a little arbitrary to me...
>

There are subtleties here.

Take the case of a machine with 1MB lowmem and 8GB highmem. =A0It has a
million cached inodes, each one with a single attached pagecache page.
The fairly common lots-of-small-files workload.

The inodes are all in lowmem. =A0Most of their pagecache is in highmem.

To satisfy a GFP_KERNEL or GFP_USER allocation request, we need to free
up some of that lowmem. =A0But none of those inodes are reclaimable,
because of their attached highmem pagecache. =A0So in this case we very
much want to shoot down those inodes' pagecache within the icache
shrinker, so we can get those inodes reclaimed.


With the proposed change= , that reclaim won't be happening until vmscan
has reached a higher priority. =A0Which means that the VM will instead go nuts reclaiming *other* lowmem objects. =A0That means all the other slabs which have shrinkers. =A0It also means lowmem pagecache: those inodes
will cause all your filesystem metadata to get evicted. =A0It also means that anonymous memory which happened to land in lowmem will get swapped
out, and program text which is in lowmem will be unmapped and evicted.

Thanks Andrew for your comments. The example makes sens= e to me although it seems to
little bit rare.=A0
=A0
On the page reclaim path, we always try the page lru first and then the shr= ink slab since the latter one
has no=A0guarantee of freeing page.= If the lowmem has user pages on the lru which could be reclaimed,
preserving the slabs might not be a bed idea? And if the page lru has hard = time to reclaim those pages,
it will raise up the priority and in= turn will affect the shrinker after the change.=A0

There may be other undesirable interactions as well - I'm not thinking<= br> too hard at present ;) =A0Thinking caps on, please.


I think the patch needs to be smarter. =A0It should at least take
into account the *amount* of memory attached to the inode -
address_space.nr_pages.

Agree. The chec= k of=A0=A0(priority > 0 &am= p;& inode->i_data.nrpages)=A0could potentially=A0
be improved.

Where "amount" is a fuzzy concept - the shrinkers try to account = for
seek cost and not just number-of-bytes, so that needs thinking about as
well.

=A0=A0 So what to do? =A0I don't immediately see = any alternative to implementing
reasonably comprehensive aging for inodes. =A0Each time around the LRU
the inode gets aged. =A0Each time it or its pages get touched, it gets
unaged. =A0When considering an inode for eviction we look to see if

=A0fn(inode age) > fn(number of seeks to reestablish inode and its page= cache)

Which is an interesting project ;)

Interesting idea, but this also has the highmem and lowme= m problem.
=A0

And yes, we need a struct shrinker_control so we can fiddle with the
argument passing without having to edit lots of files each time.

Yes, and it would be much easier later to add a s= mall feature (like this one) w/o=A0
touching so many files of the= =A0shrinkers. I am thinking if we can extend the scan_control
from page reclaim=A0and pass it down to the shrinker ?

<= /div>
--Ying

--0016362839fa9e34730495738b4f-- -- 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 policy in Canada: sign http://dissolvethecrtc.ca/ Don't email: email@kvack.org