linux-kernel - Re: [PATCH 2/8] mm/swap: Add cluster lock

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:   Wed, 28 Sep 2016 16:51:18 +0800
From:   "Huang\, Ying" <ying.huang@...el.com>
To:     Hillf Danton <hillf.zj@...baba-inc.com>
Cc:     <tim.c.chen@...ux.intel.com>,
        'Andrew Morton' <akpm@...ux-foundation.org>,
        <dave.hansen@...el.com>, <andi.kleen@...el.com>,
        <aaron.lu@...el.com>, <linux-mm@...ck.org>,
        <linux-kernel@...r.kernel.org>,
        'Huang Ying' <ying.huang@...el.com>,
        'Hugh Dickins' <hughd@...gle.com>,
        'Shaohua Li' <shli@...nel.org>,
        'Minchan Kim' <minchan@...nel.org>,
        'Rik van Riel' <riel@...hat.com>,
        'Andrea Arcangeli' <aarcange@...hat.com>,
        "'Kirill A . Shutemov'" <kirill.shutemov@...ux.intel.com>,
        'Vladimir Davydov' <vdavydov@...tuozzo.com>,
        'Johannes Weiner' <hannes@...xchg.org>,
        "'Michal Hocko'" <mhocko@...nel.org>
Subject: Re: [PATCH 2/8] mm/swap: Add cluster lock

Hillf Danton <hillf.zj@...baba-inc.com> writes:

> On Wednesday, September 28, 2016 1:18 AM Tim Chen wrote
>> 
>> @@ -447,8 +505,9 @@ static void scan_swap_map_try_ssd_cluster(struct swap_info_struct *si,
>>  	unsigned long *offset, unsigned long *scan_base)
>>  {
>>  	struct percpu_cluster *cluster;
>> +	struct swap_cluster_info *ci;
>>  	bool found_free;
>> -	unsigned long tmp;
>> +	unsigned long tmp, max;
>> 
>>  new_cluster:
>>  	cluster = this_cpu_ptr(si->percpu_cluster);
>> @@ -476,14 +535,21 @@ new_cluster:
>>  	 * check if there is still free entry in the cluster
>>  	 */
>>  	tmp = cluster->next;
>> -	while (tmp < si->max && tmp < (cluster_next(&cluster->index) + 1) *
>> -	       SWAPFILE_CLUSTER) {
>
> Currently tmp is checked to be less than both values.
>
>> +	max = max_t(unsigned long, si->max,
>> +		    (cluster_next(&cluster->index) + 1) * SWAPFILE_CLUSTER);
>> +	if (tmp >= max) {
>> +		cluster_set_null(&cluster->index);
>> +		goto new_cluster;
>> +	}
>> +	ci = lock_cluster(si, tmp);
>> +	while (tmp < max) {
>
> In this work tmp is checked to be less than the max value.
> Semantic change hoped?

Oops!  tmp should be checked to be more than the min value.  Will fix it
in the next version.  Thanks for pointing out this!

Best Regards,
Huang, Ying

>>  		if (!si->swap_map[tmp]) {
>>  			found_free = true;
>>  			break;
>>  		}
>>  		tmp++;
>>  	}
>> +	unlock_cluster(ci);
>>  	if (!found_free) {
>>  		cluster_set_null(&cluster->index);
>>  		goto new_cluster;
>> 
> thanks
> Hillf