On 8/10/23 23:00, Bart Van Assche wrote:
> On 8/9/23 18:36, Damien Le Moal wrote:
>> On 8/10/23 05:23, Bart Van Assche wrote:
>>> +static bool dd_use_zone_write_locking(struct request_queue *q)
>>> +{
>>> + return q->limits.use_zone_write_lock && blk_queue_is_zoned(q);
>>
>> use_zone_write_lock should be true ONLY if the queue is zoned.
>> So the "&& blk_queue_is_zoned(q)" seems unnecessary to me.
>> This little helper could be moved to be generic in blkdev.h too.
>
> Hi Damien,
>
> use_zone_write_lock should be set by the block driver (e.g. a SCSI
> LLD) before I/O starts. The zone model information is retrieved by
> submitting I/O. It is not clear to me how to set use_zone_write_lock
> to true only for zoned block devices before I/O starts since I/O is
> required to retrieve information about the zone model.
See my other email. Once you know that the drive is zoned, then
use_zone_write_lock can default to true. That is trivial to do in
disk_set_zoned(), which is called by all drivers. I proposed adding an argument
to this function to override the default, thus allowing a device driver to set
it to false.
The limit default set to true as you have it in your current patch does not make
sense to me. It should be false by default and turned on only for zoned drive
that require zone write locking (e.g. SMR HDDs). With that, mq-deadline can even
be simplified to not even look at the q zoned model and simply us
q->limits.use_zone_write_lock to determine if locking a zone is needed or not.
That would be a lot cleaner.
Not that device scan and revalidation never write to the device. So that can be
done safely regardless of the current value for the use_zone_write_lock limit.
--
Damien Le Moal
Western Digital Research
