Re: [PATCH v2 06/14] drm/ttm: Open-code reservation list walk

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing lock release when `dma_resv_reserve_fences()` fails, leaving the current buffer object permanently locked.
--

commit 4a941c2e22e18c4c60dbeb3162fc85117e45e7a8
Author: Kaitao Cheng <chengkaitao@kylinos.cn>

drm/ttm: Open-code reservation list walk

This patch modifies ttm_eu_reserve_buffers() to open-code the
list_for_each_entry() loop. This prepares the code for future list iterator
updates while preserving the existing traversal semantics where the cursor
can be rewound during duplicate handling.

> diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> index bc7a83a9fe443..8072f07d55579 100644
> --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> @@ -86,7 +86,9 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
>  	if (ticket)
>  		ww_acquire_init(ticket, &reservation_ww_class);
>  
> -	list_for_each_entry(entry, list, head) {
> +	for (entry = list_first_entry(list, typeof(*entry), head);
> +	     !list_entry_is_head(entry, list, head);
> +	     entry = list_next_entry(entry, head)) {

[Severity: High]
This is a pre-existing issue, but does the error path in this loop leak
the lock on entry->bo when dma_resv_reserve_fences() fails?

If ttm_bo_reserve() successfully acquires the lock on the current entry->bo,
and then dma_resv_reserve_fences() fails (for example, returning -ENOMEM),
the code cleans up using ttm_eu_backoff_reservation_reverse():

ttm_eu_reserve_buffers() {
    ...
    if (!ret) {
        ret = dma_resv_reserve_fences(bo->base.resv, num_fences);
        if (!ret)
            continue;
    }

    /* uh oh, we lost out, drop every reservation and try... */
    ttm_eu_backoff_reservation_reverse(list, entry);
    ...
}

However, ttm_eu_backoff_reservation_reverse() explicitly skips unlocking the
current entry itself because it uses list_for_each_entry_continue_reverse().

The code then bypasses the slowpath because ret is -ENOMEM (not -EDEADLK)
and returns without unlocking entry->bo:

ttm_eu_reserve_buffers() {
    ...
    if (ret == -EDEADLK) {
        ret = ttm_bo_reserve_slowpath(bo, intr, ticket);
    }
    ...
    if (unlikely(ret != 0)) {
        if (ticket) {
            ww_acquire_done(ticket);
            ww_acquire_fini(ticket);
        }
        return ret;
    }
    ...
}

Could this sequence leave the ww_mutex for entry->bo permanently locked?

>  		struct ttm_buffer_object *bo = entry->bo;
>  		unsigned int num_fences;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064122.95825-1-kaitao.cheng@linux.dev?part=6