Re: [PATCH 2/2] RFC: drm/xe: Prevent pinned_link double add

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe: Masked list corruption causes system suspend failure
--

commit 2ec4b5f1ac44438a30593cce5cd58f19ac02f193
Author: Kornel Dulęba <korneld@google.com>

RFC: drm/xe: Prevent pinned_link double add

The commit attempts to fix a list double-add by checking if the pinned link
is empty and adding a warning. It resolves kernel crashes on memory
constrained devices running CtsCameraTestCases.

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 98c7c17cc46cc..a5af53b36a5f2 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -3166,7 +3166,14 @@ int xe_bo_pin_external(struct xe_bo *bo, bool in_place, struct drm_exec *exec)
>  		}
>  
>  		spin_lock(&xe->pinned.lock);
> -		list_add_tail(&bo->pinned_link, &xe->pinned.late.external);
> +		if (list_empty(&bo->pinned_link)) {
> +			list_add_tail(&bo->pinned_link,
> +				      &xe->pinned.late.external);
> +		} else {
> +			WARN_ONCE(true,
> +				  "pinned_link already added to a list, bo->ttm.pin_count: %d\n",
> +				  bo->ttm.pin_count);
> +		}

[Severity: High]
Does this list_empty() check mask the root cause of a desynchronized
lifecycle when a buffer object is pinned by multiple subsystems, such as
external and display?

If xe_bo_unpin_external() is called while the buffer object is still pinned
elsewhere, the pin_count is greater than 1, and it skips removing the
buffer object from xe->pinned.late.external:

drivers/gpu/drm/xe/xe_bo.c:xe_bo_unpin_external() {
    ...
	if (bo->ttm.pin_count == 1 && !list_empty(&bo->pinned_link))
		list_del_init(&bo->pinned_link);
    ...
}

If a subsequent display unpin happens, it calls ttm_bo_unpin() directly and
drops the pin_count to 0, completely bypassing the external pinned list
removal:

drivers/gpu/drm/xe/display/xe_fb_pin.c:__xe_unpin_fb_vma() {
    ...
	ttm_bo_unpin(&vma->bo->ttm);
    ...
}

This leaves the unpinned buffer object stranded on the pinned list. During
system suspend, power management eviction encounters this stranded unpinned
buffer object, triggering a warning and returning -EINVAL:

drivers/gpu/drm/xe/xe_bo.c:xe_bo_evict_pinned() {
    ...
		if (WARN_ON(!xe_bo_is_pinned(bo))) {
			ret = -EINVAL;
			break;
		}
    ...
}

Will this cause power management eviction to abort and break system suspend?

>  		spin_unlock(&xe->pinned.lock);
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006175741.3582911-1-fnu.vishwanatha@intel.com?part=2