* 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).