Remove usages of deprecated create_singlethread_workqueue

PATCH Applied Bhaktipriya Shridhar 4 participants 12 messages Aug 14, 2016 – Aug 23, 2016

Subsystems (3)

HID Input drivers Wolfson microelectronics drivers

lore

Share & tools

lore

Bhaktipriya Shridhar

author

2016-08-14 09:27 UTC #0 lore raw

cover Remove usages of deprecated create_singlethread_workqueue

This patch set removes the instances of deprecated create_singlethread_workqueue (scheduled for removal) in drivers/input by making the appropriate conversions.

Bhaktipriya Shridhar (3):
Input: psmouse: Remove deprecated create_singletheread_workqueue
Input: touchscreen: mc13783_ts: Remove deprecated
create_singletheread_workqueue
Input: wm97xx: Remove deprecated create_singletheread_workqueue

 drivers/input/mouse/psmouse-base.c      |  2 +-
 drivers/input/touchscreen/mc13783_ts.c  | 19 ++++---------------
 drivers/input/touchscreen/wm97xx-core.c |  2 +-
 3 files changed, 6 insertions(+), 17 deletions(-)

--
2.1.4

Bhaktipriya Shridhar

author

2016-08-14 05:23 UTC #1 lore raw

PATCH 1/3 Input: psmouse: Remove deprecated create_singletheread_workqueue

alloc_ordered_workqueue replaces the deprecated
create_singlethread_workqueue.

There are multiple work items on the work queue viz &priv->dev3_register_work, &priv->recalib_wq, &psmouse->resync_work, which require execution ordering. Hence, an ordered workqueue has been used.

The workqueue is not being used on a memory reclaim path. Hence, WQ_MEM_RECLAIM has not been set.

Signed-off-by Bhaktipriya Shridhar <[email protected]>

---
 drivers/input/mouse/psmouse-base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/input/mouse/psmouse-base.c +1 -1

index 5784e20..fb4b185 100644
@@ -1916,7 +1916,7 @@ static int __init psmouse_init(void)
 	synaptics_module_init();
 	hgpk_module_init();
 
-	kpsmoused_wq = create_singlethread_workqueue("kpsmoused");
+	kpsmoused_wq = alloc_ordered_workqueue("kpsmoused", 0);
 	if (!kpsmoused_wq) {
 		pr_err("failed to create kpsmoused workqueue\n");
 		return -ENOMEM;

--
2.1.4

Tejun Heo

Acked-by on v1 1/3

2016-08-15 22:53 UTC #2 lore raw

Re: [PATCH 1/3] Input: psmouse: Remove deprecated create_singletheread_workqueue

Quoted 8 lines · Bhaktipriya Shridhar

alloc_ordered_workqueue replaces the deprecated
create_singlethread_workqueue.
There are multiple work items on the work queue viz &priv->dev3_register_work, &priv->recalib_wq, &psmouse->resync_work, which require execution ordering. Hence, an ordered workqueue has been used.

Quoted 4 lines

The workqueue is not being used on a memory reclaim path. Hence, WQ_MEM_RECLAIM has not been set.
Signed-off-by Bhaktipriya Shridhar <[email protected]>

Acked-by Tejun Heo <[email protected]>

Thanks.

Signature · 1 line

tejun

Dmitry Torokhov

maintainer Applied

2016-08-23 20:51 UTC #3 lore raw

Re: [PATCH 1/3] Input: psmouse: Remove deprecated create_singletheread_workqueue

Quoted 9 lines · Tejun Heo

Quoted 8 lines · Bhaktipriya Shridhar
alloc_ordered_workqueue replaces the deprecated
create_singlethread_workqueue.
There are multiple work items on the work queue viz &priv->dev3_register_work, &priv->recalib_wq, &psmouse->resync_work, which require execution ordering. Hence, an ordered workqueue has been used.

Quoted 6 lines

Quoted 4 lines
The workqueue is not being used on a memory reclaim path. Hence, WQ_MEM_RECLAIM has not been set.
Signed-off-by Bhaktipriya Shridhar <[email protected]>
Acked-by Tejun Heo <[email protected]>

Applied, thank you.

Signature · 1 line

Dmitry

Bhaktipriya Shridhar

author

2016-08-14 05:25 UTC #4 lore raw

PATCH 2/3 Input: touchscreen: mc13783_ts: Remove deprecated create_singletheread_workqueue

In reply to Bhaktipriya Shridhar

The workqueue "workqueue" has a single workitem(&priv->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.

Workitem is sync cancelled in mc13783_ts_remove() to ensure that there are no workitems pending when the driver is disconnected.

Signed-off-by Bhaktipriya Shridhar <[email protected]>

---
 drivers/input/touchscreen/mc13783_ts.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

drivers/input/touchscreen/mc13783_ts.c +4 ~~-15~~

index 913e25a..3b19035 100644
@@ -37,7 +37,6 @@ struct mc13783_ts_priv {
 	struct input_dev *idev;
 	struct mc13xxx *mc13xxx;
 	struct delayed_work work;
-	struct workqueue_struct *workq;
 	unsigned int sample[4];
 	struct mc13xxx_ts_platform_data *touch;
 };

@@ -54,7 +53,7 @@ static irqreturn_t mc13783_ts_handler(int irq, void *data)
 	 * be rescheduled for immediate execution here. However the rearm
 	 * delay is HZ / 50 which is acceptable.
 	 */
-	queue_delayed_work(priv->workq, &priv->work, 0);
+	schedule_delayed_work(&priv->work, 0);
 
 	return IRQ_HANDLED;
 }

@@ -106,7 +105,7 @@ static void mc13783_ts_report_sample(struct mc13783_ts_priv *priv)
 
 			dev_dbg(&idev->dev, "report (%d, %d, %d)\n",
 					x1, y1, 0x1000 - cr0);
-			queue_delayed_work(priv->workq, &priv->work, HZ / 50);
+			schedule_delayed_work(&priv->work, HZ / 50);
 		} else
 			dev_dbg(&idev->dev, "report release\n");

