On Mon, Apr 21, 2014 at 09:50:07PM +0800, Zheng Liu wrote: > First question is about 'nr_to_scan'. Do you want me to generate a > patch to fix it in this merge window? Because I can imagine that this > patch should be trival and easy for reviewing. I looked at trying to create such a patch for this development cycle, and I decided not to try to fix this for 3.15 because it's a bit subtle. What we have in nr_to_scan is not broken per se. I'm not sure it's the best behaviour, but it is at least consistent. Right now, when nr_to_scan is zero, we are return to the shrinker the number of items which _can_ be shrunk (i.e., we don't include the extents subject to delalloc). Hence what we are effectively telling the shrinker is that the extents that are in the extent cache due to delalloc don't exist for the purposes of counting them. So if we don't count them when we report the number of items in the cache, then it's consistent that we don't count then when we tell the shrinker that we've scanned that many items. So if we change this, we should also change the number of items that we have in the cache (i.e., when the shrinker is calle with nr_to_scan is zero) to include _all_ of the shrinkers in the cache, and if we run out items, we need to return SHRINK_STOP. I suspect we should make this change, but it's not as simple patch as I had initially thought, and I think we'll want to be very careful in testing it to make sure it behaves correctly. So instead of including this as a bug fix for 3.15, I suspect it may be better to code and test it for 3.16, and if we're confident, we can include a cc: stable@xxxxxxxxxxxxxxx so it gets backported to 3.15.x. What do other folks think? > Second question is about your deeply thought. From your comment, it > seems that now we need a replacement algorithm that looks like we do in > memory management subsystem. Now in memory management subsystem, we > have an active list and an inactive list which tracks some pages. When > the system read/write some pages, these pages will be put on inactive > list. Then if some pages are accessed again, they will be promoted into > active list. When 'kswapd' thread tries to reclaim some pages, it will > drop some pages from inactive list and demote some pages from active > list to inactive list. I am happy to give it a try later. Yes, although we'll need to be careful that we don't introduce scalability problems caused by needing to take too many locks. So I don't think we want to move items from the inactive to active list when the extent cache is referenced. Instead, we'll probably want to bump a ref count in the cache entry, and then in the shrinker, when we scan the inactive list, we can check the ref count and decide then to move the item to the active list. That way, only the shrinker needs to take the global lock. Cheers, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
