Re: [PATCH v3 50/77] ncr5380: Change instance->host_lock to hostdata->lock

2015-12-21 Thread Hannes Reinecke 

On 12/22/2015 02:18 AM, Finn Thain wrote:

NCR5380.c presently uses the instance->host_lock spin lock. Convert this
to a new spin lock that protects the NCR5380_hostdata struct.

atari_NCR5380.c previously used local_irq_save/restore() rather than a
spin lock. Convert this to hostdata->lock in irq mode. For SMP platforms,
the interrupt handler now also acquires the spin lock.

This brings all locking in the two core drivers into agreement.

Adding this locking also means that a bunch of volatile qualifiers can be
removed from the members of the NCR5380_hostdata struct. This is done in
a subsequent patch.

Proper locking will allow the abort handler to locate a command being
aborted. This is presently impossible if the abort handler is invoked when
the command has been moved from a queue to a pointer on the stack. (If
eh_abort_handler can't determine whether a command has been completed
or is still being processed then it can't decide whether to return
success or failure.)

The hostdata spin lock is now held when calling NCR5380_select() and
NCR5380_information_transfer(). Where possible, the lock is dropped for
polling and PIO transfers.

Clean up the now-redundant SELECT_ENABLE_REG writes, that used to provide
limited mutual exclusion between information_transfer() and reselect().

Accessing hostdata->connected without data races means taking the lock;
cleanup these accesses.

The new spin lock falls away for m68k and other UP builds, so this should
have little impact there. In the SMP case the new lock should be
uncontested even when the SCSI bus is contested.

Signed-off-by: Finn Thain 


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 50/77] ncr5380: Change instance->host_lock to hostdata->lock

2015-12-21 Thread Finn Thain 
NCR5380.c presently uses the instance->host_lock spin lock. Convert this
to a new spin lock that protects the NCR5380_hostdata struct.

atari_NCR5380.c previously used local_irq_save/restore() rather than a
spin lock. Convert this to hostdata->lock in irq mode. For SMP platforms,
the interrupt handler now also acquires the spin lock.

This brings all locking in the two core drivers into agreement.

Adding this locking also means that a bunch of volatile qualifiers can be
removed from the members of the NCR5380_hostdata struct. This is done in
a subsequent patch.

Proper locking will allow the abort handler to locate a command being
aborted. This is presently impossible if the abort handler is invoked when
the command has been moved from a queue to a pointer on the stack. (If
eh_abort_handler can't determine whether a command has been completed
or is still being processed then it can't decide whether to return
success or failure.)

The hostdata spin lock is now held when calling NCR5380_select() and
NCR5380_information_transfer(). Where possible, the lock is dropped for
polling and PIO transfers.

Clean up the now-redundant SELECT_ENABLE_REG writes, that used to provide
limited mutual exclusion between information_transfer() and reselect().

Accessing hostdata->connected without data races means taking the lock;
cleanup these accesses.

The new spin lock falls away for m68k and other UP builds, so this should
have little impact there. In the SMP case the new lock should be
uncontested even when the SCSI bus is contested.

Signed-off-by: Finn Thain 

---
 drivers/scsi/NCR5380.c   |   82 --
 drivers/scsi/NCR5380.h   |1 
 drivers/scsi/atari_NCR5380.c |  135 +--
 3 files changed, 109 insertions(+), 109 deletions(-)

