* 3w-9xxxx dma unmap fix @ 2015-10-03 17:16 Christoph Hellwig 2015-10-03 17:16 ` [PATCH] 3w-9xxx: don't unmap bounce buffered commands Christoph Hellwig 0 siblings, 1 reply; 3+ messages in thread From: Christoph Hellwig @ 2015-10-03 17:16 UTC (permalink / raw) To: James Bottomley; +Cc: atoth, aradford, linux-scsi This is the properly documented and signed off version of the 3w-9xxx fix. From code inspection it seems like the two other 3ware drivers aren't affected as they never bounce buffer commands. ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] 3w-9xxx: don't unmap bounce buffered commands 2015-10-03 17:16 3w-9xxxx dma unmap fix Christoph Hellwig @ 2015-10-03 17:16 ` Christoph Hellwig 2015-10-06 0:52 ` adam radford 0 siblings, 1 reply; 3+ messages in thread From: Christoph Hellwig @ 2015-10-03 17:16 UTC (permalink / raw) To: James Bottomley; +Cc: atoth, aradford, linux-scsi 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@atoth.sote.hu> Tested-by: T??th Attila <atoth@atoth.sote.hu> Signed-off-by: Christoph Hellwig <hch@lst.de> --- 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] 3w-9xxx: don't unmap bounce buffered commands 2015-10-03 17:16 ` [PATCH] 3w-9xxx: don't unmap bounce buffered commands Christoph Hellwig @ 2015-10-06 0:52 ` adam radford 0 siblings, 0 replies; 3+ messages in thread From: adam radford @ 2015-10-06 0:52 UTC (permalink / raw) To: Christoph Hellwig; +Cc: James Bottomley, Attila Tóth, linux-scsi On Sat, Oct 3, 2015 at 10:16 AM, Christoph Hellwig <hch@lst.de> 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@atoth.sote.hu> > Tested-by: T??th Attila <atoth@atoth.sote.hu> > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > 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@gmail.com> -Adam ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-10-06 0:52 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-03 17:16 3w-9xxxx dma unmap fix Christoph Hellwig 2015-10-03 17:16 ` [PATCH] 3w-9xxx: don't unmap bounce buffered commands Christoph Hellwig 2015-10-06 0:52 ` adam radford
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).