On 9/8/26 11:17, Sunil Khatri wrote:
> drm_exec_prepare_array() silently returns success without calling
> drm_exec_lock_contended() when num_objects is zero. This breaks the
> invariant upheld by drm_exec_lock_obj(), where every entry point into
> the locking sequence must first attempt to lock any previously
> contended object before proceeding.
>
> Drivers that chain multiple drm_exec_prepare_array() calls per
> drm_exec_until_all_locked() iteration (e.g. amdgpu's userq signal/wait
> ioctls, which prepare separate read and write BO arrays) can pass an
> empty array for one of the two calls. If contention is hit while
> preparing the non-empty array, exec->contended is set and the loop
> retries; on retry, the empty-array call preceding it is a no-op that
> never clears exec->contended, so drm_exec_retry_on_contention()
> immediately jumps back to the top of the loop without ever reaching
> the call that would resolve the contention. This spins forever.
>
> Fix it by having drm_exec_prepare_array() call drm_exec_lock_contended()
> directly when num_objects is zero, so a pending contended object dont
> loop infinitely.
>
> Fixes: 09593216bff1 ("drm: execution context for GEM buffers v7")
> CC: [email protected]
I've added a # v6.6+ here.
BTW: If you identified the commit you can use "git tag --contains=09593216bff1"
to figure that out.
> Signed-off-by: Sunil Khatri <[email protected]>
Reviewed-by: Christian König <[email protected]>
I'm going to push this to drm-misc-fixes, please sync with Alex to get that
cherry picked into amd-staging-drm-next as well.
Thanks,
Christian.
> ---
> drivers/gpu/drm/drm_exec.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c
> index fa923852fae4..f4503ab82c66 100644
> --- a/drivers/gpu/drm/drm_exec.c
> +++ b/drivers/gpu/drm/drm_exec.c
> @@ -322,6 +322,19 @@ int drm_exec_prepare_array(struct drm_exec *exec,
> {
> int ret;
>
> + /*
> + * Make sure to lock a contended object even when no objects are
> + * given, otherwise drm_exec_retry_on_contention() would loop
> + * forever on patterns like:
> + *
> + * ret = drm_exec_prepare_array(exec, objs, num_objects, ...);
> + * drm_exec_retry_on_contention(exec);
> + *
> + * with num_objects == 0.
> + */
> + if (!num_objects)
> + return drm_exec_lock_contended(exec);
> +
> for (unsigned int i = 0; i < num_objects; ++i) {
> ret = drm_exec_prepare_obj(exec, objects[i], num_fences);
> if (unlikely(ret))
