[PATCH v2.6.36-rc7 1/2] pciehp: update workqueue usage

[PATCH v2.6.36-rc7 1/2] pciehp: update workqueue usage

[Tejun Heo]

* Rename pciehp_wq to pciehp_ordered_wq and add non-ordered pciehp_wq
  which is used instead of the system workqueue.  This is to remove
  the use of flush_scheduled_work() which is deprecated and scheduled
  for removal.

* With cmwq in place, there's no point in creating workqueues lazily.
  Create both pciehp_wq and pciehp_ordered_wq upfront.

* Include workqueue.h from pciehp.h.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
Both patches are only compile tested.  They should be safe but please
proceed with caution.

Thank you.

 drivers/pci/hotplug/pciehp.h      |    2 ++
 drivers/pci/hotplug/pciehp_core.c |   18 +++++++++++++++++-
 drivers/pci/hotplug/pciehp_ctrl.c |    9 ++++-----
 drivers/pci/hotplug/pciehp_hpc.c  |   20 +-------------------
 4 files changed, 24 insertions(+), 25 deletions(-)

Index: work/drivers/pci/hotplug/pciehp.h
===================================================================
--- work.orig/drivers/pci/hotplug/pciehp.h
+++ work/drivers/pci/hotplug/pciehp.h
@@ -36,6 +36,7 @@
 #include <linux/sched.h>		/* signal_pending() */
 #include <linux/pcieport_if.h>
 #include <linux/mutex.h>
+#include <linux/workqueue.h>

 #define MY_NAME	"pciehp"

@@ -44,6 +45,7 @@ extern int pciehp_poll_time;
 extern int pciehp_debug;
 extern int pciehp_force;
 extern struct workqueue_struct *pciehp_wq;
+extern struct workqueue_struct *pciehp_ordered_wq;

 #define dbg(format, arg...)						\
 do {									\
Index: work/drivers/pci/hotplug/pciehp_core.c
===================================================================
--- work.orig/drivers/pci/hotplug/pciehp_core.c
+++ work/drivers/pci/hotplug/pciehp_core.c
@@ -43,6 +43,7 @@ int pciehp_poll_mode;
 int pciehp_poll_time;
 int pciehp_force;
 struct workqueue_struct *pciehp_wq;
+struct workqueue_struct *pciehp_ordered_wq;

 #define DRIVER_VERSION	"0.4"
 #define DRIVER_AUTHOR	"Dan Zink <dan.zink@compaq.com>, Greg Kroah-Hartman <greg@kroah.com>, Dely Sy <dely.l.sy@intel.com>"
@@ -340,18 +341,33 @@ static int __init pcied_init(void)
 {
 	int retval = 0;

+	pciehp_wq = alloc_workqueue("pciehp", 0, 0);
+	if (!pciehp_wq)
+		return -ENOMEM;
+
+	pciehp_ordered_wq = alloc_ordered_workqueue("pciehp_ordered", 0);
+	if (!pciehp_ordered_wq) {
+		destroy_workqueue(pciehp_wq);
+		return -ENOMEM;
+	}
+
 	pciehp_firmware_init();
 	retval = pcie_port_service_register(&hpdriver_portdrv);
  	dbg("pcie_port_service_register = %d\n", retval);
   	info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
- 	if (retval)
+ 	if (retval) {
+		destroy_workqueue(pciehp_ordered_wq);
+		destroy_workqueue(pciehp_wq);
 		dbg("Failure to register service\n");
+	}
 	return retval;
 }

 static void __exit pcied_cleanup(void)
 {
 	dbg("unload_pciehpd()\n");
+	destroy_workqueue(pciehp_ordered_wq);
+	destroy_workqueue(pciehp_wq);
 	pcie_port_service_unregister(&hpdriver_portdrv);
 	info(DRIVER_DESC " version: " DRIVER_VERSION " unloaded\n");
 }
Index: work/drivers/pci/hotplug/pciehp_ctrl.c
===================================================================
--- work.orig/drivers/pci/hotplug/pciehp_ctrl.c
+++ work/drivers/pci/hotplug/pciehp_ctrl.c
@@ -32,7 +32,6 @@
 #include <linux/types.h>
 #include <linux/slab.h>
 #include <linux/pci.h>
-#include <linux/workqueue.h>
 #include "../pci.h"
 #include "pciehp.h"

@@ -50,7 +49,7 @@ static int queue_interrupt_event(struct
 	info->p_slot = p_slot;
 	INIT_WORK(&info->work, interrupt_event_handler);

-	schedule_work(&info->work);
+	queue_work(pciehp_wq, &info->work);

 	return 0;
 }
@@ -345,7 +344,7 @@ void pciehp_queue_pushbutton_work(struct
 		kfree(info);
 		goto out;
 	}
-	queue_work(pciehp_wq, &info->work);
+	queue_work(pciehp_ordered_wq, &info->work);
  out:
 	mutex_unlock(&p_slot->lock);
 }
