Re: [PATCH] drm/exynos: Remove deprecated create_singlethread_workqueue

[Bhaktipriya Shridhar]

Please ignore this mail.



On Tue, Jun 28, 2016 at 10:48 PM, Bhaktipriya Shridhar
<bhaktipriya96@xxxxxxxxx> wrote:
> The workqueue g2d_workq has only a single workitem(&g2d->runqueue_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.
>
> Occurences of the label err_destroy_workqueue have also been removed
> because with the usage of system_wq, calls to destroy_workqueue() have
> been dropped, which makes the label unnecessary.
>
> Work item has been flushed in g2d_remove() to ensure that nothing is
> pending when the driver disconnects.
>
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@xxxxxxxxx>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index 4935523..defb9d0 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> @@ -224,7 +224,6 @@ struct g2d_data {
>         struct clk                      *gate_clk;
>         void __iomem                    *regs;
>         int                             irq;
> -       struct workqueue_struct         *g2d_workq;
>         struct work_struct              runqueue_work;
>         struct exynos_drm_subdrv        subdrv;
>         bool                            suspended;
> @@ -921,7 +920,7 @@ static irqreturn_t g2d_irq_handler(int irq, void *dev_id)
>         }
>
>         if (pending & G2D_INTP_ACMD_FIN)
> -               queue_work(g2d->g2d_workq, &g2d->runqueue_work);
> +               schedule_work(&g2d->runqueue_work);
>
>         return IRQ_HANDLED;
>  }
> @@ -1380,13 +1379,6 @@ static int g2d_probe(struct platform_device *pdev)
>
>         g2d->dev = dev;
>
> -       g2d->g2d_workq = create_singlethread_workqueue("g2d");
> -       if (!g2d->g2d_workq) {
> -               dev_err(dev, "failed to create workqueue\n");
> -               ret = -EINVAL;
> -               goto err_destroy_slab;
> -       }
> -
>         INIT_WORK(&g2d->runqueue_work, g2d_runqueue_worker);
>         INIT_LIST_HEAD(&g2d->free_cmdlist);
>         INIT_LIST_HEAD(&g2d->runqueue);
> @@ -1398,7 +1390,7 @@ static int g2d_probe(struct platform_device *pdev)
>         if (IS_ERR(g2d->gate_clk)) {
>                 dev_err(dev, "failed to get gate clock\n");
>                 ret = PTR_ERR(g2d->gate_clk);
> -               goto err_destroy_workqueue;
> +               goto err_destroy_slab;
>         }
>
>         pm_runtime_enable(dev);
> @@ -1449,8 +1441,6 @@ static int g2d_probe(struct platform_device *pdev)
>
>  err_put_clk:
>         pm_runtime_disable(dev);
> -err_destroy_workqueue:
> -       destroy_workqueue(g2d->g2d_workq);
>  err_destroy_slab:
>         kmem_cache_destroy(g2d->runqueue_slab);
>         return ret;
> @@ -1471,7 +1461,7 @@ static int g2d_remove(struct platform_device *pdev)
>         pm_runtime_disable(&pdev->dev);
>
>         g2d_fini_cmdlist(g2d);
> -       destroy_workqueue(g2d->g2d_workq);
> +       flush_work(&g2d->runqueue_work);
>         kmem_cache_destroy(g2d->runqueue_slab);
>
>         return 0;
> --
> 2.1.4
>