On Tue, Aug 31, 2021 at 03:35:56PM +0200, Boris Brezillon wrote: > The labels are misleading. Even though they are all prefixed with 'fail_' > the success case also takes that path, and we should definitely not > cleanup the job if it's been queued. While at it, let's rename those > labels so we don't do the same mistake again. > > Fixes: 53516280cc38 ("drm/panfrost: use scheduler dependency tracking") > Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx> Oops sorry about that, I thought I've had a t-b from panfrost already? Anyway this looks good to me: Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 16212b6b202e..077cbbfa506b 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -253,7 +253,7 @@ static int panfrost_ioctl_submit(struct drm_device *dev, void *data, > job = kzalloc(sizeof(*job), GFP_KERNEL); > if (!job) { > ret = -ENOMEM; > - goto fail_out_sync; > + goto out_put_syncout; > } > > kref_init(&job->refcount); > @@ -270,29 +270,30 @@ static int panfrost_ioctl_submit(struct drm_device *dev, void *data, > &job->file_priv->sched_entity[slot], > NULL); > if (ret) > - goto fail_job_put; > + goto out_put_job; > > ret = panfrost_copy_in_sync(dev, file, args, job); > if (ret) > - goto fail_job; > + goto out_cleanup_job; > > ret = panfrost_lookup_bos(dev, file, args, job); > if (ret) > - goto fail_job; > + goto out_cleanup_job; > > ret = panfrost_job_push(job); > if (ret) > - goto fail_job; > + goto out_cleanup_job; > > /* Update the return sync object for the job */ > if (sync_out) > drm_syncobj_replace_fence(sync_out, job->render_done_fence); > > -fail_job: > - drm_sched_job_cleanup(&job->base); > -fail_job_put: > +out_cleanup_job: > + if (ret) > + drm_sched_job_cleanup(&job->base); > +out_put_job: > panfrost_job_put(job); > -fail_out_sync: > +out_put_syncout: > if (sync_out) > drm_syncobj_put(sync_out); > > -- > 2.31.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch