Re: [PATCH v3 57/77] ncr5380: Use standard list data structure

2015-12-21 Thread Hannes Reinecke 

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

The NCR5380 drivers have a home-spun linked list implementation for
scsi_cmnd structs that uses cmd->host_scribble as a 'next' pointer. Adopt
the standard list_head data structure and list operations instead. Remove
the eh_abort_handler rather than convert it. Doing the conversion would
only be churn because the existing EH handlers don't work and get replaced
in a subsequent patch.

Signed-off-by: Finn Thain 

---

Changed since v2:
- Fix NULL pointer dereference in NCR5380_reselect() when SUPPORT_TAGS is
   enabled in the atari_NCR5380.c core driver.

Well, using ->host_scribble allows for an easy check on the midlayer if 
a command has been properly released by the LLDD. But that's just a 
side-note.


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 57/77] ncr5380: Use standard list data structure

2015-12-21 Thread Finn Thain 
The NCR5380 drivers have a home-spun linked list implementation for
scsi_cmnd structs that uses cmd->host_scribble as a 'next' pointer. Adopt
the standard list_head data structure and list operations instead. Remove
the eh_abort_handler rather than convert it. Doing the conversion would
only be churn because the existing EH handlers don't work and get replaced
in a subsequent patch.

Signed-off-by: Finn Thain 

---

Changed since v2:
- Fix NULL pointer dereference in NCR5380_reselect() when SUPPORT_TAGS is
  enabled in the atari_NCR5380.c core driver.

---
 drivers/scsi/NCR5380.c   |  214 +---
 drivers/scsi/NCR5380.h   |   16 ++
 drivers/scsi/arm/cumana_1.c  |1 
 drivers/scsi/arm/oak.c   |1 
 drivers/scsi/atari_NCR5380.c |  287 ++-
 drivers/scsi/atari_scsi.c|1 
 drivers/scsi/dmx3191d.c  |1 
 drivers/scsi/dtc.c   |1 
 drivers/scsi/g_NCR5380.c |1 
 drivers/scsi/mac_scsi.c  |1 
 drivers/scsi/pas16.c |1 
 drivers/scsi/sun3_scsi.c |1 
 drivers/scsi/t128.c  |1 
 13 files changed, 106 insertions(+), 421 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===
--- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:16:56.0 +1100
+++ linux/drivers/scsi/NCR5380.c2015-12-22 12:16:58.0 +1100
@@ -622,8 +622,9 @@ static int NCR5380_init(struct Scsi_Host
 #endif
spin_lock_init(>lock);
hostdata->connected = NULL;
-   hostdata->issue_queue = NULL;
-   hostdata->disconnected_queue = NULL;
+   INIT_LIST_HEAD(>unissued);
+   INIT_LIST_HEAD(>disconnected);
+
hostdata->flags = flags;

INIT_WORK(>main_task, NCR5380_main);
@@ -738,7 +739,7 @@ static int NCR5380_queue_command(struct
  struct scsi_cmnd *cmd)
 {
struct NCR5380_hostdata *hostdata = shost_priv(instance);
-   struct scsi_cmnd *tmp;
+   struct NCR5380_cmd *ncmd = scsi_cmd_priv(cmd);
unsigned long flags;
 
 #if (NDEBUG & NDEBUG_NO_WRITE)
@@ -752,12 +753,6 @@ static int NCR5380_queue_command(struct
}
 #endif /* (NDEBUG & NDEBUG_NO_WRITE) */
 
-   /* 
-* We use the host_scribble field as a pointer to the next command  
-* in a queue 
-*/
-
-   cmd->host_scribble = NULL;
cmd->result = 0;
 
spin_lock_irqsave(>lock, flags);
@@ -769,13 +764,11 @@ static int NCR5380_queue_command(struct
 * sense data is only guaranteed to be valid while the condition exists.
 */
 
-   if (!(hostdata->issue_queue) || (cmd->cmnd[0] == REQUEST_SENSE)) {
-   cmd->host_scribble = (unsigned char *) hostdata->issue_queue;
-   hostdata->issue_queue = cmd;
-   } else {
-   for (tmp = (struct scsi_cmnd *) hostdata->issue_queue; 
tmp->host_scribble; tmp = (struct scsi_cmnd *) tmp->host_scribble);
-   tmp->host_scribble = (unsigned char *) cmd;
-   }
+   if (cmd->cmnd[0] == REQUEST_SENSE)
+   list_add(>list, >unissued);
+   else
+   list_add_tail(>list, >unissued);
+
spin_unlock_irqrestore(>lock, flags);
 
dsprintk(NDEBUG_QUEUES, instance, "command %p added to %s of queue\n",
@@ -803,7 +796,7 @@ static void NCR5380_main(struct work_str
struct NCR5380_hostdata *hostdata =
container_of(work, struct NCR5380_hostdata, main_task);
struct Scsi_Host *instance = hostdata->host;
-   struct scsi_cmnd *tmp, *prev;
+   struct NCR5380_cmd *ncmd, *n;
int done;

spin_lock_irq(>lock);
@@ -816,23 +809,20 @@ static void NCR5380_main(struct work_str
 * Search through the issue_queue for a command destined
 * for a target that's not busy.
 */
-   for (tmp = (struct scsi_cmnd *) hostdata->issue_queue, 
prev = NULL; tmp; prev = tmp, tmp = (struct scsi_cmnd *) tmp->host_scribble)
-   {
+   list_for_each_entry_safe(ncmd, n, >unissued,
+list) {
+   struct scsi_cmnd *tmp = NCR5380_to_scmd(ncmd);
+
dsprintk(NDEBUG_QUEUES, instance, "main: tmp=%p 
target=%d busy=%d lun=%llu\n",
 tmp, scmd_id(tmp), 
hostdata->busy[scmd_id(tmp)],
 tmp->device->lun);
/*  When we find one, remove it from the issue 
queue. */
if (!(hostdata->busy[tmp->device->id] &
  (1 << (u8)(tmp->device->lun & 0xff {
-   if (prev) {
-   prev->host_scribble =

[PATCH v3 57/77] ncr5380: Use standard list data structure

2015-12-21 Thread Finn Thain 
The NCR5380 drivers have a home-spun linked list implementation for
scsi_cmnd structs that uses cmd->host_scribble as a 'next' pointer. Adopt
the standard list_head data structure and list operations instead. Remove
the eh_abort_handler rather than convert it. Doing the conversion would
only be churn because the existing EH handlers don't work and get replaced
in a subsequent patch.

Signed-off-by: Finn Thain 

---

Changed since v2:
- Fix NULL pointer dereference in NCR5380_reselect() when SUPPORT_TAGS is
  enabled in the atari_NCR5380.c core driver.

---
 drivers/scsi/NCR5380.c   |  214 +---
 drivers/scsi/NCR5380.h   |   16 ++
 drivers/scsi/arm/cumana_1.c  |1 
 drivers/scsi/arm/oak.c   |1 
 drivers/scsi/atari_NCR5380.c |  287 ++-
 drivers/scsi/atari_scsi.c|1 
 drivers/scsi/dmx3191d.c  |1 
 drivers/scsi/dtc.c   |1 
 drivers/scsi/g_NCR5380.c |1 
 drivers/scsi/mac_scsi.c  |1 
 drivers/scsi/pas16.c |1 
 drivers/scsi/sun3_scsi.c |1 
 drivers/scsi/t128.c  |1 
 13 files changed, 106 insertions(+), 421 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===
--- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:16:56.0 +1100
+++ linux/drivers/scsi/NCR5380.c2015-12-22 12:16:58.0 +1100
@@ -622,8 +622,9 @@ static int NCR5380_init(struct Scsi_Host
 #endif
spin_lock_init(>lock);
hostdata->connected = NULL;
-   hostdata->issue_queue = NULL;
-   hostdata->disconnected_queue = NULL;
+   INIT_LIST_HEAD(>unissued);
+   INIT_LIST_HEAD(>disconnected);
+
hostdata->flags = flags;

INIT_WORK(>main_task, NCR5380_main);
@@ -738,7 +739,7 @@ static int NCR5380_queue_command(struct
  struct scsi_cmnd *cmd)
 {
struct NCR5380_hostdata *hostdata = shost_priv(instance);
-   struct scsi_cmnd *tmp;
+   struct NCR5380_cmd *ncmd = scsi_cmd_priv(cmd);
unsigned long flags;
 
 #if (NDEBUG & NDEBUG_NO_WRITE)
@@ -752,12 +753,6 @@ static int NCR5380_queue_command(struct
}
 #endif /* (NDEBUG & NDEBUG_NO_WRITE) */
 
-   /* 
-* We use the host_scribble field as a pointer to the next command  
-* in a queue 
-*/
-
-   cmd->host_scribble = NULL;
cmd->result = 0;
 
spin_lock_irqsave(>lock, flags);
@@ -769,13 +764,11 @@ static int NCR5380_queue_command(struct
 * sense data is only guaranteed to be valid while the condition exists.
 */
 
-   if (!(hostdata->issue_queue) || (cmd->cmnd[0] == REQUEST_SENSE)) {
-   cmd->host_scribble = (unsigned char *) hostdata->issue_queue;
-   hostdata->issue_queue = cmd;
-   } else {
-   for (tmp = (struct scsi_cmnd *) hostdata->issue_queue; 
tmp->host_scribble; tmp = (struct scsi_cmnd *) tmp->host_scribble);
-   tmp->host_scribble = (unsigned char *) cmd;
-   }
+   if (cmd->cmnd[0] == REQUEST_SENSE)
+   list_add(>list, >unissued);
+   else
+   list_add_tail(>list, >unissued);
+
spin_unlock_irqrestore(>lock, flags);
 
dsprintk(NDEBUG_QUEUES, instance, "command %p added to %s of queue\n",
@@ -803,7 +796,7 @@ static void NCR5380_main(struct work_str
struct NCR5380_hostdata *hostdata =
container_of(work, struct NCR5380_hostdata, main_task);
struct Scsi_Host *instance = hostdata->host;
-   struct scsi_cmnd *tmp, *prev;
+   struct NCR5380_cmd *ncmd, *n;
int done;

spin_lock_irq(>lock);
@@ -816,23 +809,20 @@ static void NCR5380_main(struct work_str
 * Search through the issue_queue for a command destined
 * for a target that's not busy.
 */
-   for (tmp = (struct scsi_cmnd *) hostdata->issue_queue, 
prev = NULL; tmp; prev = tmp, tmp = (struct scsi_cmnd *) tmp->host_scribble)
-   {
+   list_for_each_entry_safe(ncmd, n, >unissued,
+list) {
+   struct scsi_cmnd *tmp = NCR5380_to_scmd(ncmd);
+
dsprintk(NDEBUG_QUEUES, instance, "main: tmp=%p 
target=%d busy=%d lun=%llu\n",
 tmp, scmd_id(tmp), 
hostdata->busy[scmd_id(tmp)],
 tmp->device->lun);
/*  When we find one, remove it from the issue 
queue. */
if (!(hostdata->busy[tmp->device->id] &
  (1 << (u8)(tmp->device->lun & 0xff {
-   if (prev) {
-

Re: [PATCH v3 57/77] ncr5380: Use standard list data structure

2015-12-21 Thread Hannes Reinecke 

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

The NCR5380 drivers have a home-spun linked list implementation for
scsi_cmnd structs that uses cmd->host_scribble as a 'next' pointer. Adopt
the standard list_head data structure and list operations instead. Remove
the eh_abort_handler rather than convert it. Doing the conversion would
only be churn because the existing EH handlers don't work and get replaced
in a subsequent patch.

Signed-off-by: Finn Thain 

---

Changed since v2:
- Fix NULL pointer dereference in NCR5380_reselect() when SUPPORT_TAGS is
   enabled in the atari_NCR5380.c core driver.

Well, using ->host_scribble allows for an easy check on the midlayer if 
a command has been properly released by the LLDD. But that's just a 
side-note.


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/