Re: [PATCH] 3w-9xxx: don't unmap bounce buffered commands

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Oct 3, 2015 at 10:16 AM, Christoph Hellwig <hch@xxxxxx> wrote:
> 3w controller don't dma map small single SGL entry commands but instead
> bounce buffer them.  Add a helper to identify these commands and don't
> call scsi_dma_unmap for them.
>
> Based on an earlier patch from James Bottomley.
>
> Fixes: 118c85 ("3w-9xxx: fix command completion race")
> Reported-by: T??th Attila <atoth@xxxxxxxxxxxxx>
> Tested-by: T??th Attila <atoth@xxxxxxxxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
>  drivers/scsi/3w-9xxx.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
> index add419d..a56a7b2 100644
> --- a/drivers/scsi/3w-9xxx.c
> +++ b/drivers/scsi/3w-9xxx.c
> @@ -212,6 +212,17 @@ static const struct file_operations twa_fops = {
>         .llseek         = noop_llseek,
>  };
>
> +/*
> + * The controllers use an inline buffer instead of a mapped SGL for small,
> + * single entry buffers.  Note that we treat a zero-length transfer like
> + * a mapped SGL.
> + */
> +static bool twa_command_mapped(struct scsi_cmnd *cmd)
> +{
> +       return scsi_sg_count(cmd) != 1 ||
> +               scsi_bufflen(cmd) >= TW_MIN_SGL_LENGTH;
> +}
> +
>  /* This function will complete an aen request from the isr */
>  static int twa_aen_complete(TW_Device_Extension *tw_dev, int request_id)
>  {
> @@ -1339,7 +1350,8 @@ static irqreturn_t twa_interrupt(int irq, void *dev_instance)
>                                 }
>
>                                 /* Now complete the io */
> -                               scsi_dma_unmap(cmd);
> +                               if (twa_command_mapped(cmd))
> +                                       scsi_dma_unmap(cmd);
>                                 cmd->scsi_done(cmd);
>                                 tw_dev->state[request_id] = TW_S_COMPLETED;
>                                 twa_free_request_id(tw_dev, request_id);
> @@ -1582,7 +1594,8 @@ static int twa_reset_device_extension(TW_Device_Extension *tw_dev)
>                                 struct scsi_cmnd *cmd = tw_dev->srb[i];
>
>                                 cmd->result = (DID_RESET << 16);
> -                               scsi_dma_unmap(cmd);
> +                               if (twa_command_mapped(cmd))
> +                                       scsi_dma_unmap(cmd);
>                                 cmd->scsi_done(cmd);
>                         }
>                 }
> @@ -1765,12 +1778,14 @@ static int twa_scsi_queue_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_
>         retval = twa_scsiop_execute_scsi(tw_dev, request_id, NULL, 0, NULL);
>         switch (retval) {
>         case SCSI_MLQUEUE_HOST_BUSY:
> -               scsi_dma_unmap(SCpnt);
> +               if (twa_command_mapped(SCpnt))
> +                       scsi_dma_unmap(SCpnt);
>                 twa_free_request_id(tw_dev, request_id);
>                 break;
>         case 1:
>                 SCpnt->result = (DID_ERROR << 16);
> -               scsi_dma_unmap(SCpnt);
> +               if (twa_command_mapped(SCpnt))
> +                       scsi_dma_unmap(SCpnt);
>                 done(SCpnt);
>                 tw_dev->state[request_id] = TW_S_COMPLETED;
>                 twa_free_request_id(tw_dev, request_id);
> @@ -1831,8 +1846,7 @@ static int twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id,
>                 /* Map sglist from scsi layer to cmd packet */
>
>                 if (scsi_sg_count(srb)) {
> -                       if ((scsi_sg_count(srb) == 1) &&
> -                           (scsi_bufflen(srb) < TW_MIN_SGL_LENGTH)) {
> +                       if (!twa_command_mapped(srb)) {
>                                 if (srb->sc_data_direction == DMA_TO_DEVICE ||
>                                     srb->sc_data_direction == DMA_BIDIRECTIONAL)
>                                         scsi_sg_copy_to_buffer(srb,
> @@ -1905,7 +1919,7 @@ static void twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int re
>  {
>         struct scsi_cmnd *cmd = tw_dev->srb[request_id];
>
> -       if (scsi_bufflen(cmd) < TW_MIN_SGL_LENGTH &&
> +       if (!twa_command_mapped(cmd) &&
>             (cmd->sc_data_direction == DMA_FROM_DEVICE ||
>              cmd->sc_data_direction == DMA_BIDIRECTIONAL)) {
>                 if (scsi_sg_count(cmd) == 1) {
> --
> 1.9.1
>

Christoph/James,

Thanks for fixing this.  The patch looks good to me.

Acked-by: Adam Radford <aradford@xxxxxxxxx>

-Adam
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux