Re: [PATCH 1/2 RESEND] fix return value for mb_cache_shrink_fn when nr_to_scan > 0

[Eric Sandeen <sandeen@redhat.com>]

Eric Sandeen wrote:

...

> Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Actually retract that, as Andreas pointed out:

>>  fs/mbcache.c |   22 +++++++++++-----------
>>  1 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/mbcache.c b/fs/mbcache.c
>> index ec88ff3..5697d9e 100644
>> --- a/fs/mbcache.c
>> +++ b/fs/mbcache.c
>> @@ -201,21 +201,13 @@ mb_cache_shrink_fn(int nr_to_scan, gfp_t gfp_mask)
>>  {
>>  	LIST_HEAD(free_list);
>>  	struct list_head *l, *ltmp;
>> +	struct mb_cache *cache;
>>  	int count = 0;
>>
>> -	spin_lock(&mb_cache_spinlock);

you've lost this spin_lock ...

>> -	list_for_each(l, &mb_cache_list) {
>> -		struct mb_cache *cache =
>> -			list_entry(l, struct mb_cache, c_cache_list);
>> -		mb_debug("cache %s (%d)", cache->c_name,
>> -			  atomic_read(&cache->c_entry_count));
>> -		count += atomic_read(&cache->c_entry_count);
>> -	}
>>  	mb_debug("trying to free %d entries", nr_to_scan);
>> -	if (nr_to_scan == 0) {
>> -		spin_unlock(&mb_cache_spinlock);
>> +	if (nr_to_scan == 0)
>>  		goto out;
>> -	}
>> +

and here you're iterating over it while unlocked....

>>  	while (nr_to_scan-- && !list_empty(&mb_cache_lru_list)) {
>>  		struct mb_cache_entry *ce =
>>  			list_entry(mb_cache_lru_list.next,
                                   struct mb_cache_entry, e_lru_list);
                list_move_tail(&ce->e_lru_list, &free_list);
                __mb_cache_entry_unhash(ce);
        }
        spin_unlock(&mb_cache_spinlock);

.... and here you unlock an unlocked spinlock.

Sorry I missed that.

-Eric

>> @@ -229,6 +221,14 @@ mb_cache_shrink_fn(int nr_to_scan, gfp_t gfp_mask)
>>  						   e_lru_list), gfp_mask);
>>  	}
>>  out:
>> +	spin_lock(&mb_cache_spinlock);
>> +	list_for_each_entry(cache, &mb_cache_list, c_cache_list) {
>> +		mb_debug("cache %s (%d)", cache->c_name,
>> +			  atomic_read(&cache->c_entry_count));
>> +		count += atomic_read(&cache->c_entry_count);
>> +	}
>> +	spin_unlock(&mb_cache_spinlock);
>> +
>>  	return (count / 100) * sysctl_vfs_cache_pressure;
>>  }
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html