From mboxrd@z Thu Jan 1 00:00:00 1970 From: Glauber Costa Subject: Re: [PATCH v2 02/28] vmscan: take at least one pass with shrinkers Date: Tue, 9 Apr 2013 11:43:33 +0400 Message-ID: <5163C6A5.5050307@parallels.com> References: <1364548450-28254-1-git-send-email-glommer@parallels.com> <1364548450-28254-3-git-send-email-glommer@parallels.com> <20130408084202.GA21654@lge.com> <51628412.6050803@parallels.com> <20130408090131.GB21654@lge.com> <51628877.5000701@parallels.com> <20130409005547.GC21654@lge.com> <20130409012931.GE17758@dastard> <20130409020505.GA4218@lge.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Cc: Dave Chinner , , , , Michal Hocko , Johannes Weiner , , Andrew Morton , Greg Thelen , , , Theodore Ts'o , Al Viro To: Joonsoo Kim Return-path: In-Reply-To: <20130409020505.GA4218@lge.com> Sender: owner-linux-mm@kvack.org List-Id: linux-fsdevel.vger.kernel.org On 04/09/2013 06:05 AM, Joonsoo Kim wrote: > I don't think so. > Yes, lowmem_shrink() return number of (in)active lru pages > when nr_to_scan is 0. And in shrink_slab(), we divide it by lru_pages. > lru_pages can vary where shrink_slab() is called, anyway, perhaps this > logic makes total_scan below 128. > You may benefit from looking at the lowmemory patches in this patchset itself. We modified the shrinker API to separate the count and scan phases. With this, the whole nr_to_scan == 0 disappears and the code gets easier to follow. >> > >> > And, interestingly enough, when the file cache has been pruned down >> > to it's smallest possible size, that's when the shrinker *won't run* >> > because the that's when the total_scan will be smaller than the >> > batch size and hence shrinker won't get called. >> > >> > The shrinker is hacky, abuses the shrinker API, and doesn't appear >> > to do what it is intended to do. You need to fix the shrinker, not >> > use it's brokenness as an excuse to hold up a long overdue shrinker >> > rework. > Agreed. I also think shrinker rework is valuable and I don't want > to become a stopper for this change. But, IMHO, at least, we should > notify users of shrinker API to know how shrinker API behavior changed, Except that the behavior didn't change. > because this is unexpected behavior change when they used this API. > When they used this API, they can assume that it is possible to control > logic with seeks and return value(when nr_to_scan=0), but with this patch, > this assumption is broken. > Jonsoo, you are still missing the point. nr_to_scan=0 has nothing to do with this, or with this patch. nr_to_scan will reach 0 ANYWAY if you shrink all objects you have to shrink, which is a *very* common thing to happen. The only case changed here is where this happen when attempting to shrink a small number of objects that is smaller than the batch size. Also, again, the nr_to_scan=0 checks in the shrinker calls have nothing to do with that. They reflect the situation *BEFORE* the shrinker was called. So how many objects we shrunk afterwards have zero to do with it. This is just the shrinker API using the magic value of 0 to mean : "don't shrink, just tell me how much do you have", vs a positive number meaning "try to shrink as much as nr_to_scan objects". -- 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