@@ -189,14 +188,6 @@ static int __init mc13783_ts_probe(struct platform_device *pdev)
 		goto err_free_mem;
 	}
 
-	/*
-	 * We need separate workqueue because mc13783_adc_do_conversion
-	 * uses keventd and thus would deadlock.
-	 */
-	priv->workq = create_singlethread_workqueue("mc13783_ts");
-	if (!priv->workq)
-		goto err_free_mem;
-
 	idev->name = MC13783_TS_NAME;
 	idev->dev.parent = &pdev->dev;

@@ -215,14 +206,12 @@ static int __init mc13783_ts_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(&pdev->dev,
 			"register input device failed with %d\n", ret);
-		goto err_destroy_wq;
+		goto err_free_mem;
 	}
 
 	platform_set_drvdata(pdev, priv);
 	return 0;
 
-err_destroy_wq:
-	destroy_workqueue(priv->workq);
 err_free_mem:
 	input_free_device(idev);
 	kfree(priv);

@@ -233,7 +222,7 @@ static int mc13783_ts_remove(struct platform_device *pdev)
 {
 	struct mc13783_ts_priv *priv = platform_get_drvdata(pdev);
 
-	destroy_workqueue(priv->workq);
+	cancel_delayed_work_sync(&priv->work);
 	input_unregister_device(priv->idev);
 	kfree(priv);

--
2.1.4

Tejun Heo

Acked-by on v1 2/3

2016-08-15 22:54 UTC #5 lore raw

Re: [PATCH 2/3] Input: touchscreen: mc13783_ts: Remove deprecated create_singletheread_workqueue

Quoted 14 lines · Bhaktipriya Shridhar

The workqueue "workqueue" has a single workitem(&priv->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.

Quoted 4 lines

Workitem is sync cancelled in mc13783_ts_remove() to ensure that there are no workitems pending when the driver is disconnected.
Signed-off-by Bhaktipriya Shridhar <[email protected]>

Acked-by Tejun Heo <[email protected]>

Thanks.

Signature · 1 line

tejun

Dmitry Torokhov

maintainer Applied

2016-08-23 20:53 UTC #6 lore raw

Re: [PATCH 2/3] Input: touchscreen: mc13783_ts: Remove deprecated create_singletheread_workqueue

In reply to Bhaktipriya Shridhar

Quoted 14 lines · Bhaktipriya Shridhar

The workqueue "workqueue" has a single workitem(&priv->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.

Quoted 4 lines

Workitem is sync cancelled in mc13783_ts_remove() to ensure that there are no workitems pending when the driver is disconnected.
Signed-off-by Bhaktipriya Shridhar <[email protected]>

Applied, thank you. But:

drivers/input/touchscreen/mc13783_ts.c static int mc13783_ts_remove(struct platform_device *pdev) · line 225

Quoted 6 lines

drivers/input/touchscreen/mc13783_ts.c +1 -1

@@ -233,7 +222,7 @@ static int mc13783_ts_remove(struct platform_device *pdev)
 {
 	struct mc13783_ts_priv *priv = platform_get_drvdata(pdev);
 
-	destroy_workqueue(priv->workq);
+	cancel_delayed_work_sync(&priv->work);

This is not needed, we cancel work in mc13783_ts_close(). I dropped this bit.

Quoted 6 lines

drivers/input/touchscreen/mc13783_ts.c +0 -1
 	input_unregister_device(priv->idev);
 	kfree(priv);
 
--
2.1.4

Signature · 1 line

Dmitry

Bhaktipriya Shridhar

author

2016-08-14 09:02 UTC #7 lore raw

PATCH 3/3 Input: wm97xx: Remove deprecated create_singletheread_workqueue

In reply to Bhaktipriya Shridhar

alloc_ordered_workqueue() with WQ_MEM_RECLAIM set, replaces deprecated create_singlethread_workqueue(). This is the identity conversion. The workqueue "ts_workq" has been identity converted.

It queues work items viz &wm->ts_reader and &wm->pen_event_work which map to wm97xx_pen_irq_worker (handles a pen down interrupt) and wm97xx_ts_reader (the touchscreen sample reader) respectively. Hence, an ordered dedicated workqueue has been used.

WQ_MEM_RECLAIM has been set to ensure forward progress under memory pressure.

Signed-off-by Bhaktipriya Shridhar <[email protected]>

---
 drivers/input/touchscreen/wm97xx-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/input/touchscreen/wm97xx-core.c +1 -1

index 1534e9b..c0af844 100644
@@ -500,7 +500,7 @@ static int wm97xx_ts_input_open(struct input_dev *idev)
 {
 	struct wm97xx *wm = input_get_drvdata(idev);
 
-	wm->ts_workq = create_singlethread_workqueue("kwm97xx");
+	wm->ts_workq = alloc_ordered_workqueue("kwm97xx", WQ_MEM_RECLAIM);
 	if (wm->ts_workq == NULL) {
 		dev_err(wm->dev,
 			"Failed to create workqueue\n");

--
2.1.4

Charles Keepax

Acked-by on v1 3/3

2016-08-15 08:56 UTC #8 lore raw

Re: [PATCH 3/3] Input: wm97xx: Remove deprecated create_singletheread_workqueue

Quoted 9 lines · Bhaktipriya Shridhar

alloc_ordered_workqueue() with WQ_MEM_RECLAIM set, replaces deprecated create_singlethread_workqueue(). This is the identity conversion. The workqueue "ts_workq" has been identity converted.
It queues work items viz &wm->ts_reader and &wm->pen_event_work which map to wm97xx_pen_irq_worker (handles a pen down interrupt) and wm97xx_ts_reader (the touchscreen sample reader) respectively. Hence, an ordered dedicated workqueue has been used.

Quoted 5 lines

WQ_MEM_RECLAIM has been set to ensure forward progress under memory pressure.
Signed-off-by Bhaktipriya Shridhar <[email protected]>
---

Acked-by Charles Keepax <[email protected]>

Thanks,
Charles

Tejun Heo

2016-08-15 22:53 UTC #9 lore raw

Re: [PATCH 3/3] Input: wm97xx: Remove deprecated create_singletheread_workqueue

In reply to Bhaktipriya Shridhar

Quoted 11 lines · Bhaktipriya Shridhar

alloc_ordered_workqueue() with WQ_MEM_RECLAIM set, replaces deprecated create_singlethread_workqueue(). This is the identity conversion. The workqueue "ts_workq" has been identity converted.
It queues work items viz &wm->ts_reader and &wm->pen_event_work which map to wm97xx_pen_irq_worker (handles a pen down interrupt) and wm97xx_ts_reader (the touchscreen sample reader) respectively. Hence, an ordered dedicated workqueue has been used.
WQ_MEM_RECLAIM has been set to ensure forward progress under memory pressure.

I'm not sure WQ_MEM_RECLAIM makes sense on an input driver.

Thanks.

Signature · 1 line

tejun

Dmitry Torokhov

maintainer Applied

2016-08-23 21:01 UTC #10 lore raw

Re: [PATCH 3/3] Input: wm97xx: Remove deprecated create_singletheread_workqueue

In reply to Bhaktipriya Shridhar

Quoted 9 lines · Bhaktipriya Shridhar

alloc_ordered_workqueue() with WQ_MEM_RECLAIM set, replaces deprecated create_singlethread_workqueue(). This is the identity conversion. The workqueue "ts_workq" has been identity converted.
It queues work items viz &wm->ts_reader and &wm->pen_event_work which map to wm97xx_pen_irq_worker (handles a pen down interrupt) and wm97xx_ts_reader (the touchscreen sample reader) respectively. Hence, an ordered dedicated workqueue has been used.

Quoted 4 lines

WQ_MEM_RECLAIM has been set to ensure forward progress under memory pressure.
Signed-off-by Bhaktipriya Shridhar <[email protected]>

Applied, thank you.

Quoted 20 lines

---
 drivers/input/touchscreen/wm97xx-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/input/touchscreen/wm97xx-core.c +1 -2

index 1534e9b..c0af844 100644
@@ -500,7 +500,7 @@ static int wm97xx_ts_input_open(struct input_dev *idev)
 {
 	struct wm97xx *wm = input_get_drvdata(idev);
 
-	wm->ts_workq = create_singlethread_workqueue("kwm97xx");
+	wm->ts_workq = alloc_ordered_workqueue("kwm97xx", WQ_MEM_RECLAIM);
 	if (wm->ts_workq == NULL) {
 		dev_err(wm->dev,
 			"Failed to create workqueue\n");
--

2.1.4

Signature · 1 line

Dmitry

Dmitry Torokhov

maintainer

2016-08-23 21:02 UTC #11 lore raw

Re: [PATCH 3/3] Input: wm97xx: Remove deprecated create_singletheread_workqueue

Quoted 10 lines · Dmitry Torokhov

Quoted 9 lines · Bhaktipriya Shridhar
alloc_ordered_workqueue() with WQ_MEM_RECLAIM set, replaces deprecated create_singlethread_workqueue(). This is the identity conversion. The workqueue "ts_workq" has been identity converted.
It queues work items viz &wm->ts_reader and &wm->pen_event_work which map to wm97xx_pen_irq_worker (handles a pen down interrupt) and wm97xx_ts_reader (the touchscreen sample reader) respectively. Hence, an ordered dedicated workqueue has been used.

Quoted 6 lines

Quoted 4 lines
WQ_MEM_RECLAIM has been set to ensure forward progress under memory pressure.
Signed-off-by Bhaktipriya Shridhar <[email protected]>
Applied, thank you.

(Sans WQ_MEM_RECLAIM).

Quoted 24 lines

Quoted 21 lines

---
 drivers/input/touchscreen/wm97xx-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/input/touchscreen/wm97xx-core.c +1 -2

index 1534e9b..c0af844 100644
@@ -500,7 +500,7 @@ static int wm97xx_ts_input_open(struct input_dev *idev)
 {
 	struct wm97xx *wm = input_get_drvdata(idev);
 
-	wm->ts_workq = create_singlethread_workqueue("kwm97xx");
+	wm->ts_workq = alloc_ordered_workqueue("kwm97xx", WQ_MEM_RECLAIM);
 	if (wm->ts_workq == NULL) {
 		dev_err(wm->dev,
 			"Failed to create workqueue\n");
--

2.1.4

Signature · 1 line

Dmitry

Signature · 1 line

Dmitry