Re: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Moving `drm_sched_job_cleanup()` into the job's kref release callback creates an unbreakable circular dependency that permanently leaks the job, the hardware fence, and the scheduler fence.
--

--- Patch [1]: [RFC PATCH] drm/sched/tests: Let the DRM scheduler manage job lifetimes ---
commit b37403ca6e558383820ecec4748cf84b025d3af9
Author: Marco Pagani <marco.pagani@linux.dev>

drm/sched/tests: Let the DRM scheduler manage job lifetimes

Currently, the mock scheduler uses KUnit-managed memory for jobs. This ties
the job's memory lifetime to the test suite rather than the DRM scheduler's
callbacks. Update the mock scheduler to let the lifetime of jobs be managed
by the DRM scheduler's asynchronous callbacks instead of KUnit managed memory.

> --- a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
> +++ b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
[ ... ]
> @@ -232,21 +264,26 @@ mock_sched_timedout_job(struct drm_sched_job *sched_job)
>  	}
>  	spin_unlock_irqrestore(&sched->lock, flags);
>  
> -	dma_fence_put(&job->hw_fence);
> -	drm_sched_job_cleanup(sched_job);
> -	/* Mock job itself is freed by the kunit framework. */
> +	mock_sched_free_job(sched_job);
>  
>  	return DRM_GPU_SCHED_STAT_RESET;
>  }
>  
> -static void mock_sched_free_job(struct drm_sched_job *sched_job)
> +static void drm_mock_sched_job_release(struct kref *ref)
>  {
> -	struct drm_mock_sched_job *job = drm_sched_job_to_mock_job(sched_job);
> +	struct drm_mock_sched_job *job;
> +
> +	job = container_of(ref, struct drm_mock_sched_job, refcount);
>  
> -	dma_fence_put(&job->hw_fence);
> -	drm_sched_job_cleanup(sched_job);
> +	drm_sched_job_cleanup(&job->base);

[Severity: Medium]
Does moving drm_sched_job_cleanup() into the kref release function create an
unbreakable circular dependency?

Looking at how the fences and job reference each other:

1. mock_sched_run_job() increments the job's refcount on behalf of the
hw_fence. This reference is only released in drm_mock_sched_hw_fence_release()
when the hw_fence refcount reaches zero.

2. The DRM scheduler stores the hw_fence in job->s_fence->parent, so the
s_fence holds a reference to the hw_fence.

3. The s_fence is freed by drm_sched_job_cleanup().

Because drm_sched_job_cleanup() is now only called when the job's refcount
reaches zero, the s_fence cannot be freed. This means the hw_fence is never
released, which in turn prevents it from dropping its reference to the job,
permanently leaking the job, the hw_fence, and the s_fence.

Prior to this patch, mock_sched_free_job() directly called
drm_sched_job_cleanup(), which broke the chain by freeing the s_fence before
the job was put.

>  
> -	/* Mock job itself is freed by the kunit framework. */
> +	kfree(job);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707114807.154572-1-marco.pagani@linux.dev?part=1