From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx144.postini.com [74.125.245.144]) by kanga.kvack.org (Postfix) with SMTP id 11E796B025B for ; Thu, 2 May 2013 09:37:59 -0400 (EDT) From: Glauber Costa Subject: Re: [PATCH v4 17/31] drivers: convert shrinkers to new count/scan API Date: Thu, 2 May 2013 13:37:54 +0000 Message-ID: References: <1367018367-11278-1-git-send-email-glommer@openvz.org> <1367018367-11278-18-git-send-email-glommer@openvz.org> <20130430215355.GN6415@suse.de> <20130430220050.GK9931@google.com>,<20130502093744.GJ11497@suse.de> In-Reply-To: <20130502093744.GJ11497@suse.de> Reply-To: Glauber Costa Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_xm7djq7r1xfsf4uo0hohgyxj1367501887374emailandroidcom_" MIME-Version: 1.0 Sender: owner-linux-mm@kvack.org List-ID: To: "mgorman@suse.de" , "koverstreet@google.com" Cc: "glommer@openvz.org" , "linux-mm@kvack.org" , "cgroups@vger.kernel.org" , "akpm@linux-foundation.org" , "gthelen@google.com" , "kamezawa.hiroyu@jp.fujitsu.com" , "mhocko@suse.cz" , "hannes@cmpxchg.org" , "dchinner@redhat.com" , "intel-gfx@lists.freedesktop.org" , "dri-devel@lists.freedesktop.org" , "devel@driverdev.osuosl.org" , "dan.magenheimer@oracle.com" --_000_xm7djq7r1xfsf4uo0hohgyxj1367501887374emailandroidcom_ Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: quoted-printable Sorry for the following crappy message. I came travelling without my laptop= . Please note that one of my patches implement one shot shrinkers onto of vmp= ressure mechanism. It can still be called frequently, because right now it = is called every time userspace would get an event. But at least it won't it= erate. You can try investigating if that interface suits your i915 needs better Sent by Samsung Mobile -------- Original message -------- From: Mel Gorman Date: To: Kent Overstreet Cc: Glauber Costa ,linux-mm@kvack.org,cgroups@vger.kern= el.org,Andrew Morton ,Greg Thelen ,kamezawa.hiroyu@jp.fujitsu.com,Michal Hocko ,Johanne= s Weiner ,Dave Chinner ,intel-gfx@= lists.freedesktop.org,dri-devel@lists.freedesktop.org,devel@driverdev.osuos= l.org,Dan Magenheimer Subject: Re: [PATCH v4 17/31] drivers: convert shrinkers to new count/scan = API On Tue, Apr 30, 2013 at 03:00:50PM -0700, Kent Overstreet wrote: > On Tue, Apr 30, 2013 at 10:53:55PM +0100, Mel Gorman wrote: > > On Sat, Apr 27, 2013 at 03:19:13AM +0400, Glauber Costa wrote: > > > diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c > > > index 03e44c1..8b9c1a6 100644 > > > --- a/drivers/md/bcache/btree.c > > > +++ b/drivers/md/bcache/btree.c > > > @@ -599,11 +599,12 @@ static int mca_reap(struct btree *b, struct clo= sure *cl, unsigned min_order) > > > return 0; > > > } > > > > > > -static int bch_mca_shrink(struct shrinker *shrink, struct shrink_con= trol *sc) > > > +static long bch_mca_scan(struct shrinker *shrink, struct shrink_cont= rol *sc) > > > { > > > struct cache_set *c =3D container_of(shrink, struct cache_set, shr= ink); > > > struct btree *b, *t; > > > unsigned long i, nr =3D sc->nr_to_scan; > > > + long freed =3D 0; > > > > > > if (c->shrinker_disabled) > > > return 0; > > > > -1 if shrinker disabled? > > > > Otherwise if the shrinker is disabled we ultimately hit this loop in > > shrink_slab_one() > > My memory is very hazy on this stuff, but I recall there being another > loop that'd just spin if we always returned -1. > > (It might've been /proc/sys/vm/drop_caches, or maybe that was another > bug..) > It might be worth chasing down what that bug was and fixing it. > But 0 should certainly be safe - if we're always returning 0, then we're > claiming we don't have anything to shrink. > It won't crash, but in Glauber's current code, it'll call you a few more times uselessly and the scanned statistics become misleading. I think Glauber/Dave's series is a big improvement over what we currently have and it would be nice to get it ironed out. -- Mel Gorman SUSE Labs --_000_xm7djq7r1xfsf4uo0hohgyxj1367501887374emailandroidcom_ Content-Type: text/html; charset="iso-8859-15" Content-Transfer-Encoding: quoted-printable
Sorry for the following crappy message. I came travelling without my l= aptop.

