[PATCH] usb: lvstest: Remove deprecated create_singlethread_workqueue

[PATCH] usb: lvstest: Remove deprecated create_singlethread_workqueue

[Bhaktipriya Shridhar]

The workqueue has a single work item(&lvs->rh_work) and hence
doesn't require ordering. Also, it is not being used on a memory
reclaim path. Hence, the singlethreaded workqueue has been replaced
with the use of system_wq.

System workqueues have been able to handle high level of concurrency
for a long time now and hence it's not required to have a singlethreaded
workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue
created with create_singlethread_workqueue(), system_wq allows multiple
work items to overlap executions even on the same CPU; however, a
per-cpu workqueue doesn't have any CPU locality or global ordering
guarantee unless the target CPU is explicitly specified and thus the
increase of local concurrency shouldn't make any difference.

The work item has been flushed in lvs_rh_disconnect() to ensure that
there are no pending tasks while disconnecting the driver.

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
---
 drivers/usb/misc/lvstest.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/misc/lvstest.c b/drivers/usb/misc/lvstest.c
index 86b4e4b..a985cad 100644
--- a/drivers/usb/misc/lvstest.c
+++ b/drivers/usb/misc/lvstest.c
@@ -34,8 +34,6 @@ struct lvs_rh {
 	struct usb_hub_descriptor descriptor;
 	/* urb for polling interrupt pipe */
 	struct urb *urb;
-	/* LVS RH work queue */
-	struct workqueue_struct *rh_queue;
 	/* LVH RH work */
 	struct work_struct	rh_work;
 	/* RH port status */
@@ -355,7 +353,7 @@ static void lvs_rh_irq(struct urb *urb)
 {
 	struct lvs_rh *lvs = urb->context;

-	queue_work(lvs->rh_queue, &lvs->rh_work);
+	schedule_work(&lvs->rh_work);
 }

 static int lvs_rh_probe(struct usb_interface *intf,
@@ -402,19 +400,12 @@ static int lvs_rh_probe(struct usb_interface *intf,
 		return -ENOMEM;
 	}

-	lvs->rh_queue = create_singlethread_workqueue("lvs_rh_queue");
-	if (!lvs->rh_queue) {
-		dev_err(&intf->dev, "couldn't create workqueue\n");
-		ret = -ENOMEM;
-		goto free_urb;
-	}
-
 	INIT_WORK(&lvs->rh_work, lvs_rh_work);

 	ret = sysfs_create_group(&intf->dev.kobj, &lvs_attr_group);
 	if (ret < 0) {
 		dev_err(&intf->dev, "Failed to create sysfs node %d\n", ret);
-		goto destroy_queue;
+		goto free_urb;
 	}

 	pipe = usb_rcvintpipe(hdev, endpoint->bEndpointAddress);
@@ -432,8 +423,6 @@ static int lvs_rh_probe(struct usb_interface *intf,

 sysfs_remove:
 	sysfs_remove_group(&intf->dev.kobj, &lvs_attr_group);
-destroy_queue:
-	destroy_workqueue(lvs->rh_queue);
 free_urb:
 	usb_free_urb(lvs->urb);
 	return ret;
@@ -444,7 +433,7 @@ static void lvs_rh_disconnect(struct usb_interface *intf)
 	struct lvs_rh *lvs = usb_get_intfdata(intf);

 	sysfs_remove_group(&intf->dev.kobj, &lvs_attr_group);
-	destroy_workqueue(lvs->rh_queue);
+	flush_work(&lvs->rh_work);
 	usb_free_urb(lvs->urb);
 }

--
2.1.4

Re: [PATCH] usb: lvstest: Remove deprecated create_singlethread_workqueue

[Tejun Heo]

On Thu, Jul 28, 2016 at 02:15:11PM +0530, Bhaktipriya Shridhar wrote:
> The workqueue has a single work item(&lvs->rh_work) and hence
> doesn't require ordering. Also, it is not being used on a memory
> reclaim path. Hence, the singlethreaded workqueue has been replaced
> with the use of system_wq.
> 
> System workqueues have been able to handle high level of concurrency
> for a long time now and hence it's not required to have a singlethreaded
> workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue
> created with create_singlethread_workqueue(), system_wq allows multiple
> work items to overlap executions even on the same CPU; however, a
> per-cpu workqueue doesn't have any CPU locality or global ordering
> guarantee unless the target CPU is explicitly specified and thus the
> increase of local concurrency shouldn't make any difference.
> 
> The work item has been flushed in lvs_rh_disconnect() to ensure that
> there are no pending tasks while disconnecting the driver.
> 
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun