On 8/17/23 04:53, Bart Van Assche wrote:
> Sort SCSI commands by LBA before the SCSI error handler resubmits
> these commands. This is necessary when resubmitting zoned writes
> (REQ_OP_WRITE) if multiple writes have been queued for a single zone.
>
> Cc: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
> Cc: Damien Le Moal <dlemoal@xxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Ming Lei <ming.lei@xxxxxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> drivers/scsi/sd.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 3c668cfb146d..8a4b0874e7fe 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -47,6 +47,7 @@
> #include <linux/blkpg.h>
> #include <linux/blk-pm.h>
> #include <linux/delay.h>
> +#include <linux/list_sort.h>
> #include <linux/major.h>
> #include <linux/mutex.h>
> #include <linux/string_helpers.h>
> @@ -117,6 +118,7 @@ static void sd_uninit_command(struct scsi_cmnd *SCpnt);
> static int sd_done(struct scsi_cmnd *);
> static void sd_eh_reset(struct scsi_cmnd *);
> static int sd_eh_action(struct scsi_cmnd *, int);
> +static void sd_prepare_resubmit(struct list_head *cmd_list);
> static void sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer);
> static void scsi_disk_release(struct device *cdev);
>
> @@ -617,6 +619,7 @@ static struct scsi_driver sd_template = {
> .done = sd_done,
> .eh_action = sd_eh_action,
> .eh_reset = sd_eh_reset,
> + .eh_prepare_resubmit = sd_prepare_resubmit,
> };
>
> /*
> @@ -2018,6 +2021,38 @@ static int sd_eh_action(struct scsi_cmnd *scmd, int eh_disp)
> return eh_disp;
> }
>
> +static int sd_cmp_sector(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);
> + struct request *rq_a = scsi_cmd_to_rq(a);
> + struct request *rq_b = scsi_cmd_to_rq(b);
> + bool use_zwl_a = rq_a->q->limits.use_zone_write_lock;
> + bool use_zwl_b = rq_b->q->limits.use_zone_write_lock;
> +
> + /*
> + * Order the commands that need zone write locking after the commands
> + * that do not need zone write locking. Order the commands that do not
> + * need zone write locking by LBA. Do not reorder the commands that
> + * need zone write locking. See also the comment above the list_sort()
> + * definition.
> + */
> + if (use_zwl_a || use_zwl_b)
> + return use_zwl_a > use_zwl_b;
> + return blk_rq_pos(rq_a) > blk_rq_pos(rq_b);
> +}
> +
> +static void sd_prepare_resubmit(struct list_head *cmd_list)
> +{
> + /*
> + * Sort pending SCSI commands in starting sector order. This is
> + * important if one of the SCSI devices associated with @shost is a
> + * zoned block device for which zone write locking is disabled.
> + */
> + list_sort(NULL, cmd_list, sd_cmp_sector);
We should not need to do this for regular devices or zoned devices using zone
write locking. So returning doing nothing would be better but the callback
lacks a pointer to the scsi_device to test that.
> +}
> +
> static unsigned int sd_completed_bytes(struct scsi_cmnd *scmd)
> {
> struct request *req = scsi_cmd_to_rq(scmd);
--
Damien Le Moal
Western Digital Research
