A queued job is owned by whoever reaches it last: the submit ioctl hands it to the job lists and the job done worker frees it. This means that the ioctl cannot touch vc4_exec_info once job_lock is dropped. That forces everything that touches `exec` to sit inside the critical section, even work that has nothing to do with the job queues.
Give the vc4_exec_info a refcount. The submit path holds one until it is done with the job and the job lists hold another from the moment the job is queued. The teardown happens when the last one goes away. Signed-off-by: Maíra Canal <[email protected]> --- drivers/gpu/drm/vc4/vc4_drv.h | 2 ++ drivers/gpu/drm/vc4/vc4_gem.c | 30 ++++++++++++++++++++++-------- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index b0e82c2dc1ba..d18a0fece93c 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -671,6 +671,8 @@ struct vc4_crtc_state { struct vc4_exec_info { struct vc4_dev *dev; + struct kref refcount; + /* Sequence number for this bin/render job. */ uint64_t seqno; diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c index 0fef9b48d3c6..b56952bb6b97 100644 --- a/drivers/gpu/drm/vc4/vc4_gem.c +++ b/drivers/gpu/drm/vc4/vc4_gem.c @@ -639,6 +639,8 @@ vc4_queue_submit(struct drm_device *dev, struct vc4_exec_info *exec, fence->seqno = exec->seqno; exec->fence = &fence->base; + kref_get(&exec->refcount); + /* The job can complete and drop exec->fence as soon as job_lock is * released, so hold our own reference. */ @@ -719,11 +721,11 @@ vc4_cl_lookup_bos(struct drm_device *dev, fail_dec_usecnt: /* Decrease usecnt on acquired objects. - * We cannot rely on vc4_complete_exec() to release resources here, - * because vc4_complete_exec() has no information about which BO has - * had its ->usecnt incremented. + * We cannot rely on vc4_release_exec() to release resources here, + * because it has no information about which BO has had its ->usecnt + * incremented. * To make things easier we just free everything explicitly and set - * exec->bo to NULL so that vc4_complete_exec() skips the 'BO release' + * exec->bo to NULL so that vc4_release_exec() skips the 'BO release' * step. */ for (i-- ; i >= 0; i--) @@ -854,9 +856,11 @@ vc4_get_bcl(struct drm_device *dev, struct vc4_exec_info *exec) } static void -vc4_complete_exec(struct drm_device *dev, struct vc4_exec_info *exec) +vc4_release_exec(struct kref *ref) { - struct vc4_dev *vc4 = to_vc4_dev(dev); + struct vc4_exec_info *exec = container_of(ref, struct vc4_exec_info, + refcount); + struct vc4_dev *vc4 = exec->dev; unsigned long irqflags; unsigned i; @@ -908,6 +912,12 @@ vc4_complete_exec(struct drm_device *dev, struct vc4_exec_info *exec) kfree(exec); } +static void +vc4_exec_put(struct vc4_exec_info *exec) +{ + kref_put(&exec->refcount, vc4_release_exec); +} + void vc4_job_handle_completed(struct vc4_dev *vc4) { @@ -924,7 +934,7 @@ vc4_job_handle_completed(struct vc4_dev *vc4) list_del(&exec->head); spin_unlock_irqrestore(&vc4->job_lock, irqflags); - vc4_complete_exec(&vc4->base, exec); + vc4_exec_put(exec); spin_lock_irqsave(&vc4->job_lock, irqflags); } @@ -1071,6 +1081,8 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data, return ret; } + kref_init(&exec->refcount); + exec->args = args; INIT_LIST_HEAD(&exec->unref_list); @@ -1160,12 +1172,14 @@ vc4_submit_cl_ioctl(struct drm_device *dev, void *data, /* Return the seqno for our job. */ args->seqno = vc4->emit_seqno; + vc4_exec_put(exec); + return 0; fail_unreserve: drm_exec_fini(&exec_ctx); fail: - vc4_complete_exec(&vc4->base, exec); + vc4_exec_put(exec); return ret; } -- 2.55.0
