Skip to contentSkip to conversation

drm/qxl: Remove deprecated create_singlethread_workqueue

PATCH Applied Bhaktipriya Shridhar 3 participants 3 messages Jul 2, 2016 – Jul 12, 2016

Subsystems (3)
DRM drivers DRM misc DRM driver for QXL virtual GPU
lore
Share & tools
lore

Bhaktipriya Shridhar

author
#0 lore raw

PATCH 1/1 drm/qxl: Remove deprecated create_singlethread_workqueue

System workqueues have been able to handle high level of concurrency for a long time now and there's no reason to use dedicated workqueues just to gain concurrency. Since the workqueue in the QXL graphics device driver is involved in freeing and processing the release ring (workitem &qdev->gc_workqxl, maps to gc_work which calls qxl_garbage_collect) and is not being used on a memory reclaim path, dedicated gc_queue has been replaced with the use of system_wq.

Unlike a dedicated per-cpu workqueue created with create_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 guarantees unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference.

flush_work() has been called in qxl_device_fini() to ensure that there are no pending tasks while disconnecting the driver.

Signed-off-by Bhaktipriya Shridhar <[email protected]>
---
 drivers/gpu/drm/qxl/qxl_cmd.c | 2 +-
 drivers/gpu/drm/qxl/qxl_drv.h | 1 -
 drivers/gpu/drm/qxl/qxl_kms.c | 6 +-----
 3 files changed, 2 insertions(+), 7 deletions(-)
drivers/gpu/drm/qxl/qxl_cmd.c +1 -1
index b5d4b41..04270f5 100644
@@ -203,7 +203,7 @@ qxl_push_cursor_ring_release(struct qxl_device *qdev, struct qxl_release *releas
 bool qxl_queue_garbage_collect(struct qxl_device *qdev, bool flush)
 {
 	if (!qxl_check_idle(qdev->release_ring)) {
-		queue_work(qdev->gc_queue, &qdev->gc_work);
+		schedule_work(&qdev->gc_work);
 		if (flush)
 			flush_work(&qdev->gc_work);
 		return true;
drivers/gpu/drm/qxl/qxl_drv.h +0 -1
index 3ad6604..8e633ca 100644
@@ -321,7 +321,6 @@ struct qxl_device {
 	struct qxl_bo *current_release_bo[3];
 	int current_release_bo_offset[3];
 
-	struct workqueue_struct *gc_queue;
 	struct work_struct gc_work;
 
 	struct drm_property *hotplug_mode_update_property;
drivers/gpu/drm/qxl/qxl_kms.c +1 -5
index 2319800..144627c 100644
@@ -258,7 +258,6 @@ static int qxl_device_init(struct qxl_device *qdev,
 		 (unsigned long)qdev->surfaceram_size);
 
 
-	qdev->gc_queue = create_singlethread_workqueue("qxl_gc");
 	INIT_WORK(&qdev->gc_work, qxl_gc_work);
 
 	return 0;

@@ -270,10 +269,7 @@ static void qxl_device_fini(struct qxl_device *qdev)
 		qxl_bo_unref(&qdev->current_release_bo[0]);
 	if (qdev->current_release_bo[1])
 		qxl_bo_unref(&qdev->current_release_bo[1]);
-	flush_workqueue(qdev->gc_queue);
-	destroy_workqueue(qdev->gc_queue);
-	qdev->gc_queue = NULL;
-
+	flush_work(&qdev->gc_work);
 	qxl_ring_free(qdev->command_ring);
 	qxl_ring_free(qdev->cursor_ring);
 	qxl_ring_free(qdev->release_ring);

--
2.1.4

Acked-by on v1 1/1
#1 lore raw
Quoted 15 lines · Bhaktipriya Shridhar

System workqueues have been able to handle high level of concurrency for a long time now and there's no reason to use dedicated workqueues just to gain concurrency. Since the workqueue in the QXL graphics device driver is involved in freeing and processing the release ring (workitem &qdev->gc_workqxl, maps to gc_work which calls qxl_garbage_collect) and is not being used on a memory reclaim path, dedicated gc_queue has been replaced with the use of system_wq.

Unlike a dedicated per-cpu workqueue created with create_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 guarantees unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference.

Quoted 4 lines

flush_work() has been called in qxl_device_fini() to ensure that there are no pending tasks while disconnecting the driver.

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

Thanks.

Signature · 5 lines
tejun
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Applied
#2 lore raw
Quoted 16 lines · Tejun Heo
Quoted 15 lines · Bhaktipriya Shridhar

System workqueues have been able to handle high level of concurrency for a long time now and there's no reason to use dedicated workqueues just to gain concurrency. Since the workqueue in the QXL graphics device driver is involved in freeing and processing the release ring (workitem &qdev->gc_workqxl, maps to gc_work which calls qxl_garbage_collect) and is not being used on a memory reclaim path, dedicated gc_queue has been replaced with the use of system_wq.

Unlike a dedicated per-cpu workqueue created with create_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 guarantees unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference.

Quoted 6 lines
Quoted 4 lines

flush_work() has been called in qxl_device_fini() to ensure that there are no pending tasks while disconnecting the driver.

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

Applied to drm-misc, thanks.
-Daniel

Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel