Re: drm/amdgpu: dead empty checks on e->list in ring_mux ib_mark_offset and end_ib?

["Christian König" <christian.koenig@amd.com>]

On 5/19/26 21:41, Maoyi Xie wrote:
> Hi all,
> 
> While auditing list_last_entry callsites, I noticed two places in
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c where the developer
> wrote a NULL check for an empty list but used the unsafe API. The
> check is dead code. I would appreciate it if you could take a
> look and let me know whether these are worth fixing.
> 
> The two sites are amdgpu_ring_mux_ib_mark_offset() and
> amdgpu_ring_mux_end_ib() (linux-7.1-rc1, around lines 497 and
> 530):
> 
>     chunk = list_last_entry(&e->list, struct amdgpu_mux_chunk, entry);
>     if (!chunk) {
>             DRM_ERROR("cannot find chunk!\n");
>             return;
>     }
> 
> list_last_entry() returns container_of(&e->list, struct
> amdgpu_mux_chunk, entry) when e->list is empty, never NULL. The
> "cannot find chunk!" error path is dead code.

Good catch, as far as I can see the NULL check can safely be removed.

Regards,
Christian.

> 
> With an empty e->list, the fall through pointer aliases &e->list
> inside struct amdgpu_mux_entry. The writes that follow then
> corrupt fields of the mux_entry at the corresponding offsets.
> mark_offset writes cntl_offset, de_offset and ce_offset. end_ib
> writes end and sync_seq.
> 
> e->list is empty if a software ring submits an IB mark or IB end
> before any chunk is queued for that ring. This can happen on a
> fresh start_ib path, or after end_ib drops the last chunk.
> 
> A candidate fix is a one liner per site. Switch to
> list_last_entry_or_null so the existing error path runs.
> 
> Similar dead empty checks after list_first_entry / list_last_entry
> have been cleaned up in the same shape, for example commit
> fbb8bc408027 (net: qed: Remove redundant NULL checks after
> list_first_entry), commit c708d3fad421 (crypto: atmel: use
> list_first_entry_or_null to simplify find_dev) and commit
> 10379171f346 (ksmbd: use list_first_entry_or_null for
> opinfo_get_list). The qed commit message describes the exact
> shape we observe here. These two sites appear to be missed by
> those cleanups.
> 
> If this is intentional or already known, please disregard.
> Otherwise I am happy to send a [PATCH] or to leave the fix to you.
> 
> Thanks,
> Maoyi Xie
> https://maoyixie.com/