Hello, Glauber. On Tue, Apr 09, 2013 at 11:43:33AM +0400, Glauber Costa wrote: > 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". Yes, I know that :) It seems that I mislead you and you misunderstand what I want to say. Sorry for my poor English. I mean to say, changing when we attempt to shrink a small number of objects(below batch size) can affect some users of API and their system. Maybe they assume that if they have a little objects, shrinker will not call do_shrinker_shrink(). But, with this patch, although they have a little objects, shrinker call do_shrinker_shrink() at least once. Thanks. > > > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@xxxxxxxxx. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a> -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
