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
