subject:"\[PATCH v3 20\/77\] ncr5380\: Introduce unbound workqueue"

Re: [PATCH v3 20/77] ncr5380: Introduce unbound workqueue

2015-12-22 Thread Hannes Reinecke


On 12/22/2015 01:44 PM, Finn Thain wrote:


On Tue, 22 Dec 2015, Hannes Reinecke wrote:


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

Allocate a work queue that will permit busy waiting and sleeping. This
means NCR5380_init() can potentially fail, so add this error path.

Signed-off-by: Finn Thain 

---

In subsequent patches, the work function adopts this work queue so it
can sleep while polling, which allows the removal of some flawed and
complicated code in NCR5380_select() in NCR5380.c.

Changed since v1:
- Dropped WQ_CPU_INTENSIVE flag because Documentation/workqueue.txt says it
"is meaningless for unbound wq".

---
   drivers/scsi/NCR5380.c   |   15 +++
   drivers/scsi/NCR5380.h   |1 +
   drivers/scsi/arm/cumana_1.c  |8 ++--
   drivers/scsi/arm/oak.c   |8 ++--
   drivers/scsi/atari_NCR5380.c |8 +++-
   drivers/scsi/atari_scsi.c|5 -
   drivers/scsi/dmx3191d.c  |   17 +++--
   drivers/scsi/dtc.c   |   11 +--
   drivers/scsi/g_NCR5380.c |   31 +++
   drivers/scsi/mac_scsi.c  |5 -
   drivers/scsi/pas16.c |   10 --
   drivers/scsi/sun3_scsi.c |5 -
   drivers/scsi/t128.c  |   13 ++---
   13 files changed, 96 insertions(+), 41 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===
--- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:15:52.0 +1100
+++ linux/drivers/scsi/NCR5380.c2015-12-22 12:15:56.0 +1100
@@ -514,7 +514,7 @@ static int should_disconnect(unsigned ch
   static void NCR5380_set_timer(struct NCR5380_hostdata *hostdata, unsigned
   long timeout)
   {
hostdata->time_expires = jiffies + timeout;
-   schedule_delayed_work(>coroutine, timeout);
+   queue_delayed_work(hostdata->work_q, >coroutine, timeout);
   }


@@ -791,7 +791,12 @@ static int NCR5380_init(struct Scsi_Host
hostdata->disconnected_queue = NULL;

INIT_DELAYED_WORK(>coroutine, NCR5380_main);
-   
+   hostdata->work_q = alloc_workqueue("ncr5380_%d",
+   WQ_UNBOUND | WQ_MEM_RECLAIM,
+   1, instance->host_no);
+   if (!hostdata->work_q)
+   return -ENOMEM;
+
/* The CHECK code seems to break the 53C400. Will check it later maybe */
if (flags & FLAG_NCR53C400)
 hostdata->flags = FLAG_HAS_LAST_BYTE_SENT | flags;


Wouldn't it be better to use a normal (ie bound) workqueue here?


The polling algorithm I've used requires that the workqueue item is free
to busy-wait and sleep. Perhaps a kthread_worker would be better?


SCSI-2 is pretty much single-threaded, so shifting things onto arbitrary
CPUs don't sound very appealing.


Most of these drivers only run on UP systems. For the x86 drivers, I
suspect that the cache miss penalty would be insignificant compared to
some of the other overheads. The 5380 chip requires that the CPU is
involved in SCSI bus signalling and merely accessing a chip register
takes over a microsecond.


I know.
But using a bound workqueue would mean you could use 
'create_workqueue()' instead of open-coding it :-)


But in the end it's up to you. If the thing works I'm not that concerned.

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/

Re: [PATCH v3 20/77] ncr5380: Introduce unbound workqueue

2015-12-22 Thread Finn Thain


On Tue, 22 Dec 2015, Hannes Reinecke wrote:

> On 12/22/2015 02:17 AM, Finn Thain wrote:
> > Allocate a work queue that will permit busy waiting and sleeping. This
> > means NCR5380_init() can potentially fail, so add this error path.
> >
> > Signed-off-by: Finn Thain 
> >
> > ---
> >
> > In subsequent patches, the work function adopts this work queue so it
> > can sleep while polling, which allows the removal of some flawed and
> > complicated code in NCR5380_select() in NCR5380.c.
> >
> > Changed since v1:
> > - Dropped WQ_CPU_INTENSIVE flag because Documentation/workqueue.txt says it
> >"is meaningless for unbound wq".
> >
> > ---
> >   drivers/scsi/NCR5380.c   |   15 +++
> >   drivers/scsi/NCR5380.h   |1 +
> >   drivers/scsi/arm/cumana_1.c  |8 ++--
> >   drivers/scsi/arm/oak.c   |8 ++--
> >   drivers/scsi/atari_NCR5380.c |8 +++-
> >   drivers/scsi/atari_scsi.c|5 -
> >   drivers/scsi/dmx3191d.c  |   17 +++--
> >   drivers/scsi/dtc.c   |   11 +--
> >   drivers/scsi/g_NCR5380.c |   31 +++
> >   drivers/scsi/mac_scsi.c  |5 -
> >   drivers/scsi/pas16.c |   10 --
> >   drivers/scsi/sun3_scsi.c |5 -
> >   drivers/scsi/t128.c  |   13 ++---
> >   13 files changed, 96 insertions(+), 41 deletions(-)
> >
> > Index: linux/drivers/scsi/NCR5380.c
> > ===
> > --- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:15:52.0 
> > +1100
> > +++ linux/drivers/scsi/NCR5380.c2015-12-22 12:15:56.0 +1100
> > @@ -514,7 +514,7 @@ static int should_disconnect(unsigned ch
> >   static void NCR5380_set_timer(struct NCR5380_hostdata *hostdata, unsigned
> >   long timeout)
> >   {
> > hostdata->time_expires = jiffies + timeout;
> > -   schedule_delayed_work(>coroutine, timeout);
> > +   queue_delayed_work(hostdata->work_q, >coroutine, timeout);
> >   }
> >
> >
> > @@ -791,7 +791,12 @@ static int NCR5380_init(struct Scsi_Host
> >hostdata->disconnected_queue = NULL;
> >
> >INIT_DELAYED_WORK(>coroutine, NCR5380_main);
> > -   
> > +   hostdata->work_q = alloc_workqueue("ncr5380_%d",
> > +   WQ_UNBOUND | WQ_MEM_RECLAIM,
> > +   1, instance->host_no);
> > +   if (!hostdata->work_q)
> > +   return -ENOMEM;
> > +
> >/* The CHECK code seems to break the 53C400. Will check it later maybe */
> >if (flags & FLAG_NCR53C400)
> > hostdata->flags = FLAG_HAS_LAST_BYTE_SENT | flags;
> 
> Wouldn't it be better to use a normal (ie bound) workqueue here?

The polling algorithm I've used requires that the workqueue item is free 
to busy-wait and sleep. Perhaps a kthread_worker would be better?

> SCSI-2 is pretty much single-threaded, so shifting things onto arbitrary 
> CPUs don't sound very appealing.

Most of these drivers only run on UP systems. For the x86 drivers, I 
suspect that the cache miss penalty would be insignificant compared to 
some of the other overheads. The 5380 chip requires that the CPU is 
involved in SCSI bus signalling and merely accessing a chip register 
takes over a microsecond.

-- 

> 
> Cheers,
> 
> Hannes
> 
--
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/

Re: [PATCH v3 20/77] ncr5380: Introduce unbound workqueue

2015-12-22 Thread Finn Thain


On Tue, 22 Dec 2015, Hannes Reinecke wrote:

> On 12/22/2015 02:17 AM, Finn Thain wrote:
> > Allocate a work queue that will permit busy waiting and sleeping. This
> > means NCR5380_init() can potentially fail, so add this error path.
> >
> > Signed-off-by: Finn Thain 
> >
> > ---
> >
> > In subsequent patches, the work function adopts this work queue so it
> > can sleep while polling, which allows the removal of some flawed and
> > complicated code in NCR5380_select() in NCR5380.c.
> >
> > Changed since v1:
> > - Dropped WQ_CPU_INTENSIVE flag because Documentation/workqueue.txt says it
> >"is meaningless for unbound wq".
> >
> > ---
> >   drivers/scsi/NCR5380.c   |   15 +++
> >   drivers/scsi/NCR5380.h   |1 +
> >   drivers/scsi/arm/cumana_1.c  |8 ++--
> >   drivers/scsi/arm/oak.c   |8 ++--
> >   drivers/scsi/atari_NCR5380.c |8 +++-
> >   drivers/scsi/atari_scsi.c|5 -
> >   drivers/scsi/dmx3191d.c  |   17 +++--
> >   drivers/scsi/dtc.c   |   11 +--
> >   drivers/scsi/g_NCR5380.c |   31 +++
> >   drivers/scsi/mac_scsi.c  |5 -
> >   drivers/scsi/pas16.c |   10 --
> >   drivers/scsi/sun3_scsi.c |5 -
> >   drivers/scsi/t128.c  |   13 ++---
> >   13 files changed, 96 insertions(+), 41 deletions(-)
> >
> > Index: linux/drivers/scsi/NCR5380.c
> > ===
> > --- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:15:52.0 
> > +1100
> > +++ linux/drivers/scsi/NCR5380.c2015-12-22 12:15:56.0 +1100
> > @@ -514,7 +514,7 @@ static int should_disconnect(unsigned ch
> >   static void NCR5380_set_timer(struct NCR5380_hostdata *hostdata, unsigned
> >   long timeout)
> >   {
> > hostdata->time_expires = jiffies + timeout;
> > -   schedule_delayed_work(>coroutine, timeout);
> > +   queue_delayed_work(hostdata->work_q, >coroutine, timeout);
> >   }
> >
> >
> > @@ -791,7 +791,12 @@ static int NCR5380_init(struct Scsi_Host
> >hostdata->disconnected_queue = NULL;
> >
> >INIT_DELAYED_WORK(>coroutine, NCR5380_main);
> > -   
> > +   hostdata->work_q = alloc_workqueue("ncr5380_%d",
> > +   WQ_UNBOUND | WQ_MEM_RECLAIM,
> > +   1, instance->host_no);
> > +   if (!hostdata->work_q)
> > +   return -ENOMEM;
> > +
> >/* The CHECK code seems to break the 53C400. Will check it later maybe */
> >if (flags & FLAG_NCR53C400)
> > hostdata->flags = FLAG_HAS_LAST_BYTE_SENT | flags;
> 
> Wouldn't it be better to use a normal (ie bound) workqueue here?

The polling algorithm I've used requires that the workqueue item is free 
to busy-wait and sleep. Perhaps a kthread_worker would be better?

> SCSI-2 is pretty much single-threaded, so shifting things onto arbitrary 
> CPUs don't sound very appealing.

Most of these drivers only run on UP systems. For the x86 drivers, I 
suspect that the cache miss penalty would be insignificant compared to 
some of the other overheads. The 5380 chip requires that the CPU is 
involved in SCSI bus signalling and merely accessing a chip register 
takes over a microsecond.

-- 

> 
> Cheers,
> 
> Hannes
> 
--
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/

Re: [PATCH v3 20/77] ncr5380: Introduce unbound workqueue

2015-12-22 Thread Hannes Reinecke


On 12/22/2015 01:44 PM, Finn Thain wrote:


On Tue, 22 Dec 2015, Hannes Reinecke wrote:


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

Allocate a work queue that will permit busy waiting and sleeping. This
means NCR5380_init() can potentially fail, so add this error path.

Signed-off-by: Finn Thain 

---

In subsequent patches, the work function adopts this work queue so it
can sleep while polling, which allows the removal of some flawed and
complicated code in NCR5380_select() in NCR5380.c.

Changed since v1:
- Dropped WQ_CPU_INTENSIVE flag because Documentation/workqueue.txt says it
"is meaningless for unbound wq".

---
   drivers/scsi/NCR5380.c   |   15 +++
   drivers/scsi/NCR5380.h   |1 +
   drivers/scsi/arm/cumana_1.c  |8 ++--
   drivers/scsi/arm/oak.c   |8 ++--
   drivers/scsi/atari_NCR5380.c |8 +++-
   drivers/scsi/atari_scsi.c|5 -
   drivers/scsi/dmx3191d.c  |   17 +++--
   drivers/scsi/dtc.c   |   11 +--
   drivers/scsi/g_NCR5380.c |   31 +++
   drivers/scsi/mac_scsi.c  |5 -
   drivers/scsi/pas16.c |   10 --
   drivers/scsi/sun3_scsi.c |5 -
   drivers/scsi/t128.c  |   13 ++---
   13 files changed, 96 insertions(+), 41 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===
--- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:15:52.0 +1100
+++ linux/drivers/scsi/NCR5380.c2015-12-22 12:15:56.0 +1100
@@ -514,7 +514,7 @@ static int should_disconnect(unsigned ch
   static void NCR5380_set_timer(struct NCR5380_hostdata *hostdata, unsigned
   long timeout)
   {
hostdata->time_expires = jiffies + timeout;
-   schedule_delayed_work(>coroutine, timeout);
+   queue_delayed_work(hostdata->work_q, >coroutine, timeout);
   }


@@ -791,7 +791,12 @@ static int NCR5380_init(struct Scsi_Host
hostdata->disconnected_queue = NULL;

INIT_DELAYED_WORK(>coroutine, NCR5380_main);
-   
+   hostdata->work_q = alloc_workqueue("ncr5380_%d",
+   WQ_UNBOUND | WQ_MEM_RECLAIM,
+   1, instance->host_no);
+   if (!hostdata->work_q)
+   return -ENOMEM;
+
/* The CHECK code seems to break the 53C400. Will check it later maybe */
if (flags & FLAG_NCR53C400)
 hostdata->flags = FLAG_HAS_LAST_BYTE_SENT | flags;


Wouldn't it be better to use a normal (ie bound) workqueue here?


The polling algorithm I've used requires that the workqueue item is free
to busy-wait and sleep. Perhaps a kthread_worker would be better?


SCSI-2 is pretty much single-threaded, so shifting things onto arbitrary
CPUs don't sound very appealing.


Most of these drivers only run on UP systems. For the x86 drivers, I
suspect that the cache miss penalty would be insignificant compared to
some of the other overheads. The 5380 chip requires that the CPU is
involved in SCSI bus signalling and merely accessing a chip register
takes over a microsecond.


I know.
But using a bound workqueue would mean you could use 
'create_workqueue()' instead of open-coding it :-)


But in the end it's up to you. If the thing works I'm not that concerned.

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/

Re: [PATCH v3 20/77] ncr5380: Introduce unbound workqueue

2015-12-21 Thread Hannes Reinecke


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

Allocate a work queue that will permit busy waiting and sleeping. This
means NCR5380_init() can potentially fail, so add this error path.

Signed-off-by: Finn Thain 

---

In subsequent patches, the work function adopts this work queue so it
can sleep while polling, which allows the removal of some flawed and
complicated code in NCR5380_select() in NCR5380.c.

Changed since v1:
- Dropped WQ_CPU_INTENSIVE flag because Documentation/workqueue.txt says it
   "is meaningless for unbound wq".

---
  drivers/scsi/NCR5380.c   |   15 +++
  drivers/scsi/NCR5380.h   |1 +
  drivers/scsi/arm/cumana_1.c  |8 ++--
  drivers/scsi/arm/oak.c   |8 ++--
  drivers/scsi/atari_NCR5380.c |8 +++-
  drivers/scsi/atari_scsi.c|5 -
  drivers/scsi/dmx3191d.c  |   17 +++--
  drivers/scsi/dtc.c   |   11 +--
  drivers/scsi/g_NCR5380.c |   31 +++
  drivers/scsi/mac_scsi.c  |5 -
  drivers/scsi/pas16.c |   10 --
  drivers/scsi/sun3_scsi.c |5 -
  drivers/scsi/t128.c  |   13 ++---
  13 files changed, 96 insertions(+), 41 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===
--- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:15:52.0 +1100
+++ linux/drivers/scsi/NCR5380.c2015-12-22 12:15:56.0 +1100
@@ -514,7 +514,7 @@ static int should_disconnect(unsigned ch
  static void NCR5380_set_timer(struct NCR5380_hostdata *hostdata, unsigned 
long timeout)
  {
hostdata->time_expires = jiffies + timeout;
-   schedule_delayed_work(>coroutine, timeout);
+   queue_delayed_work(hostdata->work_q, >coroutine, timeout);
  }


@@ -791,7 +791,12 @@ static int NCR5380_init(struct Scsi_Host
hostdata->disconnected_queue = NULL;

INIT_DELAYED_WORK(>coroutine, NCR5380_main);
-   
+   hostdata->work_q = alloc_workqueue("ncr5380_%d",
+   WQ_UNBOUND | WQ_MEM_RECLAIM,
+   1, instance->host_no);
+   if (!hostdata->work_q)
+   return -ENOMEM;
+
/* The CHECK code seems to break the 53C400. Will check it later maybe 
*/
if (flags & FLAG_NCR53C400)
hostdata->flags = FLAG_HAS_LAST_BYTE_SENT | flags;


Wouldn't it be better to use a normal (ie bound) workqueue here?
SCSI-2 is pretty much single-threaded, so shifting things onto arbitrary 
CPUs don't sound very appealing.


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 20/77] ncr5380: Introduce unbound workqueue

2015-12-21 Thread Finn Thain

Allocate a work queue that will permit busy waiting and sleeping. This
means NCR5380_init() can potentially fail, so add this error path.

Signed-off-by: Finn Thain 

---

In subsequent patches, the work function adopts this work queue so it
can sleep while polling, which allows the removal of some flawed and
complicated code in NCR5380_select() in NCR5380.c.

Changed since v1:
- Dropped WQ_CPU_INTENSIVE flag because Documentation/workqueue.txt says it
  "is meaningless for unbound wq".

---
 drivers/scsi/NCR5380.c   |   15 +++
 drivers/scsi/NCR5380.h   |1 +
 drivers/scsi/arm/cumana_1.c  |8 ++--
 drivers/scsi/arm/oak.c   |8 ++--
 drivers/scsi/atari_NCR5380.c |8 +++-
 drivers/scsi/atari_scsi.c|5 -
 drivers/scsi/dmx3191d.c  |   17 +++--
 drivers/scsi/dtc.c   |   11 +--
 drivers/scsi/g_NCR5380.c |   31 +++
 drivers/scsi/mac_scsi.c  |5 -
 drivers/scsi/pas16.c |   10 --
 drivers/scsi/sun3_scsi.c |5 -
 drivers/scsi/t128.c  |   13 ++---
 13 files changed, 96 insertions(+), 41 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===
--- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:15:52.0 +1100
+++ linux/drivers/scsi/NCR5380.c2015-12-22 12:15:56.0 +1100
@@ -514,7 +514,7 @@ static int should_disconnect(unsigned ch
 static void NCR5380_set_timer(struct NCR5380_hostdata *hostdata, unsigned long 
timeout)
 {
hostdata->time_expires = jiffies + timeout;
-   schedule_delayed_work(>coroutine, timeout);
+   queue_delayed_work(hostdata->work_q, >coroutine, timeout);
 }
 
 
@@ -791,7 +791,12 @@ static int NCR5380_init(struct Scsi_Host
hostdata->disconnected_queue = NULL;

INIT_DELAYED_WORK(>coroutine, NCR5380_main);
-   
+   hostdata->work_q = alloc_workqueue("ncr5380_%d",
+   WQ_UNBOUND | WQ_MEM_RECLAIM,
+   1, instance->host_no);
+   if (!hostdata->work_q)
+   return -ENOMEM;
+
/* The CHECK code seems to break the 53C400. Will check it later maybe 
*/
if (flags & FLAG_NCR53C400)
hostdata->flags = FLAG_HAS_LAST_BYTE_SENT | flags;
@@ -872,6 +877,7 @@ static void NCR5380_exit(struct Scsi_Hos
struct NCR5380_hostdata *hostdata = (struct NCR5380_hostdata *) 
instance->hostdata;
 
cancel_delayed_work_sync(>coroutine);
+   destroy_workqueue(hostdata->work_q);
 }
 
 /**
@@ -932,7 +938,7 @@ static int NCR5380_queue_command_lck(str
 
/* Run the coroutine if it isn't already running. */
/* Kick off command processing */
-   schedule_delayed_work(>coroutine, 0);
+   queue_delayed_work(hostdata->work_q, >coroutine, 0);
return 0;
 }
 
@@ -1128,7 +1134,8 @@ static irqreturn_t NCR5380_intr(int dumm
}   /* if BASR_IRQ */
spin_unlock_irqrestore(instance->host_lock, flags);
if(!done)
-   schedule_delayed_work(>coroutine, 0);
+   queue_delayed_work(hostdata->work_q,
+  >coroutine, 0);
} while (!done);
return IRQ_HANDLED;
 }
Index: linux/drivers/scsi/NCR5380.h
===
--- linux.orig/drivers/scsi/NCR5380.h   2015-12-22 12:15:48.0 +1100
+++ linux/drivers/scsi/NCR5380.h2015-12-22 12:15:56.0 +1100
@@ -284,6 +284,7 @@ struct NCR5380_hostdata {
unsigned spin_max_r;
unsigned spin_max_w;
 #endif
+   struct workqueue_struct *work_q;
 };
 
 #ifdef __KERNEL__
Index: linux/drivers/scsi/arm/cumana_1.c
===
--- linux.orig/drivers/scsi/arm/cumana_1.c  2015-12-22 12:15:54.0 
+1100
+++ linux/drivers/scsi/arm/cumana_1.c   2015-12-22 12:15:56.0 +1100
@@ -238,7 +238,9 @@ static int cumanascsi1_probe(struct expa
 
host->irq = ec->irq;
 
-   NCR5380_init(host, 0);
+   ret = NCR5380_init(host, 0);
+   if (ret)
+   goto out_unmap;
 
NCR5380_maybe_reset_bus(host);
 
@@ -250,7 +252,7 @@ static int cumanascsi1_probe(struct expa
if (ret) {
printk("scsi%d: IRQ%d not free: %d\n",
host->host_no, host->irq, ret);
-   goto out_unmap;
+   goto out_exit;
}
 
ret = scsi_add_host(host, >dev);
@@ -262,6 +264,8 @@ static int cumanascsi1_probe(struct expa
 
  out_free_irq:
free_irq(host->irq, host);
+ out_exit:
+   NCR5380_exit(host);
  out_unmap:
iounmap(priv(host)->base);
iounmap(priv(host)->dma);
Index: linux/drivers/scsi/arm/oak.c
===
---

[PATCH v3 20/77] ncr5380: Introduce unbound workqueue

2015-12-21 Thread Finn Thain

Allocate a work queue that will permit busy waiting and sleeping. This
means NCR5380_init() can potentially fail, so add this error path.

Signed-off-by: Finn Thain 

---

In subsequent patches, the work function adopts this work queue so it
can sleep while polling, which allows the removal of some flawed and
complicated code in NCR5380_select() in NCR5380.c.

Changed since v1:
- Dropped WQ_CPU_INTENSIVE flag because Documentation/workqueue.txt says it
  "is meaningless for unbound wq".

---
 drivers/scsi/NCR5380.c   |   15 +++
 drivers/scsi/NCR5380.h   |1 +
 drivers/scsi/arm/cumana_1.c  |8 ++--
 drivers/scsi/arm/oak.c   |8 ++--
 drivers/scsi/atari_NCR5380.c |8 +++-
 drivers/scsi/atari_scsi.c|5 -
 drivers/scsi/dmx3191d.c  |   17 +++--
 drivers/scsi/dtc.c   |   11 +--
 drivers/scsi/g_NCR5380.c |   31 +++
 drivers/scsi/mac_scsi.c  |5 -
 drivers/scsi/pas16.c |   10 --
 drivers/scsi/sun3_scsi.c |5 -
 drivers/scsi/t128.c  |   13 ++---
 13 files changed, 96 insertions(+), 41 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===
--- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:15:52.0 +1100
+++ linux/drivers/scsi/NCR5380.c2015-12-22 12:15:56.0 +1100
@@ -514,7 +514,7 @@ static int should_disconnect(unsigned ch
 static void NCR5380_set_timer(struct NCR5380_hostdata *hostdata, unsigned long 
timeout)
 {
hostdata->time_expires = jiffies + timeout;
-   schedule_delayed_work(>coroutine, timeout);
+   queue_delayed_work(hostdata->work_q, >coroutine, timeout);
 }
 
 
@@ -791,7 +791,12 @@ static int NCR5380_init(struct Scsi_Host
hostdata->disconnected_queue = NULL;

INIT_DELAYED_WORK(>coroutine, NCR5380_main);
-   
+   hostdata->work_q = alloc_workqueue("ncr5380_%d",
+   WQ_UNBOUND | WQ_MEM_RECLAIM,
+   1, instance->host_no);
+   if (!hostdata->work_q)
+   return -ENOMEM;
+
/* The CHECK code seems to break the 53C400. Will check it later maybe 
*/
if (flags & FLAG_NCR53C400)
hostdata->flags = FLAG_HAS_LAST_BYTE_SENT | flags;
@@ -872,6 +877,7 @@ static void NCR5380_exit(struct Scsi_Hos
struct NCR5380_hostdata *hostdata = (struct NCR5380_hostdata *) 
instance->hostdata;
 
cancel_delayed_work_sync(>coroutine);
+   destroy_workqueue(hostdata->work_q);
 }
 
 /**
@@ -932,7 +938,7 @@ static int NCR5380_queue_command_lck(str
 
/* Run the coroutine if it isn't already running. */
/* Kick off command processing */
-   schedule_delayed_work(>coroutine, 0);
+   queue_delayed_work(hostdata->work_q, >coroutine, 0);
return 0;
 }
 
@@ -1128,7 +1134,8 @@ static irqreturn_t NCR5380_intr(int dumm
}   /* if BASR_IRQ */
spin_unlock_irqrestore(instance->host_lock, flags);
if(!done)
-   schedule_delayed_work(>coroutine, 0);
+   queue_delayed_work(hostdata->work_q,
+  >coroutine, 0);
} while (!done);
return IRQ_HANDLED;
 }
Index: linux/drivers/scsi/NCR5380.h
===
--- linux.orig/drivers/scsi/NCR5380.h   2015-12-22 12:15:48.0 +1100
+++ linux/drivers/scsi/NCR5380.h2015-12-22 12:15:56.0 +1100
@@ -284,6 +284,7 @@ struct NCR5380_hostdata {
unsigned spin_max_r;
unsigned spin_max_w;
 #endif
+   struct workqueue_struct *work_q;
 };
 
 #ifdef __KERNEL__
Index: linux/drivers/scsi/arm/cumana_1.c
===
--- linux.orig/drivers/scsi/arm/cumana_1.c  2015-12-22 12:15:54.0 
+1100
+++ linux/drivers/scsi/arm/cumana_1.c   2015-12-22 12:15:56.0 +1100
@@ -238,7 +238,9 @@ static int cumanascsi1_probe(struct expa
 
host->irq = ec->irq;
 
-   NCR5380_init(host, 0);
+   ret = NCR5380_init(host, 0);
+   if (ret)
+   goto out_unmap;
 
NCR5380_maybe_reset_bus(host);
 
@@ -250,7 +252,7 @@ static int cumanascsi1_probe(struct expa
if (ret) {
printk("scsi%d: IRQ%d not free: %d\n",
host->host_no, host->irq, ret);
-   goto out_unmap;
+   goto out_exit;
}
 
ret = scsi_add_host(host, >dev);
@@ -262,6 +264,8 @@ static int cumanascsi1_probe(struct expa
 
  out_free_irq:
free_irq(host->irq, host);
+ out_exit:
+   NCR5380_exit(host);
  out_unmap:
iounmap(priv(host)->base);
iounmap(priv(host)->dma);
Index: linux/drivers/scsi/arm/oak.c
===
---

Re: [PATCH v3 20/77] ncr5380: Introduce unbound workqueue

2015-12-21 Thread Hannes Reinecke


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

Allocate a work queue that will permit busy waiting and sleeping. This
means NCR5380_init() can potentially fail, so add this error path.

Signed-off-by: Finn Thain 

---

In subsequent patches, the work function adopts this work queue so it
can sleep while polling, which allows the removal of some flawed and
complicated code in NCR5380_select() in NCR5380.c.

Changed since v1:
- Dropped WQ_CPU_INTENSIVE flag because Documentation/workqueue.txt says it
   "is meaningless for unbound wq".

---
  drivers/scsi/NCR5380.c   |   15 +++
  drivers/scsi/NCR5380.h   |1 +
  drivers/scsi/arm/cumana_1.c  |8 ++--
  drivers/scsi/arm/oak.c   |8 ++--
  drivers/scsi/atari_NCR5380.c |8 +++-
  drivers/scsi/atari_scsi.c|5 -
  drivers/scsi/dmx3191d.c  |   17 +++--
  drivers/scsi/dtc.c   |   11 +--
  drivers/scsi/g_NCR5380.c |   31 +++
  drivers/scsi/mac_scsi.c  |5 -
  drivers/scsi/pas16.c |   10 --
  drivers/scsi/sun3_scsi.c |5 -
  drivers/scsi/t128.c  |   13 ++---
  13 files changed, 96 insertions(+), 41 deletions(-)

Index: linux/drivers/scsi/NCR5380.c
===
--- linux.orig/drivers/scsi/NCR5380.c   2015-12-22 12:15:52.0 +1100
+++ linux/drivers/scsi/NCR5380.c2015-12-22 12:15:56.0 +1100
@@ -514,7 +514,7 @@ static int should_disconnect(unsigned ch
  static void NCR5380_set_timer(struct NCR5380_hostdata *hostdata, unsigned 
long timeout)
  {
hostdata->time_expires = jiffies + timeout;
-   schedule_delayed_work(>coroutine, timeout);
+   queue_delayed_work(hostdata->work_q, >coroutine, timeout);
  }


@@ -791,7 +791,12 @@ static int NCR5380_init(struct Scsi_Host
hostdata->disconnected_queue = NULL;

INIT_DELAYED_WORK(>coroutine, NCR5380_main);
-   
+   hostdata->work_q = alloc_workqueue("ncr5380_%d",
+   WQ_UNBOUND | WQ_MEM_RECLAIM,
+   1, instance->host_no);
+   if (!hostdata->work_q)
+   return -ENOMEM;
+
/* The CHECK code seems to break the 53C400. Will check it later maybe 
*/
if (flags & FLAG_NCR53C400)
hostdata->flags = FLAG_HAS_LAST_BYTE_SENT | flags;


Wouldn't it be better to use a normal (ie bound) workqueue here?
SCSI-2 is pretty much single-threaded, so shifting things onto arbitrary 
CPUs don't sound very appealing.


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/

Re: [PATCH v3 20/77] ncr5380: Introduce unbound workqueue

Re: [PATCH v3 20/77] ncr5380: Introduce unbound workqueue

Re: [PATCH v3 20/77] ncr5380: Introduce unbound workqueue

Re: [PATCH v3 20/77] ncr5380: Introduce unbound workqueue

Re: [PATCH v3 20/77] ncr5380: Introduce unbound workqueue

[PATCH v3 20/77] ncr5380: Introduce unbound workqueue

[PATCH v3 20/77] ncr5380: Introduce unbound workqueue

Re: [PATCH v3 20/77] ncr5380: Introduce unbound workqueue

8 matches

Site Navigation

Mail list logo

Footer information