On 8/17/23 04:10, Damien Le Moal wrote:
On 8/17/23 04:53, Bart Van Assche wrote:+/* + * Returns true if the commands in @done_q should be sorted in LBA order + * before being resubmitted. + */ +static bool scsi_needs_sorting(struct list_head *done_q) +{ + struct scsi_cmnd *scmd; + + list_for_each_entry(scmd, done_q, eh_entry) { + struct request *rq = scsi_cmd_to_rq(scmd); + + if (!rq->q->limits.use_zone_write_lock && + blk_rq_is_seq_zoned_write(rq)) + return true; + } + + return false; +} + +/* + * Comparison function that allows to sort SCSI commands by ULD driver. + */ +static int scsi_cmp_uld(void *priv, const struct list_head *_a, + const struct list_head *_b) +{ + struct scsi_cmnd *a = list_entry(_a, typeof(*a), eh_entry); + struct scsi_cmnd *b = list_entry(_b, typeof(*b), eh_entry); + + /* See also the comment above the list_sort() definition. */ + return scsi_cmd_to_driver(a) > scsi_cmd_to_driver(b); +} + +void scsi_call_prepare_resubmit(struct list_head *done_q) +{ + struct scsi_cmnd *scmd, *next; + + if (!scsi_needs_sorting(done_q)) + return;This is strange. The eh_prepare_resubmit callback is generic and its name does not indicate anything related to sorting by LBAs. So this check would prevent other actions not related to sorting by LBA. This should go away. In patch 6, based on the device characteristics, the sd driver should decides if it needs to set .eh_prepare_resubmit or not. And ideally, if all ULDs have eh_prepare_resubmit == NULL, this function should return here before going through the list of commands to resubmit. Given that this list should generally be small, going through it and doing nothing should be OK though...
I can add a eh_prepare_resubmit == NULL check early in scsi_call_prepare_resubmit(). Regarding the code inside scsi_needs_sorting(), how about moving that code into an additional callback function, e.g. eh_needs_prepare_resubmit? Setting .eh_prepare_resubmit depending on the zone model would prevent constification of struct scsi_driver.
Thanks, Bart.
