[PATCH 0/3] Small fixes and cleanup

[PATCH 0/3] Small fixes and cleanup

[Finn Thain]

Miscellaneous small patches for an interrupt latency issue,
a compiler warning and a documentation cleanup.

---
 Documentation/kernel-parameters.txt    |    2 --
 Documentation/scsi/scsi-parameters.txt |    2 --
 drivers/scsi/NCR5380.c                 |   13 +++----------
 drivers/scsi/NCR5380.h                 |    8 +++++++-
 4 files changed, 10 insertions(+), 15 deletions(-)

[PATCH 0/3] Small fixes and cleanup

[Finn Thain]

Miscellaneous small patches for an interrupt latency issue,
a compiler warning and a documentation cleanup.

---
 Documentation/kernel-parameters.txt    |    2 --
 Documentation/scsi/scsi-parameters.txt |    2 --
 drivers/scsi/NCR5380.c                 |   13 +++----------
 drivers/scsi/NCR5380.h                 |    8 +++++++-
 4 files changed, 10 insertions(+), 15 deletions(-)

[PATCH 1/3] documentation/scsi: Remove nodisconnect parameter

[Finn Thain]

[-- Attachment #1: cleanup-nodisconnect --]
[-- Type: text/plain, Size: 1596 bytes --]

The driver that used the 'nodisconnect' parameter was removed in
commit 565bae6a4a8f ("[SCSI] 53c7xx: kill driver"). Related documentation
was cleaned up in commit f37a7238d379 ("[SCSI] 53c7xx: fix removal
fallout"), except for the remaining two mentions that are removed here.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
 Documentation/kernel-parameters.txt    |    2 --
 Documentation/scsi/scsi-parameters.txt |    2 --
 2 files changed, 4 deletions(-)

Index: linux/Documentation/kernel-parameters.txt
===================================================================
--- linux.orig/Documentation/kernel-parameters.txt	2016-08-27 12:29:57.000000000 +1000
+++ linux/Documentation/kernel-parameters.txt	2016-08-27 12:29:57.000000000 +1000
@@ -2543,8 +2543,6 @@ bytes respectively. Such letter suffixes
 
 	nodelayacct	[KNL] Disable per-task delay accounting
 
-	nodisconnect	[HW,SCSI,M68K] Disables SCSI disconnects.
-
 	nodsp		[SH] Disable hardware DSP at boot time.
 
 	noefi		Disable EFI runtime services support.
Index: linux/Documentation/scsi/scsi-parameters.txt
===================================================================
--- linux.orig/Documentation/scsi/scsi-parameters.txt	2016-08-27 12:29:57.000000000 +1000
+++ linux/Documentation/scsi/scsi-parameters.txt	2016-08-27 12:29:57.000000000 +1000
@@ -79,8 +79,6 @@ parameters may be changed at runtime by
 
 	ncr53c8xx=	[HW,SCSI]
 
-	nodisconnect	[HW,SCSI,M68K] Disables SCSI disconnects.
-
 	osst=		[HW,SCSI] SCSI Tape Driver
 			Format: <buffer_size>,<write_threshold>
 			See also Documentation/scsi/st.txt.

[PATCH 1/3] documentation/scsi: Remove nodisconnect parameter

[Finn Thain]

[-- Attachment #1: cleanup-nodisconnect --]
[-- Type: text/plain, Size: 1596 bytes --]

The driver that used the 'nodisconnect' parameter was removed in
commit 565bae6a4a8f ("[SCSI] 53c7xx: kill driver"). Related documentation
was cleaned up in commit f37a7238d379 ("[SCSI] 53c7xx: fix removal
fallout"), except for the remaining two mentions that are removed here.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
 Documentation/kernel-parameters.txt    |    2 --
 Documentation/scsi/scsi-parameters.txt |    2 --
 2 files changed, 4 deletions(-)

Index: linux/Documentation/kernel-parameters.txt
===================================================================
--- linux.orig/Documentation/kernel-parameters.txt	2016-08-27 12:29:57.000000000 +1000
+++ linux/Documentation/kernel-parameters.txt	2016-08-27 12:29:57.000000000 +1000
@@ -2543,8 +2543,6 @@ bytes respectively. Such letter suffixes
 
 	nodelayacct	[KNL] Disable per-task delay accounting
 
-	nodisconnect	[HW,SCSI,M68K] Disables SCSI disconnects.
-
 	nodsp		[SH] Disable hardware DSP at boot time.
 
 	noefi		Disable EFI runtime services support.
Index: linux/Documentation/scsi/scsi-parameters.txt
===================================================================
--- linux.orig/Documentation/scsi/scsi-parameters.txt	2016-08-27 12:29:57.000000000 +1000
+++ linux/Documentation/scsi/scsi-parameters.txt	2016-08-27 12:29:57.000000000 +1000
@@ -79,8 +79,6 @@ parameters may be changed at runtime by
 
 	ncr53c8xx=	[HW,SCSI]
 
-	nodisconnect	[HW,SCSI,M68K] Disables SCSI disconnects.
-
 	osst=		[HW,SCSI] SCSI Tape Driver
 			Format: <buffer_size>,<write_threshold>
 			See also Documentation/scsi/st.txt.

Re: [PATCH 1/3] documentation/scsi: Remove nodisconnect parameter

[Geert Uytterhoeven]

On Sat, Aug 27, 2016 at 4:29 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> The driver that used the 'nodisconnect' parameter was removed in
> commit 565bae6a4a8f ("[SCSI] 53c7xx: kill driver"). Related documentation
> was cleaned up in commit f37a7238d379 ("[SCSI] 53c7xx: fix removal
> fallout"), except for the remaining two mentions that are removed here.
>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/3] documentation/scsi: Remove nodisconnect parameter

[Jonathan Corbet]

On Sat, 27 Aug 2016 12:29:59 +1000
Finn Thain <fthain@telegraphics.com.au> wrote:

> The driver that used the 'nodisconnect' parameter was removed in
> commit 565bae6a4a8f ("[SCSI] 53c7xx: kill driver"). Related documentation
> was cleaned up in commit f37a7238d379 ("[SCSI] 53c7xx: fix removal
> fallout"), except for the remaining two mentions that are removed here.

Applied to the docs tree, thanks.

jon

Re: [PATCH 1/3] documentation/scsi: Remove nodisconnect parameter

[Jonathan Corbet]

On Sat, 27 Aug 2016 12:29:59 +1000
Finn Thain <fthain@telegraphics.com.au> wrote:

> The driver that used the 'nodisconnect' parameter was removed in
> commit 565bae6a4a8f ("[SCSI] 53c7xx: kill driver"). Related documentation
> was cleaned up in commit f37a7238d379 ("[SCSI] 53c7xx: fix removal
> fallout"), except for the remaining two mentions that are removed here.

Applied to the docs tree, thanks.

jon

[PATCH 2/3] scsi/ncr5380: Avoid a compiler warning

[Finn Thain]

[-- Attachment #1: ncr5380-warning-declared-inline-after-called --]
[-- Type: text/plain, Size: 2439 bytes --]

With commit 3a0f64bfa907 ("mac_scsi: Fix pseudo DMA implementation")
some versions of gcc now warn:

In file included from drivers/scsi/mac_scsi.c:335:
drivers/scsi/NCR5380.h:295: warning: `NCR5380_poll_politely' declared inline after being called
drivers/scsi/NCR5380.h:295: warning: previous declaration of `NCR5380_poll_politely' was here

Avoid this by defining NCR5380_poll_politely() in NCR5380.h.

Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
 drivers/scsi/NCR5380.c |    7 -------
 drivers/scsi/NCR5380.h |    8 +++++++-
 2 files changed, 7 insertions(+), 8 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/NCR5380.c	2016-08-27 12:29:57.000000000 +1000
+++ linux/drivers/scsi/NCR5380.c	2016-08-27 12:29:57.000000000 +1000
@@ -230,13 +230,6 @@ static int NCR5380_poll_politely2(struct
 	return -ETIMEDOUT;
 }
 
-static inline int NCR5380_poll_politely(struct Scsi_Host *instance,
-                                        int reg, int bit, int val, int wait)
-{
-	return NCR5380_poll_politely2(instance, reg, bit, val,
-	                                        reg, bit, val, wait);
-}
-
 #if NDEBUG
 static struct {
 	unsigned char mask;
Index: linux/drivers/scsi/NCR5380.h
===================================================================
--- linux.orig/drivers/scsi/NCR5380.h	2016-08-27 12:29:57.000000000 +1000
+++ linux/drivers/scsi/NCR5380.h	2016-08-27 12:29:57.000000000 +1000
@@ -292,8 +292,14 @@ static void NCR5380_reselect(struct Scsi
 static struct scsi_cmnd *NCR5380_select(struct Scsi_Host *, struct scsi_cmnd *);
 static int NCR5380_transfer_dma(struct Scsi_Host *instance, unsigned char *phase, int *count, unsigned char **data);
 static int NCR5380_transfer_pio(struct Scsi_Host *instance, unsigned char *phase, int *count, unsigned char **data);
-static int NCR5380_poll_politely(struct Scsi_Host *, int, int, int, int);
 static int NCR5380_poll_politely2(struct Scsi_Host *, int, int, int, int, int, int, int);
 
+static inline int NCR5380_poll_politely(struct Scsi_Host *instance,
+                                        int reg, int bit, int val, int wait)
+{
+	return NCR5380_poll_politely2(instance, reg, bit, val,
+	                                        reg, bit, val, wait);
+}
+
 #endif				/* __KERNEL__ */
 #endif				/* NCR5380_H */

[PATCH 2/3] scsi/ncr5380: Avoid a compiler warning

[Finn Thain]

[-- Attachment #1: ncr5380-warning-declared-inline-after-called --]
[-- Type: text/plain, Size: 2439 bytes --]

With commit 3a0f64bfa907 ("mac_scsi: Fix pseudo DMA implementation")
some versions of gcc now warn:

In file included from drivers/scsi/mac_scsi.c:335:
drivers/scsi/NCR5380.h:295: warning: `NCR5380_poll_politely' declared inline after being called
drivers/scsi/NCR5380.h:295: warning: previous declaration of `NCR5380_poll_politely' was here

Avoid this by defining NCR5380_poll_politely() in NCR5380.h.

Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
 drivers/scsi/NCR5380.c |    7 -------
 drivers/scsi/NCR5380.h |    8 +++++++-
 2 files changed, 7 insertions(+), 8 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/NCR5380.c	2016-08-27 12:29:57.000000000 +1000
+++ linux/drivers/scsi/NCR5380.c	2016-08-27 12:29:57.000000000 +1000
@@ -230,13 +230,6 @@ static int NCR5380_poll_politely2(struct
 	return -ETIMEDOUT;
 }
 
-static inline int NCR5380_poll_politely(struct Scsi_Host *instance,
-                                        int reg, int bit, int val, int wait)
-{
-	return NCR5380_poll_politely2(instance, reg, bit, val,
-	                                        reg, bit, val, wait);
-}
-
 #if NDEBUG
 static struct {
 	unsigned char mask;
Index: linux/drivers/scsi/NCR5380.h
===================================================================
--- linux.orig/drivers/scsi/NCR5380.h	2016-08-27 12:29:57.000000000 +1000
+++ linux/drivers/scsi/NCR5380.h	2016-08-27 12:29:57.000000000 +1000
@@ -292,8 +292,14 @@ static void NCR5380_reselect(struct Scsi
 static struct scsi_cmnd *NCR5380_select(struct Scsi_Host *, struct scsi_cmnd *);
 static int NCR5380_transfer_dma(struct Scsi_Host *instance, unsigned char *phase, int *count, unsigned char **data);
 static int NCR5380_transfer_pio(struct Scsi_Host *instance, unsigned char *phase, int *count, unsigned char **data);
-static int NCR5380_poll_politely(struct Scsi_Host *, int, int, int, int);
 static int NCR5380_poll_politely2(struct Scsi_Host *, int, int, int, int, int, int, int);
 
+static inline int NCR5380_poll_politely(struct Scsi_Host *instance,
+                                        int reg, int bit, int val, int wait)
+{
+	return NCR5380_poll_politely2(instance, reg, bit, val,
+	                                        reg, bit, val, wait);
+}
+
 #endif				/* __KERNEL__ */
 #endif				/* NCR5380_H */

Re: [PATCH 2/3] scsi/ncr5380: Avoid a compiler warning

[Geert Uytterhoeven]

On Sat, Aug 27, 2016 at 4:30 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> With commit 3a0f64bfa907 ("mac_scsi: Fix pseudo DMA implementation")
> some versions of gcc now warn:
>
> In file included from drivers/scsi/mac_scsi.c:335:
> drivers/scsi/NCR5380.h:295: warning: `NCR5380_poll_politely' declared inline after being called
> drivers/scsi/NCR5380.h:295: warning: previous declaration of `NCR5380_poll_politely' was here
>
> Avoid this by defining NCR5380_poll_politely() in NCR5380.h.
>
> Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers

[Finn Thain]

[-- Attachment #1: ncr5380-pio-timing --]
[-- Type: text/plain, Size: 1556 bytes --]

Large PIO transfers are broken up into chunks to try to avoid disabling
local IRQs for long periods. But IRQs are still disabled for too long
and this causes SCC FIFO overruns during serial port transfers. This
patch fixes the problem by halving the PIO chunk size.

Testing with mac_scsi shows that the extra NCR5380_main() loop iterations
have negligible performance impact on SCSI transfers (about 1% slower).
On a faster system (using the dmx3191d module) transfers showed no
measurable change.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
 drivers/scsi/NCR5380.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/NCR5380.c	2016-08-27 12:29:57.000000000 +1000
+++ linux/drivers/scsi/NCR5380.c	2016-08-27 12:29:58.000000000 +1000
@@ -1847,11 +1847,11 @@ static void NCR5380_information_transfer
 						/* XXX - need to source or sink data here, as appropriate */
 					}
 				} else {
-					/* Break up transfer into 3 ms chunks,
-					 * presuming 6 accesses per handshake.
+					/* Transfer a small chunk so that the
+					 * irq mode lock is not held too long.
 					 */
 					transfersize = min((unsigned long)cmd->SCp.this_residual,
-					                   hostdata->accesses_per_ms / 2);
+					                   hostdata->accesses_per_ms >> 2);
 					len = transfersize;
 					NCR5380_transfer_pio(instance, &phase, &len,
 					                     (unsigned char **)&cmd->SCp.ptr);

[PATCH 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers

[Finn Thain]

[-- Attachment #1: ncr5380-pio-timing --]
[-- Type: text/plain, Size: 1556 bytes --]

Large PIO transfers are broken up into chunks to try to avoid disabling
local IRQs for long periods. But IRQs are still disabled for too long
and this causes SCC FIFO overruns during serial port transfers. This
patch fixes the problem by halving the PIO chunk size.

Testing with mac_scsi shows that the extra NCR5380_main() loop iterations
have negligible performance impact on SCSI transfers (about 1% slower).
On a faster system (using the dmx3191d module) transfers showed no
measurable change.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
 drivers/scsi/NCR5380.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/NCR5380.c	2016-08-27 12:29:57.000000000 +1000
+++ linux/drivers/scsi/NCR5380.c	2016-08-27 12:29:58.000000000 +1000
@@ -1847,11 +1847,11 @@ static void NCR5380_information_transfer
 						/* XXX - need to source or sink data here, as appropriate */
 					}
 				} else {
-					/* Break up transfer into 3 ms chunks,
-					 * presuming 6 accesses per handshake.
+					/* Transfer a small chunk so that the
+					 * irq mode lock is not held too long.
 					 */
 					transfersize = min((unsigned long)cmd->SCp.this_residual,
-					                   hostdata->accesses_per_ms / 2);
+					                   hostdata->accesses_per_ms >> 2);
 					len = transfersize;
 					NCR5380_transfer_pio(instance, &phase, &len,
 					                     (unsigned char **)&cmd->SCp.ptr);

Re: [PATCH 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers

[Geert Uytterhoeven]

Hi Finn,

On Sat, Aug 27, 2016 at 4:30 AM, Finn Thain <fthain@telegraphics.com.au> wrote:
> Large PIO transfers are broken up into chunks to try to avoid disabling
> local IRQs for long periods. But IRQs are still disabled for too long
> and this causes SCC FIFO overruns during serial port transfers. This
> patch fixes the problem by halving the PIO chunk size.
>
> Testing with mac_scsi shows that the extra NCR5380_main() loop iterations
> have negligible performance impact on SCSI transfers (about 1% slower).
> On a faster system (using the dmx3191d module) transfers showed no
> measurable change.
>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
>
> ---
>  drivers/scsi/NCR5380.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> Index: linux/drivers/scsi/NCR5380.c
> ===================================================================
> --- linux.orig/drivers/scsi/NCR5380.c   2016-08-27 12:29:57.000000000 +1000
> +++ linux/drivers/scsi/NCR5380.c        2016-08-27 12:29:58.000000000 +1000
> @@ -1847,11 +1847,11 @@ static void NCR5380_information_transfer
>                                                 /* XXX - need to source or sink data here, as appropriate */
>                                         }
>                                 } else {
> -                                       /* Break up transfer into 3 ms chunks,
> -                                        * presuming 6 accesses per handshake.
> +                                       /* Transfer a small chunk so that the
> +                                        * irq mode lock is not held too long.
>                                          */
>                                         transfersize = min((unsigned long)cmd->SCp.this_residual,
> -                                                          hostdata->accesses_per_ms / 2);
> +                                                          hostdata->accesses_per_ms >> 2);

I think it's easier to read if you use "/ 4".

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers

[Finn Thain]

On Sun, 28 Aug 2016, Geert Uytterhoeven wrote:

> Hi Finn,
> 
> On Sat, Aug 27, 2016 at 4:30 AM, Finn Thain <fthain@telegraphics.com.au> 
> wrote:
> > Large PIO transfers are broken up into chunks to try to avoid 
> > disabling local IRQs for long periods. But IRQs are still disabled for 
> > too long and this causes SCC FIFO overruns during serial port 
> > transfers. This patch fixes the problem by halving the PIO chunk size.
> >
> > Testing with mac_scsi shows that the extra NCR5380_main() loop 
> > iterations have negligible performance impact on SCSI transfers (about 
> > 1% slower). On a faster system (using the dmx3191d module) transfers 
> > showed no measurable change.
> >
> > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> >
> > ---
> >  drivers/scsi/NCR5380.c |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > Index: linux/drivers/scsi/NCR5380.c
> > ===================================================================
> > --- linux.orig/drivers/scsi/NCR5380.c   2016-08-27 12:29:57.000000000 +1000
> > +++ linux/drivers/scsi/NCR5380.c        2016-08-27 12:29:58.000000000 +1000
> > @@ -1847,11 +1847,11 @@ static void NCR5380_information_transfer
> >                                                 /* XXX - need to source or sink data here, as appropriate */
> >                                         }
> >                                 } else {
> > -                                       /* Break up transfer into 3 ms chunks,
> > -                                        * presuming 6 accesses per handshake.
> > +                                       /* Transfer a small chunk so that the
> > +                                        * irq mode lock is not held too long.
> >                                          */
> >                                         transfersize = min((unsigned long)cmd->SCp.this_residual,
> > -                                                          hostdata->accesses_per_ms / 2);
> > +                                                          hostdata->accesses_per_ms >> 2);
> 
> I think it's easier to read if you use "/ 4".

I think the factor, "1/4 byte milliseconds per access" is not very 
meaningful. The PIO transfersize can be understood as,

        pio_bytes_until_scc_fifo_overflow = accesses_per_ms /
                         (accesses_per_pio_byte / ms_until_fifo_overflow)

This loop seemed like a good place to avoid a DIV instruction (though I 
didn't try to confirm that) and so I used a bit shift to indicate that 
intention.

The shift amount was an empirical result that happened to work for the 
hardware I tested it on, at the baud rate I was using. Admittedly, if we 
want to avoid further tweaks to this then I'll have to do more testing and 
find a better approximation.

-- 

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

[PATCH v2 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers

[Finn Thain]

[-- Attachment #1: ncr5380-pio-timing --]
[-- Type: text/plain, Size: 2608 bytes --]

Large PIO transfers are broken up into chunks to try to avoid disabling
local IRQs for long periods. But IRQs are still disabled for too long
and this causes SCC FIFO overruns during serial port transfers.

This patch reduces the PIO chunk size to reduce interrupt latency to
something on the order of milliseconds, at the expense of additional CPU
overhead from extra iterations of the NCR5380_main() loop.

That CPU overhead is a problem for slow machines (e.g. mac_scsi on 25 MHz
68030) but these machines generally use PDMA not PIO. This patch doesn't
make the overhead any worse on my Mac LC III (because it only gets about
510 accesses per ms).

This patch decreases disk performance by a fraction of one percent for
dmx3191d on my 333 MHz PowerPC 750. Other affected hardware (such as
g_NCR5380 on x86) was not tested but 5380 ISA cards generally use PDMA
and not PIO.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
Changed since v1:
- PIO transfer chunk size is now hard-coded for simplicity.

---
 drivers/scsi/NCR5380.c |    8 ++++----
 drivers/scsi/NCR5380.h |    2 ++
 2 files changed, 6 insertions(+), 4 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/NCR5380.c	2016-08-31 14:44:51.000000000 +1000
+++ linux/drivers/scsi/NCR5380.c	2016-08-31 14:44:52.000000000 +1000
@@ -1849,11 +1849,11 @@ static void NCR5380_information_transfer
 						/* XXX - need to source or sink data here, as appropriate */
 					}
 				} else {
-					/* Break up transfer into 3 ms chunks,
-					 * presuming 6 accesses per handshake.
+					/* Transfer a small chunk so that the
+					 * irq mode lock is not held too long.
 					 */
-					transfersize = min((unsigned long)cmd->SCp.this_residual,
-					                   hostdata->accesses_per_ms / 2);
+					transfersize = min(cmd->SCp.this_residual,
+					                   NCR5380_PIO_CHUNK_SIZE);
 					len = transfersize;
 					NCR5380_transfer_pio(instance, &phase, &len,
 					                     (unsigned char **)&cmd->SCp.ptr);
Index: linux/drivers/scsi/NCR5380.h
===================================================================
--- linux.orig/drivers/scsi/NCR5380.h	2016-08-31 14:44:51.000000000 +1000
+++ linux/drivers/scsi/NCR5380.h	2016-08-31 14:44:52.000000000 +1000
@@ -250,6 +250,8 @@ struct NCR5380_cmd {
 
 #define NCR5380_CMD_SIZE		(sizeof(struct NCR5380_cmd))
 
+#define NCR5380_PIO_CHUNK_SIZE		256
+
 static inline struct scsi_cmnd *NCR5380_to_scmd(struct NCR5380_cmd *ncmd_ptr)
 {
 	return ((struct scsi_cmnd *)ncmd_ptr) - 1;

[PATCH v2 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers

[Finn Thain]

[-- Attachment #1: ncr5380-pio-timing --]
[-- Type: text/plain, Size: 2608 bytes --]

Large PIO transfers are broken up into chunks to try to avoid disabling
local IRQs for long periods. But IRQs are still disabled for too long
and this causes SCC FIFO overruns during serial port transfers.

This patch reduces the PIO chunk size to reduce interrupt latency to
something on the order of milliseconds, at the expense of additional CPU
overhead from extra iterations of the NCR5380_main() loop.

That CPU overhead is a problem for slow machines (e.g. mac_scsi on 25 MHz
68030) but these machines generally use PDMA not PIO. This patch doesn't
make the overhead any worse on my Mac LC III (because it only gets about
510 accesses per ms).

This patch decreases disk performance by a fraction of one percent for
dmx3191d on my 333 MHz PowerPC 750. Other affected hardware (such as
g_NCR5380 on x86) was not tested but 5380 ISA cards generally use PDMA
and not PIO.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
Changed since v1:
- PIO transfer chunk size is now hard-coded for simplicity.

---
 drivers/scsi/NCR5380.c |    8 ++++----
 drivers/scsi/NCR5380.h |    2 ++
 2 files changed, 6 insertions(+), 4 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/NCR5380.c	2016-08-31 14:44:51.000000000 +1000
+++ linux/drivers/scsi/NCR5380.c	2016-08-31 14:44:52.000000000 +1000
@@ -1849,11 +1849,11 @@ static void NCR5380_information_transfer
 						/* XXX - need to source or sink data here, as appropriate */
 					}
 				} else {
-					/* Break up transfer into 3 ms chunks,
-					 * presuming 6 accesses per handshake.
+					/* Transfer a small chunk so that the
+					 * irq mode lock is not held too long.
 					 */
-					transfersize = min((unsigned long)cmd->SCp.this_residual,
-					                   hostdata->accesses_per_ms / 2);
+					transfersize = min(cmd->SCp.this_residual,
+					                   NCR5380_PIO_CHUNK_SIZE);
 					len = transfersize;
 					NCR5380_transfer_pio(instance, &phase, &len,
 					                     (unsigned char **)&cmd->SCp.ptr);
Index: linux/drivers/scsi/NCR5380.h
===================================================================
--- linux.orig/drivers/scsi/NCR5380.h	2016-08-31 14:44:51.000000000 +1000
+++ linux/drivers/scsi/NCR5380.h	2016-08-31 14:44:52.000000000 +1000
@@ -250,6 +250,8 @@ struct NCR5380_cmd {
 
 #define NCR5380_CMD_SIZE		(sizeof(struct NCR5380_cmd))
 
+#define NCR5380_PIO_CHUNK_SIZE		256
+
 static inline struct scsi_cmnd *NCR5380_to_scmd(struct NCR5380_cmd *ncmd_ptr)
 {
 	return ((struct scsi_cmnd *)ncmd_ptr) - 1;

Re: [PATCH v2 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers

[Laurence Oberman]

----- Original Message -----
> From: "Finn Thain" <fthain@telegraphics.com.au>
> To: "James E.J. Bottomley" <jejb@linux.vnet.ibm.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, "Geert
> Uytterhoeven" <geert@linux-m68k.org>, linux-scsi@vger.kernel.org, linux-m68k@vger.kernel.org,
> linux-kernel@vger.kernel.org, "Michael Schmitz" <schmitzmic@gmail.com>
> Sent: Wednesday, August 31, 2016 12:44:56 AM
> Subject: [PATCH v2 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers
> 
> Large PIO transfers are broken up into chunks to try to avoid disabling
> local IRQs for long periods. But IRQs are still disabled for too long
> and this causes SCC FIFO overruns during serial port transfers.
> 
> This patch reduces the PIO chunk size to reduce interrupt latency to
> something on the order of milliseconds, at the expense of additional CPU
> overhead from extra iterations of the NCR5380_main() loop.
> 
> That CPU overhead is a problem for slow machines (e.g. mac_scsi on 25 MHz
> 68030) but these machines generally use PDMA not PIO. This patch doesn't
> make the overhead any worse on my Mac LC III (because it only gets about
> 510 accesses per ms).
> 
> This patch decreases disk performance by a fraction of one percent for
> dmx3191d on my 333 MHz PowerPC 750. Other affected hardware (such as
> g_NCR5380 on x86) was not tested but 5380 ISA cards generally use PDMA
> and not PIO.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> 
> ---
> Changed since v1:
> - PIO transfer chunk size is now hard-coded for simplicity.
> 
> ---
>  drivers/scsi/NCR5380.c |    8 ++++----
>  drivers/scsi/NCR5380.h |    2 ++
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> Index: linux/drivers/scsi/NCR5380.c
> ===================================================================
> --- linux.orig/drivers/scsi/NCR5380.c	2016-08-31 14:44:51.000000000 +1000
> +++ linux/drivers/scsi/NCR5380.c	2016-08-31 14:44:52.000000000 +1000
> @@ -1849,11 +1849,11 @@ static void NCR5380_information_transfer
>  						/* XXX - need to source or sink data here, as appropriate */
>  					}
>  				} else {
> -					/* Break up transfer into 3 ms chunks,
> -					 * presuming 6 accesses per handshake.
> +					/* Transfer a small chunk so that the
> +					 * irq mode lock is not held too long.
>  					 */
> -					transfersize = min((unsigned long)cmd->SCp.this_residual,
> -					                   hostdata->accesses_per_ms / 2);
> +					transfersize = min(cmd->SCp.this_residual,
> +					                   NCR5380_PIO_CHUNK_SIZE);
>  					len = transfersize;
>  					NCR5380_transfer_pio(instance, &phase, &len,
>  					                     (unsigned char **)&cmd->SCp.ptr);
> Index: linux/drivers/scsi/NCR5380.h
> ===================================================================
> --- linux.orig/drivers/scsi/NCR5380.h	2016-08-31 14:44:51.000000000 +1000
> +++ linux/drivers/scsi/NCR5380.h	2016-08-31 14:44:52.000000000 +1000
> @@ -250,6 +250,8 @@ struct NCR5380_cmd {
>  
>  #define NCR5380_CMD_SIZE		(sizeof(struct NCR5380_cmd))
>  
> +#define NCR5380_PIO_CHUNK_SIZE		256
> +
>  static inline struct scsi_cmnd *NCR5380_to_scmd(struct NCR5380_cmd
>  *ncmd_ptr)
>  {
>  	return ((struct scsi_cmnd *)ncmd_ptr) - 1;
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Patch makes sense, look good to me.

Reviwed-by: Laurence Oberman <loberman@redhat.com>

Re: [PATCH v2 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers

[Martin K. Petersen]

>>>>> "Finn" == Finn Thain <fthain@telegraphics.com.au> writes:

Finn> Large PIO transfers are broken up into chunks to try to avoid
Finn> disabling local IRQs for long periods. But IRQs are still disabled
Finn> for too long and this causes SCC FIFO overruns during serial port
Finn> transfers.

Finn> This patch reduces the PIO chunk size to reduce interrupt latency
Finn> to something on the order of milliseconds, at the expense of
Finn> additional CPU overhead from extra iterations of the
Finn> NCR5380_main() loop.

Finn> That CPU overhead is a problem for slow machines (e.g. mac_scsi on
Finn> 25 MHz
Finn> 68030) but these machines generally use PDMA not PIO. This patch
Finn>        doesn't
Finn> make the overhead any worse on my Mac LC III (because it only gets
Finn> about 510 accesses per ms).

Finn> This patch decreases disk performance by a fraction of one percent
Finn> for dmx3191d on my 333 MHz PowerPC 750. Other affected hardware
Finn> (such as g_NCR5380 on x86) was not tested but 5380 ISA cards
Finn> generally use PDMA and not PIO.

Geert?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers

[Martin K. Petersen]

>>>>> "Finn" == Finn Thain <fthain@telegraphics.com.au> writes:

Finn> Large PIO transfers are broken up into chunks to try to avoid
Finn> disabling local IRQs for long periods. But IRQs are still disabled
Finn> for too long and this causes SCC FIFO overruns during serial port
Finn> transfers.

Finn> This patch reduces the PIO chunk size to reduce interrupt latency
Finn> to something on the order of milliseconds, at the expense of
Finn> additional CPU overhead from extra iterations of the
Finn> NCR5380_main() loop.

Finn> That CPU overhead is a problem for slow machines (e.g. mac_scsi on
Finn> 25 MHz
Finn> 68030) but these machines generally use PDMA not PIO. This patch
Finn>        doesn't
Finn> make the overhead any worse on my Mac LC III (because it only gets
Finn> about 510 accesses per ms).

Finn> This patch decreases disk performance by a fraction of one percent
Finn> for dmx3191d on my 333 MHz PowerPC 750. Other affected hardware
Finn> (such as g_NCR5380 on x86) was not tested but 5380 ISA cards
Finn> generally use PDMA and not PIO.

Geert?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers

[Geert Uytterhoeven]

On Fri, Sep 9, 2016 at 1:28 PM, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>>>>>> "Finn" == Finn Thain <fthain@telegraphics.com.au> writes:
>
> Finn> Large PIO transfers are broken up into chunks to try to avoid
> Finn> disabling local IRQs for long periods. But IRQs are still disabled
> Finn> for too long and this causes SCC FIFO overruns during serial port
> Finn> transfers.
>
> Finn> This patch reduces the PIO chunk size to reduce interrupt latency
> Finn> to something on the order of milliseconds, at the expense of
> Finn> additional CPU overhead from extra iterations of the
> Finn> NCR5380_main() loop.
>
> Finn> That CPU overhead is a problem for slow machines (e.g. mac_scsi on
> Finn> 25 MHz
> Finn> 68030) but these machines generally use PDMA not PIO. This patch
> Finn>        doesn't
> Finn> make the overhead any worse on my Mac LC III (because it only gets
> Finn> about 510 accesses per ms).
>
> Finn> This patch decreases disk performance by a fraction of one percent
> Finn> for dmx3191d on my 333 MHz PowerPC 750. Other affected hardware
> Finn> (such as g_NCR5380 on x86) was not tested but 5380 ISA cards
> Finn> generally use PDMA and not PIO.
>
> Geert?

No comments from my side.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers

[Martin K. Petersen]

>>>>> "Finn" == Finn Thain <fthain@telegraphics.com.au> writes:

Finn> Large PIO transfers are broken up into chunks to try to avoid
Finn> disabling local IRQs for long periods. But IRQs are still disabled
Finn> for too long and this causes SCC FIFO overruns during serial port
Finn> transfers.

Finn> This patch reduces the PIO chunk size to reduce interrupt latency
Finn> to something on the order of milliseconds, at the expense of
Finn> additional CPU overhead from extra iterations of the
Finn> NCR5380_main() loop.

Applied to 4.9/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 3/3] scsi/ncr5380: Improve interrupt latency during PIO tranfers

[Martin K. Petersen]

>>>>> "Finn" == Finn Thain <fthain@telegraphics.com.au> writes:

Finn> Large PIO transfers are broken up into chunks to try to avoid
Finn> disabling local IRQs for long periods. But IRQs are still disabled
Finn> for too long and this causes SCC FIFO overruns during serial port
Finn> transfers.

Finn> This patch reduces the PIO chunk size to reduce interrupt latency
Finn> to something on the order of milliseconds, at the expense of
Finn> additional CPU overhead from extra iterations of the
Finn> NCR5380_main() loop.

Applied to 4.9/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/3] Small fixes and cleanup

[Martin K. Petersen]

>>>>> "Finn" == Finn Thain <fthain@telegraphics.com.au> writes:

Finn> Miscellaneous small patches for an interrupt latency issue, a
Finn> compiler warning and a documentation cleanup.

Applied patches 1 and 2 to 4.9/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/3] Small fixes and cleanup

[Martin K. Petersen]

>>>>> "Finn" == Finn Thain <fthain@telegraphics.com.au> writes:

Finn> Miscellaneous small patches for an interrupt latency issue, a
Finn> compiler warning and a documentation cleanup.

Applied patches 1 and 2 to 4.9/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering