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

[Huang\, Ying]

Hillf Danton <hillf.zj@xxxxxxxxxxxxxxx> 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