xen: xenbus: Remove create_workqueue

PATCH In review Bhaktipriya Shridhar 2 participants 5 messages May 27, 2016 – May 31, 2016 Xen hypervisor interface

Share & tools

Bhaktipriya Shridhar

author

2016-05-27 18:50 UTC #0 lore raw

PATCH 1/1 xen: xenbus: Remove create_workqueue

With concurrency managed workqueues, use of dedicated workqueues can be replaced by using system_wq. Drop xenbus_frontend_wq by using system_wq.

Since there is only a single work item, increase of concurrency level by switching to system_wq should not break anything.

Since the work item could be pending and the code expects it to run once scheduled, flush_work() has been used in xenbus_dev_suspend()

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

---
 drivers/xen/xenbus/xenbus_probe.c          |  2 ++
 drivers/xen/xenbus/xenbus_probe_frontend.c | 15 +--------------
 2 files changed, 3 insertions(+), 14 deletions(-)

drivers/xen/xenbus/xenbus_probe.c +2 -0

index 33a31cf..bc97019 100644
@@ -592,6 +592,8 @@ int xenbus_dev_suspend(struct device *dev)
 
 	DPRINTK("%s", xdev->nodename);
 
+	cancel_work_sync(&xdev->work);
+
 	if (dev->driver == NULL)
 		return 0;
 	drv = to_xenbus_driver(dev->driver);

drivers/xen/xenbus/xenbus_probe_frontend.c +1 ~~-14~~

index bcb53bd..611a231 100644
@@ -31,7 +31,6 @@
 #include "xenbus_probe.h"
 
 
-static struct workqueue_struct *xenbus_frontend_wq;
 
 /* device/<type>/<id> => <type>-<id> */
 static int frontend_bus_id(char bus_id[XEN_BUS_ID_SIZE], const char *nodename)

@@ -109,13 +108,7 @@ static int xenbus_frontend_dev_resume(struct device *dev)
 	if (xen_store_domain_type == XS_LOCAL) {
 		struct xenbus_device *xdev = to_xenbus_device(dev);
 
-		if (!xenbus_frontend_wq) {
-			pr_err("%s: no workqueue to process delayed resume\n",
-			       xdev->nodename);
-			return -EFAULT;
-		}
-
-		queue_work(xenbus_frontend_wq, &xdev->work);
+		schedule_work(&xdev->work);
 
 		return 0;
 	}

@@ -485,12 +478,6 @@ static int __init xenbus_probe_frontend_init(void)
 
 	register_xenstore_notifier(&xenstore_notifier);
 
-	if (xen_store_domain_type == XS_LOCAL) {
-		xenbus_frontend_wq = create_workqueue("xenbus_frontend");
-		if (!xenbus_frontend_wq)
-			pr_warn("create xenbus frontend workqueue failed, S3 resume is likely to fail\n");
-	}
-
 	return 0;
 }
 subsys_initcall(xenbus_probe_frontend_init);

--
2.1.4

David Vrabel

2016-05-31 16:18 UTC #1 lore raw

Quoted 8 lines · Bhaktipriya Shridhar

With concurrency managed workqueues, use of dedicated workqueues can be replaced by using system_wq. Drop xenbus_frontend_wq by using system_wq.
Since there is only a single work item, increase of concurrency level by switching to system_wq should not break anything.
Since the work item could be pending and the code expects it to run once scheduled, flush_work() has been used in xenbus_dev_suspend()

This says flush_work() but...

Quoted 12 lines

Signed-off-by Bhaktipriya Shridhar <[email protected]>
---
 drivers/xen/xenbus/xenbus_probe.c          |  2 ++
 drivers/xen/xenbus/xenbus_probe_frontend.c | 15 +--------------
 2 files changed, 3 insertions(+), 14 deletions(-)
drivers/xen/xenbus/xenbus_probe.c +0 -0
index 33a31cf..bc97019 100644

drivers/xen/xenbus/xenbus_probe.c int xenbus_dev_suspend(struct device *dev) · line 595

Quoted 4 lines

drivers/xen/xenbus/xenbus_probe.c +1 -0

@@ -592,6 +592,8 @@ int xenbus_dev_suspend(struct device *dev)
 
 	DPRINTK("%s", xdev->nodename);
 
+	cancel_work_sync(&xdev->work);

...cancel_work_sync() is called here.

David

Bhaktipriya Shridhar

author

2016-05-31 16:44 UTC #2 lore raw

Sorry about that. Will make the corrections in v2.

Thanks,
Bhaktipriya

Quoted 31 lines · David Vrabel

Quoted 8 lines · Bhaktipriya Shridhar
With concurrency managed workqueues, use of dedicated workqueues can be replaced by using system_wq. Drop xenbus_frontend_wq by using system_wq.
Since there is only a single work item, increase of concurrency level by switching to system_wq should not break anything.
Since the work item could be pending and the code expects it to run once scheduled, flush_work() has been used in xenbus_dev_suspend()
This says flush_work() but...
Quoted 12 lines
Signed-off-by Bhaktipriya Shridhar <[email protected]>
---
 drivers/xen/xenbus/xenbus_probe.c          |  2 ++
 drivers/xen/xenbus/xenbus_probe_frontend.c | 15 +--------------
 2 files changed, 3 insertions(+), 14 deletions(-)
drivers/xen/xenbus/xenbus_probe.c +0 -0
index 33a31cf..bc97019 100644
drivers/xen/xenbus/xenbus_probe.c int xenbus_dev_suspend(struct device *dev) · line 595
Quoted 4 lines
drivers/xen/xenbus/xenbus_probe.c +1 -0
@@ -592,6 +592,8 @@ int xenbus_dev_suspend(struct device *dev)
 
      DPRINTK("%s", xdev->nodename);
 
+     cancel_work_sync(&xdev->work);
...cancel_work_sync() is called here.
David

Bhaktipriya Shridhar

author

2016-05-31 16:44 UTC #3 lore raw

In reply to David Vrabel

Sorry about that. Will make the corrections in v2.

Thanks,
Bhaktipriya

Quoted 24 lines · David Vrabel

Quoted 8 lines · Bhaktipriya Shridhar
With concurrency managed workqueues, use of dedicated workqueues can be replaced by using system_wq. Drop xenbus_frontend_wq by using system_wq.
Since there is only a single work item, increase of concurrency level by switching to system_wq should not break anything.
Since the work item could be pending and the code expects it to run once scheduled, flush_work() has been used in xenbus_dev_suspend()
This says flush_work() but...
Quoted 13 lines
Signed-off-by Bhaktipriya Shridhar <[email protected]>
---
 drivers/xen/xenbus/xenbus_probe.c          |  2 ++
 drivers/xen/xenbus/xenbus_probe_frontend.c | 15 +--------------
 2 files changed, 3 insertions(+), 14 deletions(-)
drivers/xen/xenbus/xenbus_probe.c +0 -0
index 33a31cf..bc97019 100644
@@ -592,6 +592,8 @@ int xenbus_dev_suspend(struct device *dev)
 

Quoted 7 lines

drivers/xen/xenbus/xenbus_probe.c int xenbus_dev_suspend(struct device *dev) · line 595
Quoted 3 lines
drivers/xen/xenbus/xenbus_probe.c +1 -0
      DPRINTK("%s", xdev->nodename);
 
+     cancel_work_sync(&xdev->work);
...cancel_work_sync() is called here.
David

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

David Vrabel

2016-05-31 16:18 UTC #4 lore raw

In reply to Bhaktipriya Shridhar

Quoted 8 lines · Bhaktipriya Shridhar

With concurrency managed workqueues, use of dedicated workqueues can be replaced by using system_wq. Drop xenbus_frontend_wq by using system_wq.
Since there is only a single work item, increase of concurrency level by switching to system_wq should not break anything.
Since the work item could be pending and the code expects it to run once scheduled, flush_work() has been used in xenbus_dev_suspend()

This says flush_work() but...

Quoted 12 lines

Signed-off-by Bhaktipriya Shridhar <[email protected]>
---
 drivers/xen/xenbus/xenbus_probe.c          |  2 ++
 drivers/xen/xenbus/xenbus_probe_frontend.c | 15 +--------------
 2 files changed, 3 insertions(+), 14 deletions(-)
drivers/xen/xenbus/xenbus_probe.c +0 -0
index 33a31cf..bc97019 100644

drivers/xen/xenbus/xenbus_probe.c int xenbus_dev_suspend(struct device *dev) · line 595

Quoted 4 lines

drivers/xen/xenbus/xenbus_probe.c +1 -0

@@ -592,6 +592,8 @@ int xenbus_dev_suspend(struct device *dev)
 
 	DPRINTK("%s", xdev->nodename);
 
+	cancel_work_sync(&xdev->work);

...cancel_work_sync() is called here.

David

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel