Skip to contentSkip to conversation

fusion: vmware bug fix prevent inifinite retries

PATCH In review Eric Moore 3 participants 4 messages Jan 5, 2007 – Jan 6, 2007

Subsystems (2)
SCSI LSI Logic MPT fusion drivers (FC/SAS/SPI)
lore
Share & tools
lore

Eric Moore

author
#0 lore raw

PATCH 2/5 fusion: vmware bug fix prevent inifinite retries

Simialar to the previous bug fix, this patch avoids inifinite retries. This is replicated in Vmware environment when the guest OS is using the 53c1030 as a emulated storage controller. This patch will restore original driver behavior for SPI from about a year ago, though keeping same behavor for FC/SAS for which it solved a failover issue.

Signed-off-by Eric Moore <[email protected]>
drivers/message/fusion/mptscsih.c +1 -1
diff -uarpN b/drivers/message/fusion/mptscsih.c a/drivers/message/fusion/mptscsih.c
@@ -779,7 +779,7 @@ mptscsih_io_done(MPT_ADAPTER *ioc, MPT_F
 			sc->resid=0;
 		case MPI_IOCSTATUS_SCSI_RECOVERED_ERROR:	/* 0x0040 */
 		case MPI_IOCSTATUS_SUCCESS:			/* 0x0000 */
-			if (scsi_status == MPI_SCSI_STATUS_BUSY)
+			if (ioc->bus_type != SPI && scsi_status == MPI_SCSI_STATUS_BUSY)
 				sc->result = (DID_BUS_BUSY << 16) | scsi_status;
 			else
 				sc->result = (DID_OK << 16) | scsi_status;
#1 lore raw

drivers/message/fusion/mptscsih.c mptscsih_io_done(MPT_ADAPTER *ioc, MPT_F · line 785

Quoted 5 lines · Eric Moore
drivers/message/fusion/mptscsih.c +1 -1
-			if (scsi_status == MPI_SCSI_STATUS_BUSY)
+			if (ioc->bus_type != SPI && scsi_status == MPI_SCSI_STATUS_BUSY)
 				sc->result = (DID_BUS_BUSY << 16) | scsi_status;
 			else
 				sc->result = (DID_OK << 16) | scsi_status;

DID_BUS_BUSY causes an immediate retry, but it does debit the retry count, so it shouldn't cause "infinite retries" ... if it does, there's something else wrong here.

I should also point out that the MPI_SCSI_STATUS_BUSY is SAM_STAT_BUSY ... this return will cause a queue stop and a requeue, but it doesn't actually debit the retries, so it *may* cause an infinite loop if the system is permanently busy.

Finally, whatever's causing this, it should probably be treated the same for all fusion bus types ...

James

#2 lore raw
Quoted 10 lines · James Bottomley

drivers/message/fusion/mptscsih.c mptscsih_io_done(MPT_ADAPTER *ioc, MPT_F · line 785

Quoted 5 lines · Eric Moore
drivers/message/fusion/mptscsih.c +1 -1
-			if (scsi_status == MPI_SCSI_STATUS_BUSY)
+			if (ioc->bus_type != SPI && scsi_status == MPI_SCSI_STATUS_BUSY)
 				sc->result = (DID_BUS_BUSY << 16) | scsi_status;
 			else
 				sc->result = (DID_OK << 16) | scsi_status;

DID_BUS_BUSY causes an immediate retry, but it does debit the retry count, so it shouldn't cause "infinite retries" ... if it does, there's something else wrong here.

I wonder if this is the same bug I'm chasing (on ia64 machines, reproduced with both Montecito and Madison).

The symptom is a stack overflow caused by this infinite loop:

generic_unplug_device
__generic_unplug_device
scsi_request_fn [1]
blk_requeue_request
elv_requeue_request
__elv_add_request
__generic_unplug_device
scsi_request_fn [2]
blk_requeue_request
elv_requeue_request
__elv_add_request
__generic_unplug_device
scsi_request_fn [3]
scsi_dispatch_cmd
scsi_queue_insert
blk_insert_request
scsi_request_fn [4]
blk_plug_device

(stack dump courtesy of incrementing a counter each time through __generic_unplug_device and checking it in blk_plug_device() and __generic_unplug_device)

I don't see how it happens; as far as I can tell, by the time we're going to call blk_plug_device() in scsi_request_fn [4], there's no way to unplug the queue again before it gets back to scsi_request_fn [3] ... and from the point where we call scsi_dispatch_cmd(), we immediately either break or test blk_queue_plugged() and exit. There should be no way for it to call blk_requeue_request() again.

#3 lore raw
Quoted 12 lines · Matthew Wilcox
Quoted 10 lines · James Bottomley

drivers/message/fusion/mptscsih.c mptscsih_io_done(MPT_ADAPTER *ioc, MPT_F · line 785

Quoted 5 lines · Eric Moore
drivers/message/fusion/mptscsih.c +1 -1
-			if (scsi_status == MPI_SCSI_STATUS_BUSY)
+			if (ioc->bus_type != SPI && scsi_status == MPI_SCSI_STATUS_BUSY)
 				sc->result = (DID_BUS_BUSY << 16) | scsi_status;
 			else
 				sc->result = (DID_OK << 16) | scsi_status;

DID_BUS_BUSY causes an immediate retry, but it does debit the retry count, so it shouldn't cause "infinite retries" ... if it does, there's something else wrong here.

Quoted 2 lines

I wonder if this is the same bug I'm chasing (on ia64 machines, reproduced with both Montecito and Madison).

I don't think so ... the done() breaks the stack chain by queueing it up for the softirq to complete. In order to get the recursion you see, we have to go straight to requeue from dispatch_cmd.

At a rough guess I'd say what you're seeing could be a result of the requeue in scsi_dispatch_cmd caused by the device being in SDEV_BLOCK.

James