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 <[email protected]> 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/[email protected]?part=6
