linux-ext4 - Re: [PATCH 1/2 RESEND] fix return value for mb_cache_shrink_fn when nr_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 20 Jul 2010 11:49:52 -0500
From:	Eric Sandeen <sandeen@...hat.com>
To:	Wang Sheng-Hui <crosslonelyover@...il.com>
CC:	linux-fsdevel@...r.kernel.org, viro@...iv.linux.org.uk,
	linux-mm@...ck.org, linux-ext4 <linux-ext4@...r.kernel.org>,
	kernel-janitors <kernel-janitors@...r.kernel.org>,
	Christoph Hellwig <hch@...radead.org>
Subject: Re: [PATCH 1/2 RESEND] fix return value for mb_cache_shrink_fn when
 nr_to_scan > 0

Eric Sandeen wrote:

...

> Reviewed-by: Eric Sandeen <sandeen@...hat.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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html