On 8/9/17 12:57, Bart Van Assche wrote:
> On Wed, 2017-08-09 at 11:47 +0900, Damien Le Moal wrote:
>> On 8/9/17 11:19, Damien Le Moal wrote:
>>> On 8/9/17 00:07, Bart Van Assche wrote:
>>>> On Tue, 2017-08-08 at 13:17 +0900, Damien Le Moal wrote:
>>>>> acquired the lock completes can cause deadlocks with scsi-mq due to
>>>>> potential queue reordering if the lock owning request is requeued and
>>>>> not executed.
>>>>
>>>> Please note that even with scsi-sq requeueing can cause request reordering.
>>>
>>> I am painfully aware of this fact... I will update the commit message to
>>> reflect this.
>>
>> Correction: checking the code again, I just realized that the prep_fn
>> queue function, which calls sd_init_cmnd() and causes a zone to be
>> write-locked if the prepared command is a write, is called within
>> blk_peek_request(). That function is called in scsi_request_fn() with
>> the queue lock held. So for the legacy scsi path, this means that for a
>> write command to a zone, the operation "lock zone and dequeue command"
>> is atomic. Thanks to the zone lock, further dequeue of write commands
>> for the same (locked) zone cannot happen and so there is no reordering
>> possible for write commands directed at the same zone. Reordering for
>> other commands is possible, but similarly to regular disks, this is not
>> a problem with ZBC/ZAC drives.
> 
> Hello Damien,
> 
> The locking approach in the upstream code indeed prevents reordering of
> write requests that apply to the same zone. However, this patch modifies
> the locking approach. Are you sure that with this patch applied no request
> reordering will occur if requests are requeued since this patch causes the
> zone lock to be released before requeuing occurs?

scsi_requeue_command() calls blk_unprep_request() and
blk_requeue_request under the queue lock. So atomically.
I think that suppresses any possibility of write requests to the same
zone to be reordered.

Best regards.

-- 
Damien Le Moal,
Western Digital

Reply via email to