[PATCH 3.4 063/146] 3w-9xxx: fix command completion race

From: lizf
Date: Tue Sep 15 2015 - 05:43:17 EST


From: Christoph Hellwig <hch@xxxxxx>

3.4.109-rc1 review patch. If anyone has any objections, please let me know.

------------------


commit 118c855b5623f3e2e6204f02623d88c09e0c34de upstream.

The 3w-9xxx driver needs to tear down the dma mappings before returning
the command to the midlayer, as there is no guarantee the sglist and
count are valid after that point. Also remove the dma mapping helpers
which have another inherent race due to the request_id index.

Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Acked-by: Adam Radford <aradford@xxxxxxxxx>
Signed-off-by: James Bottomley <JBottomley@xxxxxxxx>
Signed-off-by: Zefan Li <lizefan@xxxxxxxxxx>
---
drivers/scsi/3w-9xxx.c | 57 ++++++++++++--------------------------------------
drivers/scsi/3w-9xxx.h | 5 -----
2 files changed, 13 insertions(+), 49 deletions(-)

diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index 3868ab2..fb37df6 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -149,7 +149,6 @@ static int twa_reset_sequence(TW_Device_Extension *tw_dev, int soft_reset);
static int twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id, char *cdb, int use_sg, TW_SG_Entry *sglistarg);
static void twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int request_id);
static char *twa_string_lookup(twa_message_type *table, unsigned int aen_code);
-static void twa_unmap_scsi_data(TW_Device_Extension *tw_dev, int request_id);

/* Functions */

@@ -1352,11 +1351,11 @@ static irqreturn_t twa_interrupt(int irq, void *dev_instance)
}

/* Now complete the io */
+ scsi_dma_unmap(cmd);
+ cmd->scsi_done(cmd);
tw_dev->state[request_id] = TW_S_COMPLETED;
twa_free_request_id(tw_dev, request_id);
tw_dev->posted_request_count--;
- tw_dev->srb[request_id]->scsi_done(tw_dev->srb[request_id]);
- twa_unmap_scsi_data(tw_dev, request_id);
}

/* Check for valid status after each drain */
@@ -1414,26 +1413,6 @@ static void twa_load_sgl(TW_Device_Extension *tw_dev, TW_Command_Full *full_comm
}
} /* End twa_load_sgl() */

-/* This function will perform a pci-dma mapping for a scatter gather list */
-static int twa_map_scsi_sg_data(TW_Device_Extension *tw_dev, int request_id)
-{
- int use_sg;
- struct scsi_cmnd *cmd = tw_dev->srb[request_id];
-
- use_sg = scsi_dma_map(cmd);
- if (!use_sg)
- return 0;
- else if (use_sg < 0) {
- TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1c, "Failed to map scatter gather list");
- return 0;
- }
-
- cmd->SCp.phase = TW_PHASE_SGLIST;
- cmd->SCp.have_data_in = use_sg;
-
- return use_sg;
-} /* End twa_map_scsi_sg_data() */
-
/* This function will poll for a response interrupt of a request */
static int twa_poll_response(TW_Device_Extension *tw_dev, int request_id, int seconds)
{
@@ -1612,9 +1591,11 @@ static int twa_reset_device_extension(TW_Device_Extension *tw_dev)
(tw_dev->state[i] != TW_S_INITIAL) &&
(tw_dev->state[i] != TW_S_COMPLETED)) {
if (tw_dev->srb[i]) {
- tw_dev->srb[i]->result = (DID_RESET << 16);
- tw_dev->srb[i]->scsi_done(tw_dev->srb[i]);
- twa_unmap_scsi_data(tw_dev, i);
+ struct scsi_cmnd *cmd = tw_dev->srb[i];
+
+ cmd->result = (DID_RESET << 16);
+ scsi_dma_unmap(cmd);
+ cmd->scsi_done(cmd);
}
}
}
@@ -1793,21 +1774,18 @@ static int twa_scsi_queue_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_
/* Save the scsi command for use by the ISR */
tw_dev->srb[request_id] = SCpnt;

- /* Initialize phase to zero */
- SCpnt->SCp.phase = TW_PHASE_INITIAL;
-
retval = twa_scsiop_execute_scsi(tw_dev, request_id, NULL, 0, NULL);
switch (retval) {
case SCSI_MLQUEUE_HOST_BUSY:
+ scsi_dma_unmap(SCpnt);
twa_free_request_id(tw_dev, request_id);
- twa_unmap_scsi_data(tw_dev, request_id);
break;
case 1:
- tw_dev->state[request_id] = TW_S_COMPLETED;
- twa_free_request_id(tw_dev, request_id);
- twa_unmap_scsi_data(tw_dev, request_id);
SCpnt->result = (DID_ERROR << 16);
+ scsi_dma_unmap(SCpnt);
done(SCpnt);
+ tw_dev->state[request_id] = TW_S_COMPLETED;
+ twa_free_request_id(tw_dev, request_id);
retval = 0;
}
out:
@@ -1875,8 +1853,8 @@ static int twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id,
command_packet->sg_list[0].address = TW_CPU_TO_SGL(tw_dev->generic_buffer_phys[request_id]);
command_packet->sg_list[0].length = cpu_to_le32(TW_MIN_SGL_LENGTH);
} else {
- sg_count = twa_map_scsi_sg_data(tw_dev, request_id);
- if (sg_count == 0)
+ sg_count = scsi_dma_map(srb);
+ if (sg_count < 0)
goto out;

scsi_for_each_sg(srb, sg, sg_count, i) {
@@ -1991,15 +1969,6 @@ static char *twa_string_lookup(twa_message_type *table, unsigned int code)
return(table[index].text);
} /* End twa_string_lookup() */

-/* This function will perform a pci-dma unmap */
-static void twa_unmap_scsi_data(TW_Device_Extension *tw_dev, int request_id)
-{
- struct scsi_cmnd *cmd = tw_dev->srb[request_id];
-
- if (cmd->SCp.phase == TW_PHASE_SGLIST)
- scsi_dma_unmap(cmd);
-} /* End twa_unmap_scsi_data() */
-
/* This function gets called when a disk is coming on-line */
static int twa_slave_configure(struct scsi_device *sdev)
{
diff --git a/drivers/scsi/3w-9xxx.h b/drivers/scsi/3w-9xxx.h
index 040f721..0fdc83c 100644
--- a/drivers/scsi/3w-9xxx.h
+++ b/drivers/scsi/3w-9xxx.h
@@ -324,11 +324,6 @@ static twa_message_type twa_error_table[] = {
#define TW_CURRENT_DRIVER_BUILD 0
#define TW_CURRENT_DRIVER_BRANCH 0

-/* Phase defines */
-#define TW_PHASE_INITIAL 0
-#define TW_PHASE_SINGLE 1
-#define TW_PHASE_SGLIST 2
-
/* Misc defines */
#define TW_9550SX_DRAIN_COMPLETED 0xFFFF
#define TW_SECTOR_SIZE 512
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/