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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/6/24 4:47 AM, Benjamin Marzinski wrote:
> On Wed, Jun 05, 2024 at 04:51:43PM +0900, Damien Le Moal wrote:
>> +static int dm_device_count_zones(struct dm_dev *dev,
>> +				 struct dm_device_zone_count *zc)
>> +{
>> +	int ret;
>> +
>> +	ret = blkdev_report_zones(dev->bdev, 0, UINT_MAX,
>> +				  dm_device_count_zones_cb, zc);
> 
> Other than the nitpick that BLK_ALL_ZONES looks better than UINT_MAX
> here, looks good.

Thanks, will make this change.

However, I realized that we have another serious issue with this, so more
changes are needed. The issue is that we have the call chain:

dm_table_set_restrictions(t, q, lim) -> dm_set_zones_restrictions(t, q, lim) ->
dm_set_zone_resource_limits(md, t, lim) which  is fine as all these functions
operate looking at the same limits. But then after calling
dm_set_zone_resource_limits() which may modify the max open/max active limits,
dm_set_zones_restrictions() calls dm_revalidate_zones(md, t), which then calls
blk_revalidate_disk_zones(). This last function looks at the max open/max
active limits of the disk queue limits to setup zone write plugging (if needed
in the case of DM). But the disk queue limits are *different* from the lim
pointer passed from dm_table_set_restrictions() as these have not been applied
yet. So we have blk_revalidate_disk_zones() looking at the "old", not yet
corrected zone resource limits.

I have 22 different test cases for testing this and none of them can detect a
problem with this. But it is still wrong and needs to be fixed.

Christoph,

Unless you have a better idea, I think we need to pass a queue_limits struct
pointer to blk_revalidate_disk_zones() -> disk_update_zone_resources(). This
pointer can be NULL, in which case disk_update_zone_resources() needs to do the
limit start_update+commit. But if it is not NULL, then
disk_update_zone_resources() must use the passed limits.

-- 
Damien Le Moal
Western Digital Research





[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux