Re: [PATCH v3 37/77] ncr5380: Standardize work queueing algorithm

2015-12-21 Thread Hannes Reinecke

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

The complex main_running/queue_main mechanism is peculiar to
atari_NCR5380.c. It isn't SMP safe and offers little value given that
the work queue already offers concurrency management. Remove this
complexity to bring atari_NCR5380.c closer to NCR5380.c.

It is not a good idea to call the information transfer state machine from
queuecommand because, according to Documentation/scsi/scsi_mid_low_api.txt
that could happen in soft irq context. Fix this.

Signed-off-by: Finn Thain 

---
  drivers/scsi/NCR5380.h   |1
  drivers/scsi/atari_NCR5380.c |   80 
+++
  2 files changed, 6 insertions(+), 75 deletions(-)


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 37/77] ncr5380: Standardize work queueing algorithm

2015-12-21 Thread Finn Thain
The complex main_running/queue_main mechanism is peculiar to
atari_NCR5380.c. It isn't SMP safe and offers little value given that
the work queue already offers concurrency management. Remove this
complexity to bring atari_NCR5380.c closer to NCR5380.c.

It is not a good idea to call the information transfer state machine from
queuecommand because, according to Documentation/scsi/scsi_mid_low_api.txt
that could happen in soft irq context. Fix this.

Signed-off-by: Finn Thain 

---
 drivers/scsi/NCR5380.h   |1 
 drivers/scsi/atari_NCR5380.c |   80 +++
 2 files changed, 6 insertions(+), 75 deletions(-)

Index: linux/drivers/scsi/NCR5380.h
===
--- linux.orig/drivers/scsi/NCR5380.h   2015-12-22 12:16:24.0 +1100
+++ linux/drivers/scsi/NCR5380.h2015-12-22 12:16:25.0 +1100
@@ -262,7 +262,6 @@ struct NCR5380_hostdata {
   * transfer to handle chip overruns */
int retain_dma_intr;
struct work_struct main_task;
-   volatile int main_running;
 #ifdef SUPPORT_TAGS
struct tag_alloc TagAlloc[8][8];/* 8 targets and 8 LUNs */
 #endif
Index: linux/drivers/scsi/atari_NCR5380.c
===
--- linux.orig/drivers/scsi/atari_NCR5380.c 2015-12-22 12:16:23.0 
+1100
+++ linux/drivers/scsi/atari_NCR5380.c  2015-12-22 12:16:25.0 +1100
@@ -602,36 +602,6 @@ static void NCR5380_print_phase(struct S
 
 #endif
 
-/*
- * ++roman: New scheme of calling NCR5380_main()
- *
- * If we're not in an interrupt, we can call our main directly, it cannot be
- * already running. Else, we queue it on a task queue, if not 'main_running'
- * tells us that a lower level is already executing it. This way,
- * 'main_running' needs not be protected in a special way.
- *
- * queue_main() is a utility function for putting our main onto the task
- * queue, if main_running is false. It should be called only from a
- * interrupt or bottom half.
- */
-
-#include 
-#include 
-#include 
-
-static inline void queue_main(struct NCR5380_hostdata *hostdata)
-{
-   if (!hostdata->main_running) {
-   /* If in interrupt and NCR5380_main() not already running,
-  queue it on the 'immediate' task queue, to be processed
-  immediately after the current interrupt processing has
-  finished. */
-   queue_work(hostdata->work_q, &hostdata->main_task);
-   }
-   /* else: nothing to do: the running NCR5380_main() will pick up
-  any newly queued command. */
-}
-
 /**
  * NCR58380_info - report driver and host information
  * @instance: relevant scsi host instance
@@ -714,8 +684,6 @@ static void __maybe_unused NCR5380_print
hostdata = (struct NCR5380_hostdata *)instance->hostdata;
 
local_irq_save(flags);
-   printk("NCR5380: coroutine is%s running.\n",
-   hostdata->main_running ? "" : "n't");
if (!hostdata->connected)
printk("scsi%d: no currently connected command\n", HOSTNO);
else
@@ -757,8 +725,6 @@ static int __maybe_unused NCR5380_show_i
hostdata = (struct NCR5380_hostdata *)instance->hostdata;
 
local_irq_save(flags);
-   seq_printf(m, "NCR5380: coroutine is%s running.\n",
-   hostdata->main_running ? "" : "n't");
if (!hostdata->connected)
seq_printf(m, "scsi%d: no currently connected command\n", 
HOSTNO);
else
@@ -997,17 +963,8 @@ static int NCR5380_queue_command(struct
dprintk(NDEBUG_QUEUES, "scsi%d: command added to %s of queue\n", 
H_NO(cmd),
  (cmd->cmnd[0] == REQUEST_SENSE) ? "head" : "tail");
 
-   /* If queue_command() is called from an interrupt (real one or bottom
-* half), we let queue_main() do the job of taking care about main. If 
it
-* is already running, this is a no-op, else main will be queued.
-*
-* If we're not in an interrupt, we can call NCR5380_main()
-* unconditionally, because it cannot be already running.
-*/
-   if (in_interrupt() || irqs_disabled())
-   queue_main(hostdata);
-   else
-   NCR5380_main(&hostdata->main_task);
+   /* Kick off command processing */
+   queue_work(hostdata->work_q, &hostdata->main_task);
return 0;
 }
 
@@ -1044,30 +1001,11 @@ static void NCR5380_main(struct work_str
unsigned long flags;
 
/*
-* We run (with interrupts disabled) until we're sure that none of
-* the host adapters have anything that can be done, at which point
-* we set main_running to 0 and exit.
-*
-* Interrupts are enabled before doing various other internal
-* instructions, after we've decided that we need to run through
-* the loop again.
-*
-*