Skip to contentSkip to conversation

Patch "ncr5380: Dont release lock for PIO transfer" has been added to the 4.5-stable tree

PATCH [email protected] 1 participant 1 message Apr 9, 2016

Subsystems (3)
Stable branch SCSI Ncr 5380 SCSI drivers
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

ncr5380: Dont release lock for PIO transfer

to the 4.5-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
ncr5380-dont-release-lock-for-pio-transfer.patch
and it can be found in the queue-4.5 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 1678847ec93040ae8280d19c42ae0ba8a4233e6d Mon Sep 17 00:00:00 2001

From: Finn Thain <[email protected]>
Date: Tue, 23 Feb 2016 10:07:05 +1100
Subject: ncr5380: Dont release lock for PIO transfer

From: Finn Thain <[email protected]>

commit 1678847ec93040ae8280d19c42ae0ba8a4233e6d upstream.

The calls to NCR5380_transfer_pio() for DATA IN and DATA OUT phases will modify cmd->SCp.this_residual, cmd->SCp.ptr and cmd->SCp.buffer. That works as long as EH does not intervene, which became possible in atari_NCR5380.c when I changed the locking to bring it closer to NCR5380.c.

If error recovery aborts the command, the scsi_cmnd in question and its buffer will be returned to the mid-layer. So the transfer has to cease, but it can't be stopped by the initiator because the target controls the bus phase.

The problem does not arise if the lock is not released. That was fine for atari_scsi, because it implements DMA. For the other drivers, we have to release the lock and re-enable interrupts for long PIO data transfers.

The solution is to split the transfer into small chunks. In between chunks the main loop releases the lock and re-enables interrupts. Thus interrupts can be serviced and eh_bus_reset_handler can intervene if need be.

This fixes an oops in NCR5380_transfer_pio() that can happen when the EH abort handler is invoked during DATA IN or DATA OUT phase.

Fixes 11d2f63b9cf5 ("ncr5380: Change instance->host_lock to hostdata->lock")
Reported-and-tested-by Michael Schmitz <[email protected]>
Signed-off-by Finn Thain <[email protected]>
Signed-off-by Martin K. Petersen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/scsi/NCR5380.c       |   16 +++++++++-------
 drivers/scsi/atari_NCR5380.c |   16 +++++++++-------
 2 files changed, 18 insertions(+), 14 deletions(-)
drivers/scsi/NCR5380.c +9 -7
@@ -1759,9 +1759,7 @@ static void NCR5380_information_transfer
 	unsigned char msgout = NOP;
 	int sink = 0;
 	int len;
-#if defined(PSEUDO_DMA) || defined(REAL_DMA_POLL)
 	int transfersize;
-#endif
 	unsigned char *data;
 	unsigned char phase, tmp, extended_msg[10], old_phase = 0xff;
 	struct scsi_cmnd *cmd;

@@ -1854,13 +1852,17 @@ static void NCR5380_information_transfer
 				} else
 #endif				/* defined(PSEUDO_DMA) || defined(REAL_DMA_POLL) */
 				{
-					spin_unlock_irq(&hostdata->lock);
-					NCR5380_transfer_pio(instance, &phase,
-					                     (int *)&cmd->SCp.this_residual,
+					/* Break up transfer into 3 ms chunks,
+					 * presuming 6 accesses per handshake.
+					 */
+					transfersize = min((unsigned long)cmd->SCp.this_residual,
+					                   hostdata->accesses_per_ms / 2);
+					len = transfersize;
+					NCR5380_transfer_pio(instance, &phase, &len,
 					                     (unsigned char **)&cmd->SCp.ptr);
-					spin_lock_irq(&hostdata->lock);
+					cmd->SCp.this_residual -= transfersize - len;
 				}
-				break;
+				return;
 			case PHASE_MSGIN:
 				len = 1;
 				data = &tmp;
drivers/scsi/atari_NCR5380.c +9 -7
@@ -1838,9 +1838,7 @@ static void NCR5380_information_transfer
 	unsigned char msgout = NOP;
 	int sink = 0;
 	int len;
-#if defined(REAL_DMA)
 	int transfersize;
-#endif
 	unsigned char *data;
 	unsigned char phase, tmp, extended_msg[10], old_phase = 0xff;
 	struct scsi_cmnd *cmd;

@@ -1983,18 +1981,22 @@ static void NCR5380_information_transfer
 				} else
 #endif /* defined(REAL_DMA) */
 				{
-					spin_unlock_irq(&hostdata->lock);
-					NCR5380_transfer_pio(instance, &phase,
-					                     (int *)&cmd->SCp.this_residual,
+					/* Break up transfer into 3 ms chunks,
+					 * presuming 6 accesses per handshake.
+					 */
+					transfersize = min((unsigned long)cmd->SCp.this_residual,
+					                   hostdata->accesses_per_ms / 2);
+					len = transfersize;
+					NCR5380_transfer_pio(instance, &phase, &len,
 					                     (unsigned char **)&cmd->SCp.ptr);
-					spin_lock_irq(&hostdata->lock);
+					cmd->SCp.this_residual -= transfersize - len;
 				}
 #if defined(CONFIG_SUN3) && defined(REAL_DMA)
 				/* if we had intended to dma that command clear it */
 				if (sun3_dma_setup_done == cmd)
 					sun3_dma_setup_done = NULL;
 #endif
-				break;
+				return;
 			case PHASE_MSGIN:
 				len = 1;
 				data = &tmp;

Patches currently in stable-queue which might be from [email protected] are

queue-4.5/ncr5380-correctly-clear-command-pointers-and-lists-after-bus-reset.patch queue-4.5/ncr5380-call-scsi_eh_prep_cmnd-and-scsi_eh_restore_cmnd-as-and-when-appropriate.patch queue-4.5/ncr5380-forget-aborted-commands.patch queue-4.5/ncr5380-dont-release-lock-for-pio-transfer.patch queue-4.5/ncr5380-dont-re-enter-ncr5380_select.patch queue-4.5/ncr5380-fix-ncr5380_select-eh-checks-and-result-handling.patch