@@ -378,7 +377,7 @@ static void handle_button_press_event(st
 		if (ATTN_LED(ctrl))
 			pciehp_set_attention_status(p_slot, 0);

-		schedule_delayed_work(&p_slot->work, 5*HZ);
+		queue_delayed_work(pciehp_wq, &p_slot->work, 5*HZ);
 		break;
 	case BLINKINGOFF_STATE:
 	case BLINKINGON_STATE:
@@ -440,7 +439,7 @@ static void handle_surprise_event(struct
 	else
 		p_slot->state = POWERON_STATE;

-	queue_work(pciehp_wq, &info->work);
+	queue_work(pciehp_ordered_wq, &info->work);
 }

 static void interrupt_event_handler(struct work_struct *work)
Index: work/drivers/pci/hotplug/pciehp_hpc.c
===================================================================
--- work.orig/drivers/pci/hotplug/pciehp_hpc.c
+++ work/drivers/pci/hotplug/pciehp_hpc.c
@@ -41,8 +41,6 @@
 #include "../pci.h"
 #include "pciehp.h"

-static atomic_t pciehp_num_controllers = ATOMIC_INIT(0);
-
 static inline int pciehp_readw(struct controller *ctrl, int reg, u16 *value)
 {
 	struct pci_dev *dev = ctrl->pcie->port;
@@ -805,8 +803,8 @@ static void pcie_cleanup_slot(struct con
 {
 	struct slot *slot = ctrl->slot;
 	cancel_delayed_work(&slot->work);
-	flush_scheduled_work();
 	flush_workqueue(pciehp_wq);
+	flush_workqueue(pciehp_ordered_wq);
 	kfree(slot);
 }

@@ -912,16 +910,6 @@ struct controller *pcie_init(struct pcie
 	/* Disable sotfware notification */
 	pcie_disable_notification(ctrl);

-	/*
-	 * If this is the first controller to be initialized,
-	 * initialize the pciehp work queue
-	 */
-	if (atomic_add_return(1, &pciehp_num_controllers) == 1) {
-		pciehp_wq = create_singlethread_workqueue("pciehpd");
-		if (!pciehp_wq)
-			goto abort_ctrl;
-	}
-
 	ctrl_info(ctrl, "HPC vendor_id %x device_id %x ss_vid %x ss_did %x\n",
 		  pdev->vendor, pdev->device, pdev->subsystem_vendor,
 		  pdev->subsystem_device);
@@ -941,11 +929,5 @@ void pciehp_release_ctrl(struct controll
 {
 	pcie_shutdown_notification(ctrl);
 	pcie_cleanup_slot(ctrl);
-	/*
-	 * If this is the last controller to be released, destroy the
-	 * pciehp work queue
-	 */
-	if (atomic_dec_and_test(&pciehp_num_controllers))
-		destroy_workqueue(pciehp_wq);
 	kfree(ctrl);
 }

[PATCH v2.6.36-rc7 2/2] shpchp: update workqueue usage

[Tejun Heo]

* Rename shpchp_wq to shpchp_ordered_wq and add non-ordered shpchp_wq
  which is used instead of the system workqueue.  This is to remove
  the use of flush_scheduled_work() which is deprecated and scheduled
  for removal.

* With cmwq in place, there's no point in creating workqueues lazily.
  Create both shpchp_wq and shpchp_ordered_wq upfront.

* Include workqueue.h from shpchp.h.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/pci/hotplug/shpchp.h      |    2 ++
 drivers/pci/hotplug/shpchp_core.c |   20 ++++++++++++++++++--
 drivers/pci/hotplug/shpchp_ctrl.c |    7 +++----
 drivers/pci/hotplug/shpchp_hpc.c  |   26 ++------------------------
 4 files changed, 25 insertions(+), 30 deletions(-)

Index: work/drivers/pci/hotplug/shpchp_core.c
===================================================================
--- work.orig/drivers/pci/hotplug/shpchp_core.c
+++ work/drivers/pci/hotplug/shpchp_core.c
@@ -33,7 +33,6 @@
 #include <linux/types.h>
 #include <linux/slab.h>
 #include <linux/pci.h>
-#include <linux/workqueue.h>
 #include "shpchp.h"

 /* Global variables */
@@ -41,6 +40,7 @@ int shpchp_debug;
 int shpchp_poll_mode;
 int shpchp_poll_time;
 struct workqueue_struct *shpchp_wq;
+struct workqueue_struct *shpchp_ordered_wq;

 #define DRIVER_VERSION	"0.4"
 #define DRIVER_AUTHOR	"Dan Zink <dan.zink@compaq.com>, Greg Kroah-Hartman <greg@kroah.com>, Dely Sy <dely.l.sy@intel.com>"
@@ -174,8 +174,8 @@ void cleanup_slots(struct controller *ct
 		slot = list_entry(tmp, struct slot, slot_list);
 		list_del(&slot->slot_list);
 		cancel_delayed_work(&slot->work);
-		flush_scheduled_work();
 		flush_workqueue(shpchp_wq);
+		flush_workqueue(shpchp_ordered_wq);
 		pci_hp_deregister(slot->hotplug_slot);
 	}
 }
@@ -360,9 +360,23 @@ static int __init shpcd_init(void)
 {
 	int retval = 0;

+	shpchp_wq = alloc_ordered_workqueue("shpchp", 0);
+	if (!shpchp_wq)
+		return -ENOMEM;
+
+	shpchp_ordered_wq = alloc_ordered_workqueue("shpchp_ordered", 0);
+	if (!shpchp_ordered_wq) {
+		destroy_workqueue(shpchp_wq);
+		return -ENOMEM;
+	}
+
 	retval = pci_register_driver(&shpc_driver);
 	dbg("%s: pci_register_driver = %d\n", __func__, retval);
 	info(DRIVER_DESC " version: " DRIVER_VERSION "\n");
+	if (retval) {
+		destroy_workqueue(shpchp_ordered_wq);
+		destroy_workqueue(shpchp_wq);
+	}
 	return retval;
 }

@@ -370,6 +384,8 @@ static void __exit shpcd_cleanup(void)
 {
 	dbg("unload_shpchpd()\n");
 	pci_unregister_driver(&shpc_driver);
+	destroy_workqueue(shpchp_ordered_wq);
+	destroy_workqueue(shpchp_wq);
 	info(DRIVER_DESC " version: " DRIVER_VERSION " unloaded\n");
 }

Index: work/drivers/pci/hotplug/shpchp_ctrl.c
===================================================================
--- work.orig/drivers/pci/hotplug/shpchp_ctrl.c
+++ work/drivers/pci/hotplug/shpchp_ctrl.c
@@ -32,7 +32,6 @@
 #include <linux/types.h>
 #include <linux/slab.h>
 #include <linux/pci.h>
-#include <linux/workqueue.h>
 #include "../pci.h"
 #include "shpchp.h"

@@ -52,7 +51,7 @@ static int queue_interrupt_event(struct
 	info->p_slot = p_slot;
 	INIT_WORK(&info->work, interrupt_event_handler);

-	schedule_work(&info->work);
+	queue_work(shpchp_wq, &info->work);

 	return 0;
 }
@@ -457,7 +456,7 @@ void shpchp_queue_pushbutton_work(struct
 		kfree(info);
 		goto out;
 	}
-	queue_work(shpchp_wq, &info->work);
+	queue_work(shpchp_ordered_wq, &info->work);
  out:
 	mutex_unlock(&p_slot->lock);
 }
@@ -505,7 +504,7 @@ static void handle_button_press_event(st
 		p_slot->hpc_ops->green_led_blink(p_slot);
 		p_slot->hpc_ops->set_attention_status(p_slot, 0);

-		schedule_delayed_work(&p_slot->work, 5*HZ);
+		queue_delayed_work(shpchp_wq, &p_slot->work, 5*HZ);
 		break;
 	case BLINKINGOFF_STATE:
 	case BLINKINGON_STATE:
Index: work/drivers/pci/hotplug/shpchp_hpc.c
===================================================================
--- work.orig/drivers/pci/hotplug/shpchp_hpc.c
+++ work/drivers/pci/hotplug/shpchp_hpc.c
@@ -179,8 +179,6 @@
 #define SLOT_EVENT_LATCH	0x2
 #define SLOT_SERR_INT_MASK	0x3

-static atomic_t shpchp_num_controllers = ATOMIC_INIT(0);
-
 static irqreturn_t shpc_isr(int irq, void *dev_id);
 static void start_int_poll_timer(struct controller *ctrl, int sec);
 static int hpc_check_cmd_status(struct controller *ctrl);
@@ -614,13 +612,6 @@ static void hpc_release_ctlr(struct cont

 	iounmap(ctrl->creg);
 	release_mem_region(ctrl->mmio_base, ctrl->mmio_size);
-
-	/*
-	 * If this is the last controller to be released, destroy the
-	 * shpchpd work queue
-	 */
-	if (atomic_dec_and_test(&shpchp_num_controllers))
-		destroy_workqueue(shpchp_wq);
 }

 static int hpc_power_on_slot(struct slot * slot)
@@ -1077,9 +1068,8 @@ int shpc_init(struct controller *ctrl, s

 		rc = request_irq(ctrl->pci_dev->irq, shpc_isr, IRQF_SHARED,
 				 MY_NAME, (void *)ctrl);
-		ctrl_dbg(ctrl, "request_irq %d for hpc%d (returns %d)\n",
-			 ctrl->pci_dev->irq,
-		    atomic_read(&shpchp_num_controllers), rc);
+		ctrl_dbg(ctrl, "request_irq %d (returns %d)\n",
+			 ctrl->pci_dev->irq, rc);
 		if (rc) {
 			ctrl_err(ctrl, "Can't get irq %d for the hotplug "
 				 "controller\n", ctrl->pci_dev->irq);
@@ -1092,18 +1082,6 @@ int shpc_init(struct controller *ctrl, s
 	shpc_get_cur_bus_speed(ctrl);

 	/*
-	 * If this is the first controller to be initialized,
-	 * initialize the shpchpd work queue
-	 */
-	if (atomic_add_return(1, &shpchp_num_controllers) == 1) {
-		shpchp_wq = create_singlethread_workqueue("shpchpd");
-		if (!shpchp_wq) {
-			rc = -ENOMEM;
-			goto abort_iounmap;
-		}
-	}
-
-	/*
 	 * Unmask all event interrupts of all slots
 	 */
 	for (hp_slot = 0; hp_slot < ctrl->num_slots; hp_slot++) {
Index: work/drivers/pci/hotplug/shpchp.h
===================================================================
--- work.orig/drivers/pci/hotplug/shpchp.h
+++ work/drivers/pci/hotplug/shpchp.h
@@ -35,6 +35,7 @@
 #include <linux/delay.h>
 #include <linux/sched.h>	/* signal_pending(), struct timer_list */
 #include <linux/mutex.h>
+#include <linux/workqueue.h>

 #if !defined(MODULE)
 	#define MY_NAME	"shpchp"
@@ -46,6 +47,7 @@ extern int shpchp_poll_mode;
 extern int shpchp_poll_time;
 extern int shpchp_debug;
 extern struct workqueue_struct *shpchp_wq;
+extern struct workqueue_struct *shpchp_ordered_wq;

 #define dbg(format, arg...)						\
 do {									\

Re: [PATCH v2.6.36-rc7 2/2] shpchp: update workqueue usage

[Jesse Barnes]

On Fri, 15 Oct 2010 16:47:38 +0200
Tejun Heo <tj@kernel.org> wrote:

> * Rename shpchp_wq to shpchp_ordered_wq and add non-ordered shpchp_wq
>   which is used instead of the system workqueue.  This is to remove
>   the use of flush_scheduled_work() which is deprecated and scheduled
>   for removal.
> 
> * With cmwq in place, there's no point in creating workqueues lazily.
>   Create both shpchp_wq and shpchp_ordered_wq upfront.
> 
> * Include workqueue.h from shpchp.h.

Applied these two, thanks.  I think Kenji-san will probably let me know
if pciehp suddenly breaks as a result. :)

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH v2.6.36-rc7 2/2] shpchp: update workqueue usage

[Tejun Heo]

Hello,

On 10/15/2010 09:52 PM, Jesse Barnes wrote:
>> * Rename shpchp_wq to shpchp_ordered_wq and add non-ordered shpchp_wq
>>   which is used instead of the system workqueue.  This is to remove
>>   the use of flush_scheduled_work() which is deprecated and scheduled
>>   for removal.
>>
>> * With cmwq in place, there's no point in creating workqueues lazily.
>>   Create both shpchp_wq and shpchp_ordered_wq upfront.
>>
>> * Include workqueue.h from shpchp.h.
> 
> Applied these two, thanks.  I think Kenji-san will probably let me know
> if pciehp suddenly breaks as a result. :)

Just in case, these are in no way aimed for v2.6.36.  They are at the
earliest for v2.6.37-rc1.  Thanks a lot.

-- 
tejun

Re: [PATCH v2.6.36-rc7 2/2] shpchp: update workqueue usage

[Jesse Barnes]

On Fri, 15 Oct 2010 22:08:24 +0200
Tejun Heo <tj@kernel.org> wrote:

> Hello,
> 
> On 10/15/2010 09:52 PM, Jesse Barnes wrote:
> >> * Rename shpchp_wq to shpchp_ordered_wq and add non-ordered shpchp_wq
> >>   which is used instead of the system workqueue.  This is to remove
> >>   the use of flush_scheduled_work() which is deprecated and scheduled
> >>   for removal.
> >>
> >> * With cmwq in place, there's no point in creating workqueues lazily.
> >>   Create both shpchp_wq and shpchp_ordered_wq upfront.
> >>
> >> * Include workqueue.h from shpchp.h.
> > 
> > Applied these two, thanks.  I think Kenji-san will probably let me know
> > if pciehp suddenly breaks as a result. :)
> 
> Just in case, these are in no way aimed for v2.6.36.  They are at the
> earliest for v2.6.37-rc1.  Thanks a lot.

Of course, everything I've applied today has been for 2.6.37; 2.6.36 is
well closed now, there are no critical missing PCI fixes that I'm aware
of.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH v2.6.36-rc7 2/2] shpchp: update workqueue usage

[Kenji Kaneshige]

(2010/10/16 4:52), Jesse Barnes wrote:
> On Fri, 15 Oct 2010 16:47:38 +0200
> Tejun Heo<tj@kernel.org>  wrote:
>
>> * Rename shpchp_wq to shpchp_ordered_wq and add non-ordered shpchp_wq
>>    which is used instead of the system workqueue.  This is to remove
>>    the use of flush_scheduled_work() which is deprecated and scheduled
>>    for removal.
>>
>> * With cmwq in place, there's no point in creating workqueues lazily.
>>    Create both shpchp_wq and shpchp_ordered_wq upfront.
>>
>> * Include workqueue.h from shpchp.h.
>
> Applied these two, thanks.  I think Kenji-san will probably let me know
> if pciehp suddenly breaks as a result. :)
>

Thank you for the patch. I'll review and test it.
Yeah, I'll let you know if I find something, Jesse.

Regards,
Kenji Kaneshige

Re: [PATCH v2.6.36-rc7 2/2] shpchp: update workqueue usage

[Jesse Barnes]

On Fri, 15 Oct 2010 16:47:38 +0200
Tejun Heo <tj@kernel.org> wrote:

> * Rename shpchp_wq to shpchp_ordered_wq and add non-ordered shpchp_wq
>   which is used instead of the system workqueue.  This is to remove
>   the use of flush_scheduled_work() which is deprecated and scheduled
>   for removal.
> 
> * With cmwq in place, there's no point in creating workqueues lazily.
>   Create both shpchp_wq and shpchp_ordered_wq upfront.
> 
> * Include workqueue.h from shpchp.h.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---

...and dropped again.  Hadn't realized they depend on stuff from your
workqueue tree.  Please just include them there assuming Kenji-san
doesn't nack them.

Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH v2.6.36-rc7 2/2] shpchp: update workqueue usage

[Tejun Heo]

Hello,

On 10/18/2010 05:03 AM, Jesse Barnes wrote:
> ...and dropped again.  Hadn't realized they depend on stuff from your
> workqueue tree.  Please just include them there assuming Kenji-san
> doesn't nack them.
> 
> Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Ah, sorry about that.  I for some reason thought alloc_ordered_queue()
was already mainline.  I'll route this through the wq tree.

Thank you.

-- 
tejun

Re: [PATCH v2.6.36-rc7 2/2] shpchp: update workqueue usage

[Tejun Heo]

On 10/18/2010 07:02 AM, Tejun Heo wrote:
> Hello,
> 
> On 10/18/2010 05:03 AM, Jesse Barnes wrote:
>> ...and dropped again.  Hadn't realized they depend on stuff from your
>> workqueue tree.  Please just include them there assuming Kenji-san
>> doesn't nack them.
>>
>> Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> Ah, sorry about that.  I for some reason thought alloc_ordered_queue()
> was already mainline.  I'll route this through the wq tree.

Both patches applied to wq#for-next.  Thank you.

-- 
tejun