public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Finn Thain <fthain@telegraphics.com.au>
To: "James E.J. Bottomley" <JBottomley@odin.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Michael Schmitz <schmitzmic@gmail.com>,
	<linux-m68k@vger.kernel.org>, <linux-scsi@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: [PATCH 2/6] ncr5380: Dont release lock for PIO transfer
Date: Tue, 23 Feb 2016 10:07:05 +1100	[thread overview]
Message-ID: <20160222230704.504641476@telegraphics.com.au> (raw)
In-Reply-To: 20160222230703.994951661@telegraphics.com.au

[-- Attachment #1: ncr5380-transfer_pio-locking --]
[-- Type: text/plain, Size: 4358 bytes --]

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 <schmitzmic@gmail.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
 drivers/scsi/NCR5380.c       |   16 +++++++++-------
 drivers/scsi/atari_NCR5380.c |   16 +++++++++-------
 2 files changed, 18 insertions(+), 14 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/NCR5380.c	2016-02-23 10:06:56.000000000 +1100
+++ linux/drivers/scsi/NCR5380.c	2016-02-23 10:06:57.000000000 +1100
@@ -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;
Index: linux/drivers/scsi/atari_NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/atari_NCR5380.c	2016-02-23 10:06:56.000000000 +1100
+++ linux/drivers/scsi/atari_NCR5380.c	2016-02-23 10:06:57.000000000 +1100
@@ -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;

  parent reply	other threads:[~2016-02-22 23:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-22 23:07 [PATCH 0/6] ncr5380: Exception handling fixes for v4.5 Finn Thain
2016-02-22 23:07 ` [PATCH 1/6] ncr5380: Correctly clear command pointers and lists after bus reset Finn Thain
2016-02-22 23:07 ` Finn Thain [this message]
2016-02-22 23:07 ` [PATCH 3/6] ncr5380: Dont re-enter NCR5380_select() Finn Thain
2016-02-22 23:07 ` [PATCH 4/6] ncr5380: Forget aborted commands Finn Thain
2016-02-22 23:07 ` [PATCH 5/6] ncr5380: Fix NCR5380_select() EH checks and result handling Finn Thain
2016-02-22 23:07 ` [PATCH 6/6] ncr5380: Call scsi_eh_prep_cmnd() and scsi_eh_restore_cmnd() as and when appropriate Finn Thain
2016-03-01  2:31 ` [PATCH 0/6] ncr5380: Exception handling fixes for v4.5 Martin K. Petersen
2016-03-01  3:32   ` Finn Thain
2016-03-01 13:00     ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160222230704.504641476@telegraphics.com.au \
    --to=fthain@telegraphics.com.au \
    --cc=JBottomley@odin.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=schmitzmic@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox