Re: [PATCH v2 2/2] dm: Improve zone resource limits handling

[Damien Le Moal <dlemoal@kernel.org>]

On 6/5/24 13:23, Christoph Hellwig wrote:
> On Wed, Jun 05, 2024 at 11:24:45AM +0900, Damien Le Moal wrote:
>> The generic stacking of limits implemented in the block layer cannot
>> correctly handle stacking of zone resource limits (max open zones and
>> max active zones)
> 
> ... for DM.  All other limits stacking ends up in a single top device.

I know. And I do not see your point here.

> 
>> +	/*
>> +	 * If the target does not map all sequential zones, the limits
>> +	 * will not be reliable.
>> +	 */
>> +	if (zc.target_nr_seq_zones < zc.total_nr_seq_zones)
>> +		zlim->reliable_limits = false;
>> +
>> +	/*
>> +	 * If the target maps less sequential zones than the limit values, then
>> +	 * we do not have limits for this target.
>> +	 */
>> +	max_active_zones = disk->queue->limits.max_active_zones;
>> +	if (max_active_zones >= zc.target_nr_seq_zones)
>> +		max_active_zones = 0;
>> +	zlim->max_active_zones =
>> +		min_not_zero(max_active_zones, zlim->max_active_zones);
>> +
>> +	max_open_zones = disk->queue->limits.max_open_zones;
>> +	if (max_open_zones >= zc.target_nr_seq_zones)
>> +		max_open_zones = 0;
>> +	zlim->max_open_zones =
>> +		min_not_zero(max_open_zones, zlim->max_open_zones);
> 
> Given that your previous patch already caps max_open/active_zones to the
> number of sequential zones, duplicating this here should not be needed.

Indeed. Will remove this.

> 
>> +	/* We cannot have more open zones than active zones. */
>> +	zlim->max_open_zones =
>> +			min(zlim->max_open_zones, zlim->max_active_zones);
> 
> Same question about the capping as in patch 1, and same comment about
> the duplication as above.

Yes, we can remove this one too.

> 
>> +	if (zlim.max_open_zones >= zlim.mapped_nr_seq_zones)
>> +		lim->max_open_zones = 0;
>> +	else
>> +		lim->max_open_zones = zlim.max_open_zones;
>> +
>> +	if (zlim.max_active_zones >= zlim.mapped_nr_seq_zones)
>> +		lim->max_active_zones = 0;
>> +	else
>> +		lim->max_active_zones = zlim.max_active_zones;
> 
> And once more here.

Yep.

> 
> 

-- 
Damien Le Moal
Western Digital Research