Re: [PATCH v7 19/34] drivers: convert shrinkers to new count/scan API

[Glauber Costa <glommer@parallels.com>]

On 06/04/2013 12:03 AM, Kent Overstreet wrote:
>> -	for (i = 0; nr && i < c->bucket_cache_used; i++) {
>> > +	for (i = 0; i < c->bucket_cache_used; i++) {
> This is a bug (but it's probably more my fault for writing it too subtly
> in the first place): previously, we broke out of the loop when nr
> reached 0 (and we'd freed all the objects we were asked to).
> 
> After your change it doesn't break out of the loop until trying to free
> _everything_ - which will break things very badly since this causes us
> to free our reserve. You'll want a if (freed >= nr) break; like you
> added in the previous loop.
> 
> (The reserve should be documented here too though, I'll write a patch
> for that...)
> 
Just please notice the following:

This came up a while ago in a discussion in the fs conversion patch.
But nr to scan is the number of objects we as you to *scan*, not to free.

previously, you would only decrement nr when you could free the object.

Since I need to fix the problem anyway here of looping through all of
them, this is what I intend to write:

        for (i = 0; (nr--) && i < c->bucket_cache_used; i++) {
		[ ... ]
        }

This won't test "freed" at all. Shout if you disagree.


--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>