Bart, On 8/9/17 11:19, Damien Le Moal wrote: > Bart, > > On 8/9/17 00:07, Bart Van Assche wrote: >> On Tue, 2017-08-08 at 13:17 +0900, Damien Le Moal wrote: >>> Releasing the write lock of a zone when the write commnand that >> ^^^^^^^^ >> command? >>> 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. This is good, but a little suboptimal as request dispatch gets stalled every time there are multiple write commands to the same zone queued. This could be optimized to let other command go through and so operate the drive at higher queue depth and reduce latency for operations like reads. The patch "block: Zoned block device single-threaded submission" that I sent previously and that Christoph Nack-ed is thus in fact not necessary at all. >>> Since sd_uninit_cmnd() is always called when a request is requeued, >>> call sd_zbc_write_unlock_zone() from that function for write requests >>> that acquired a zone lock. Acquisition of a zone lock by a write command >>> is indicated using the new command flag SCMD_ZONE_WRITE_LOCK. >> >> Hello Damien, >> >> Since this patch differs slightly from the version that was posted a few >> days ago, should a "v2" label have been added to the subject and should a >> changelog have been included? Anyway: > > Yes, forgot to do that. Reposting. > >> Reviewed-by: Bart Van Assche <bart.vanassche@xxxxxxx> > > Thanks. > -- Damien Le Moal, Western Digital