Index: linux/drivers/scsi/NCR5380.h
===
--- linux.orig/drivers/scsi/NCR5380.h   2015-12-22 12:16:45.0 +1100
+++ linux/drivers/scsi/NCR5380.h2015-12-22 12:16:47.0 +1100
@@ -256,6 +256,7 @@ struct NCR5380_hostdata {
volatile struct scsi_cmnd *connected;   /* currently connected command 
*/
volatile struct scsi_cmnd *issue_queue; /* waiting to be issued */
volatile struct scsi_cmnd *disconnected_queue;  /* waiting for 
reconnect */
+   spinlock_t lock;/* protects this struct */
int flags;
struct scsi_eh_save ses;
char info[256];
Index: linux/drivers/scsi/atari_NCR5380.c
===
--- linux.orig/drivers/scsi/atari_NCR5380.c 2015-12-22 12:16:45.0 
+1100
+++ linux/drivers/scsi/atari_NCR5380.c  2015-12-22 12:16:47.0 +1100
@@ -546,15 +546,13 @@ static struct {
 static void NCR5380_print(struct Scsi_Host *instance)
 {
unsigned char status, data, basr, mr, icr, i;
-   unsigned long flags;
 
-   local_irq_save(flags);
data = NCR5380_read(CURRENT_SCSI_DATA_REG);
status = NCR5380_read(STATUS_REG);
mr = NCR5380_read(MODE_REG);
icr = NCR5380_read(INITIATOR_COMMAND_REG);
basr = NCR5380_read(BUS_AND_STATUS_REG);
-   local_irq_restore(flags);
+
printk("STATUS_REG: %02x ", status);
for (i = 0; signals[i].mask; ++i)
if (status & signals[i].mask)
@@ -684,14 +682,12 @@ static void __maybe_unused NCR5380_print
 {
struct NCR5380_hostdata *hostdata;
struct scsi_cmnd *ptr;
-   unsigned long flags;
 
NCR5380_dprint(NDEBUG_ANY, instance);
NCR5380_dprint_phase(NDEBUG_ANY, instance);
 
hostdata = (struct NCR5380_hostdata *)instance->hostdata;
 
-   local_irq_save(flags);
if (!hostdata->connected)
printk("scsi%d: no currently connected command\n", HOSTNO);
else
@@ -704,8 +700,6 @@ static void __maybe_unused NCR5380_print
for (ptr = (struct scsi_cmnd *) hostdata->disconnected_queue; ptr;
 ptr = NEXT(ptr))
lprint_Scsi_Cmnd(ptr);
-
-   local_irq_restore(flags);
printk("\n");
 }
 
@@ -732,7 +726,7 @@ static int __maybe_unused NCR5380_show_i
 
hostdata = (struct NCR5380_hostdata *)instance->hostdata;
 
-   local_irq_save(flags);
+   spin_lock_irqsave(>lock, flags);
if (!hostdata->connected)
seq_printf(m, "scsi%d: no currently connected command\n", 
HOSTNO);
else
@@ -746,7 +740,7 @@ static int __maybe_unused NCR5380_show_i
 ptr = NEXT(ptr))
show_Scsi_Cmnd(ptr, m);
 
-   local_irq_restore(flags);
+   spin_unlock_irqrestore(>lock, flags);
return 0;
 }
 
@@ -784,6 +778,7 @@ static int __init NCR5380_init(struct Sc
 #if defined (REAL_DMA)
hostdata->dma_len = 0;
 #endif
+   spin_lock_init(>lock);
hostdata->connected = NULL;
hostdata->issue_queue = NULL;
hostdata->disconnected_queue =

[PATCH v3 50/77] ncr5380: Change instance->host_lock to hostdata->lock

2015-12-21 Thread Finn Thain 
NCR5380.c presently uses the instance->host_lock spin lock. Convert this
to a new spin lock that protects the NCR5380_hostdata struct.

atari_NCR5380.c previously used local_irq_save/restore() rather than a
spin lock. Convert this to hostdata->lock in irq mode. For SMP platforms,
the interrupt handler now also acquires the spin lock.

This brings all locking in the two core drivers into agreement.

Adding this locking also means that a bunch of volatile qualifiers can be
removed from the members of the NCR5380_hostdata struct. This is done in
a subsequent patch.

Proper locking will allow the abort handler to locate a command being
aborted. This is presently impossible if the abort handler is invoked when
the command has been moved from a queue to a pointer on the stack. (If
eh_abort_handler can't determine whether a command has been completed
or is still being processed then it can't decide whether to return
success or failure.)

The hostdata spin lock is now held when calling NCR5380_select() and
NCR5380_information_transfer(). Where possible, the lock is dropped for
polling and PIO transfers.

Clean up the now-redundant SELECT_ENABLE_REG writes, that used to provide
limited mutual exclusion between information_transfer() and reselect().

Accessing hostdata->connected without data races means taking the lock;
cleanup these accesses.

The new spin lock falls away for m68k and other UP builds, so this should
have little impact there. In the SMP case the new lock should be
uncontested even when the SCSI bus is contested.

Signed-off-by: Finn Thain 

---
 drivers/scsi/NCR5380.c   |   82 --
 drivers/scsi/NCR5380.h   |1 
 drivers/scsi/atari_NCR5380.c |  135 +--
 3 files changed, 109 insertions(+), 109 deletions(-)

Index: linux/drivers/scsi/NCR5380.h
===
--- linux.orig/drivers/scsi/NCR5380.h   2015-12-22 12:16:45.0 +1100
+++ linux/drivers/scsi/NCR5380.h2015-12-22 12:16:47.0 +1100
@@ -256,6 +256,7 @@ struct NCR5380_hostdata {
volatile struct scsi_cmnd *connected;   /* currently connected command 
*/
volatile struct scsi_cmnd *issue_queue; /* waiting to be issued */
volatile struct scsi_cmnd *disconnected_queue;  /* waiting for 
reconnect */
+   spinlock_t lock;/* protects this struct */
int flags;
struct scsi_eh_save ses;
char info[256];
Index: linux/drivers/scsi/atari_NCR5380.c
===
--- linux.orig/drivers/scsi/atari_NCR5380.c 2015-12-22 12:16:45.0 
+1100
+++ linux/drivers/scsi/atari_NCR5380.c  2015-12-22 12:16:47.0 +1100
@@ -546,15 +546,13 @@ static struct {
 static void NCR5380_print(struct Scsi_Host *instance)
 {
unsigned char status, data, basr, mr, icr, i;
-   unsigned long flags;
 
-   local_irq_save(flags);
data = NCR5380_read(CURRENT_SCSI_DATA_REG);
status = NCR5380_read(STATUS_REG);
mr = NCR5380_read(MODE_REG);
icr = NCR5380_read(INITIATOR_COMMAND_REG);
basr = NCR5380_read(BUS_AND_STATUS_REG);
-   local_irq_restore(flags);
+
printk("STATUS_REG: %02x ", status);
for (i = 0; signals[i].mask; ++i)
if (status & signals[i].mask)
@@ -684,14 +682,12 @@ static void __maybe_unused NCR5380_print
 {
struct NCR5380_hostdata *hostdata;
struct scsi_cmnd *ptr;
-   unsigned long flags;
 
NCR5380_dprint(NDEBUG_ANY, instance);
NCR5380_dprint_phase(NDEBUG_ANY, instance);
 
hostdata = (struct NCR5380_hostdata *)instance->hostdata;
 
-   local_irq_save(flags);
if (!hostdata->connected)
printk("scsi%d: no currently connected command\n", HOSTNO);
else
@@ -704,8 +700,6 @@ static void __maybe_unused NCR5380_print
for (ptr = (struct scsi_cmnd *) hostdata->disconnected_queue; ptr;
 ptr = NEXT(ptr))
lprint_Scsi_Cmnd(ptr);
-
-   local_irq_restore(flags);
printk("\n");
 }
 
@@ -732,7 +726,7 @@ static int __maybe_unused NCR5380_show_i
 
hostdata = (struct NCR5380_hostdata *)instance->hostdata;
 
-   local_irq_save(flags);
+   spin_lock_irqsave(>lock, flags);
if (!hostdata->connected)
seq_printf(m, "scsi%d: no currently connected command\n", 
HOSTNO);
else
@@ -746,7 +740,7 @@ static int __maybe_unused NCR5380_show_i
 ptr = NEXT(ptr))
show_Scsi_Cmnd(ptr, m);
 
-   local_irq_restore(flags);
+   spin_unlock_irqrestore(>lock, flags);
return 0;
 }
 
@@ -784,6 +778,7 @@ static int __init NCR5380_init(struct Sc
 #if defined (REAL_DMA)
hostdata->dma_len = 0;
 #endif
+   spin_lock_init(>lock);
hostdata->connected = NULL;
hostdata->issue_queue = NULL;

Re: [PATCH v3 50/77] ncr5380: Change instance->host_lock to hostdata->lock

2015-12-21 Thread Hannes Reinecke 

On 12/22/2015 02:18 AM, Finn Thain wrote:

NCR5380.c presently uses the instance->host_lock spin lock. Convert this
to a new spin lock that protects the NCR5380_hostdata struct.

atari_NCR5380.c previously used local_irq_save/restore() rather than a
spin lock. Convert this to hostdata->lock in irq mode. For SMP platforms,
the interrupt handler now also acquires the spin lock.

This brings all locking in the two core drivers into agreement.

Adding this locking also means that a bunch of volatile qualifiers can be
removed from the members of the NCR5380_hostdata struct. This is done in
a subsequent patch.

Proper locking will allow the abort handler to locate a command being
aborted. This is presently impossible if the abort handler is invoked when
the command has been moved from a queue to a pointer on the stack. (If
eh_abort_handler can't determine whether a command has been completed
or is still being processed then it can't decide whether to return
success or failure.)

The hostdata spin lock is now held when calling NCR5380_select() and
NCR5380_information_transfer(). Where possible, the lock is dropped for
polling and PIO transfers.

Clean up the now-redundant SELECT_ENABLE_REG writes, that used to provide
limited mutual exclusion between information_transfer() and reselect().

Accessing hostdata->connected without data races means taking the lock;
cleanup these accesses.

The new spin lock falls away for m68k and other UP builds, so this should
have little impact there. In the SMP case the new lock should be
uncontested even when the SCSI bus is contested.

Signed-off-by: Finn Thain 


Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/