Please note that one of my patches implement one shot shrinkers onto o= f vmpressure mechanism. It can still be called frequently, because right no= w it is called every time userspace would get an event. But at least it won= 't iterate.

You can try investigating if that interface suits your i915 needs bett= er


Sent by Samsung Mobile



-------- Original message --------
From: Mel Gorman <mgorman@suse.de>
Date:
To: Kent Overstreet <koverstreet@google.com>
Cc: Glauber Costa <glommer@openvz.org>,linux-mm@kvack.org,cgroups@vge= r.kernel.org,Andrew Morton <akpm@linux-foundation.org>,Greg Thelen &l= t;gthelen@google.com>,kamezawa.hiroyu@jp.fujitsu.com,Michal Hocko <mh= ocko@suse.cz>,Johannes Weiner <hannes@cmpxchg.org>,Dave Chinner <dchinner@redhat.com>,intel-gfx@lists.freedesktop.org,dri-de= vel@lists.freedesktop.org,devel@driverdev.osuosl.org,Dan Magenheimer <da= n.magenheimer@oracle.com>
Subject: Re: [PATCH v4 17/31] drivers: convert shrinkers to new count/scan = API


On Tue, Apr 30, 2013 at 03:00:50PM -0700, Kent Ove= rstreet wrote:
> On Tue, Apr 30, 2013 at 10:53:55PM +0100, Mel Gorman wrote:
> > On Sat, Apr 27, 2013 at 03:19:13AM +0400, Glauber Costa wrote= :
> > > diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/b= tree.c
> > > index 03e44c1..8b9c1a6 100644
> > > --- a/drivers/md/bcache/btree.c
> > > +++ b/drivers/md/bcache/btree.c
> > > @@ -599,11 +599,12 @@ static int mca_reap(struct btree *= b, struct closure *cl, unsigned min_order)
> > >    return 0;
> > >  }
> > > 
> > > -static int bch_mca_shrink(struct shrinker *shrink, struct s= hrink_control *sc)
> > > +static long bch_mca_scan(struct shrinker *shrink, struc= t shrink_control *sc)
> > >  {
> > >    struct cache_set *c =3D container_of(shrin= k, struct cache_set, shrink);
> > >    struct btree *b, *t;
> > >    unsigned long i, nr =3D sc->nr_to_scan;=
> > > + long freed =3D 0;
> > > 
> > >    if (c->shrinker_disabled)
> > >          &= nbsp; return 0;
> >
> > -1 if shrinker disabled?
> >
> > Otherwise if the shrinker is disabled we ultimately hit this loop= in
> > shrink_slab_one()
>
> My memory is very hazy on this stuff, but I recall there being another=
> loop that'd just spin if we always returned -1.
>
> (It might've been /proc/sys/vm/drop_caches, or maybe that was another<= br> > bug..)
>

It might be worth chasing down what that bug was and fixing it.

> But 0 should certainly be safe - if we're always returning 0, then we'= re
> claiming we don't have anything to shrink.
>

It won't crash, but in Glauber's current code, it'll call you a few more times uselessly and the scanned statistics become misleading. I think
Glauber/Dave's series is a big improvement over what we currently have
and it would be nice to get it ironed out.

--
Mel Gorman
SUSE Labs
 --_000_xm7djq7r1xfsf4uo0hohgyxj1367501887374emailandroidcom_-- -- 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/ . Don't email: email@kvack.org