Re: [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state

[Christian König]

On 6/10/26 10:18, Kaitao Cheng wrote:
> 
> 
> 在 2026/6/10 16:07, Christian König 写道:
>> On 6/10/26 08:14, Kaitao Cheng wrote:
>>> 在 2026/6/9 18:33, Christian König 写道:
>>>> On 6/9/26 08:13, Kaitao Cheng wrote:
>>>>> From: Kaito Cheng <chengkaitao@xxxxxxxxxx>
>>>>>
>>>>> This series prepares for, and then updates, the list_for_each_entry()
>>>>> family so the common entry iterators cache their next or previous cursor
>>>>> before the loop body runs.
>>>>
>>>> Why in the world would we want to do that?
>>>>
>>>> The safe and non-safe variants have very distinct use cases and that is completely intentional.
>>>>
>>>> What we could improve maybe is the documentation, from my experience an astonishing large amount of people have misconceptions about the safe variants.
>>>>
>>>>> The first 13 patches open-code loops that intentionally depend on the
>>>>> old "derive the next entry from the current cursor at the end of the
>>>>> iteration" behaviour.  These loops append work to the list being walked,
>>>>> restart traversal after dropping a lock, skip an entry consumed by the
>>>>> current iteration, or otherwise adjust the cursor in the loop body.
>>>>
>>>> Well I have to clearly reject the changes for subsystems/components I'm maintaining, that just looks horrible to me and I clearly don't see a good reason for that.
>>>
>>> Hi Christian and Andy Shevchenko,
>>>
>>> Thanks for taking a look. I would like to clarify the point you raised.
>>>
>>> The reason I started looking at this is the original motivation behind
>>> the _safe() variants.  They exist because some users need to remove, move
>>> or otherwise consume the current entry while walking the list.  In that
>>> case the next cursor has to be preserved before the loop body can modify
>>> the current entry.
>>>
>>> The unfortunate part is that this could not be expressed with the
>>> existing list_for_each_entry() interface without changing its calling
>>> convention.  The _safe() variants had to grow an extra argument for the
>>> temporary cursor, and that is why we ended up with a separate family of
>>> macros.
>>>
>>> But conceptually, the distinction does not have to be exposed as two
>>> different iterator families forever.  The difference is an implementation
>>> detail: whether the iterator keeps the next/previous cursor before the
>>> body runs.  This series makes the common list_for_each_entry() iterators
>>> do that internally, so the safe and non-safe forms can effectively be
>>> folded together, or at least the need for a separate public _safe()
>>> interface becomes much weaker.
>>>
>>> There is also a usability issue with the current _safe() interface.  The
>>> caller is forced to define a temporary cursor outside the macro and pass
>>> it in, even though almost all users never use that cursor directly.  It is
>>> just boilerplate required by the macro implementation.  I find that
>>> redundant and awkward: the temporary cursor is an internal detail of the
>>> iteration, but every caller has to spell it out.
>>>
>>> With the updated list_for_each_entry() implementation, that extra cursor
>>> can be kept inside the iterator itself.  Callers that only want to walk
>>> the list, including callers that delete or consume the current entry, no
>>> longer need to carry an otherwise-unused temporary variable just to make
>>> the macro work.
>>
>> Well the distinction between list_for_each_entry() and list_for_each_entry_safe() is *not* there because you need an extra variable to hold the next pointer, but because just 'iterating the list' and 'iterating the list while you modify it' are two distinct use cases.
>>
>> Apart from the technical implications this also has some documentation value for the code using it.
>>
>> What we could consider with C99 at hand is to have _safe() variants who uses a local hidden variable to hold the next element.
>>
>> Or maybe come up with a better name instead of _safe() because people seem to misunderstand that quite often.
>>
>> But mangling the two use cases together just because it is now technical possible is among the worst ideas I've ever heard.
>>
> 
> Should we revert to v1, or keep list_for_each_entry() and
> list_for_each_entry_safe() as they are, close this thread, and make no
> changes?
> 
> Link to v1:
> https://lore.kernel.org/all/20260529082149.76764-1-kaitao.cheng@xxxxxxxxx/
> 
> Or do you have any better suggestions?

v1 looks perfectly reasonable to me.

You should just include some patches in the same patch set to actually use the new macros.

If you modify the files under drivers/dma-buf or drivers/gpu/drm/amd to use the new macro I'm happy to review that.

Regards,
Christian.