Re: [PATCH v3 1/7] list: Add mutable iterator variants

[Kaitao Cheng <kaitao.cheng@linux.dev>]

在 2026/6/25 19:00, Jani Nikula 写道:
> On Thu, 25 Jun 2026, Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
>> 在 2026/6/24 22:23, David Laight 写道:
>>> On Wed, 24 Jun 2026 15:23:47 +0200
>>> Christian König <christian.koenig@amd.com> wrote:
>>>> On 6/24/26 15:14, Kaitao Cheng wrote:
>>>>> 在 2026/6/22 16:42, David Laight 写道:  
>>>>>> On Mon, 22 Jun 2026 12:05:31 +0800
>>>>>> Kaitao Cheng <kaitao.cheng@linux.dev> wrote:
>>>>>>  
>>>>>>> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>>>>>>>
>>>>>>> The list_for_each*_safe() helpers are used when the loop body may
>>>>>>> remove the current entry.  Their API exposes the temporary cursor at
>>>>>>> every call site, even though most users only need it for the iterator
>>>>>>> implementation and never reference it in the loop body.
>>>>>>>
>>>>>>> Add *_mutable() variants for list and hlist iteration.  The new helpers
>>>>>>> support both forms: callers may keep passing an explicit temporary cursor
>>>>>>> when they need to inspect or reset it, or omit it and let the helper use
>>>>>>> a unique internal cursor.  
>>>>>>
>>>>>> I'm not really sure 'mutable' means anything either.
>>>>>> It is possible to make it valid for the loop body (or even other threads)
>>>>>> to delete arbitrary list items - but that needs significant extra overheads.
>>>>>>
>>>>>> It might be worth doing something that doesn't need the extra variable,
>>>>>> but there is little point doing all the churn just to rename things.
>>>>>>  
>>>>>>>
>>>>>>> This makes call sites that only mutate the list through the current entry
>>>>>>> less noisy, while keeping the existing *_safe() helpers available for
>>>>>>> compatibility.
>>>>>>>
>>>>>>> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
>>>>>>> ---
>>>>>>>  include/linux/list.h | 269 +++++++++++++++++++++++++++++++++++++------
>>>>>>>  1 file changed, 231 insertions(+), 38 deletions(-)
>>>>>>>
>>>>>>> diff --git a/include/linux/list.h b/include/linux/list.h
>>>>>>> index 09d979976b3b..1081def7cea9 100644
>>>>>>> --- a/include/linux/list.h
>>>>>>> +++ b/include/linux/list.h
>>>>>>> @@ -7,6 +7,7 @@
>>>>>>>  #include <linux/stddef.h>
>>>>>>>  #include <linux/poison.h>
>>>>>>>  #include <linux/const.h>
>>>>>>> +#include <linux/args.h>
>>>>>>>  
>>>>>>>  #include <asm/barrier.h>
>>>>>>>  
>>>>>>> @@ -763,28 +764,72 @@ static inline void list_splice_tail_init(struct list_head *list,
>>>>>>>  #define list_for_each_prev(pos, head) \
>>>>>>>  	for (pos = (head)->prev; !list_is_head(pos, (head)); pos = pos->prev)
>>>>>>>  
>>>>>>> -/**
>>>>>>> - * list_for_each_safe - iterate over a list safe against removal of list entry
>>>>>>> - * @pos:	the &struct list_head to use as a loop cursor.
>>>>>>> - * @n:		another &struct list_head to use as temporary storage
>>>>>>> - * @head:	the head for your list.
>>>>>>> +/*
>>>>>>> + * list_for_each_safe is an old interface, use list_for_each_mutable instead.
>>>>>>>   */
>>>>>>>  #define list_for_each_safe(pos, n, head) \
>>>>>>>  	for (pos = (head)->next, n = pos->next; \
>>>>>>>  	     !list_is_head(pos, (head)); \
>>>>>>>  	     pos = n, n = pos->next)
>>>>>>>  
>>>>>>> +#define __list_for_each_mutable_internal(pos, tmp, head)		\
>>>>>>> +	for (typeof(pos) tmp = (pos = (head)->next)->next;		\  
>>>>>>
>>>>>> Use auto
>>>>>>  
>>>>>>> +	     !list_is_head(pos, (head));				\
>>>>>>> +	     pos = tmp, tmp = pos->next)
>>>>>>> +
>>>>>>> +#define __list_for_each_mutable1(pos, head)				\
>>>>>>> +	__list_for_each_mutable_internal(pos, __UNIQUE_ID(next), head)
>>>>>>> +
>>>>>>> +#define __list_for_each_mutable2(pos, next, head)			\
>>>>>>> +	list_for_each_safe(pos, next, head)
>>>>>>> +
>>>>>>>  /**
>>>>>>> - * list_for_each_prev_safe - iterate over a list backwards safe against removal of list entry
>>>>>>> + * list_for_each_mutable - iterate over a list safe against entry removal
>>>>>>>   * @pos:	the &struct list_head to use as a loop cursor.
>>>>>>> - * @n:		another &struct list_head to use as temporary storage
>>>>>>> - * @head:	the head for your list.
>>>>>>> + * @...:	either (head) or (next, head)
>>>>>>> + *
>>>>>>> + * next:	another &struct list_head to use as optional temporary storage.
>>>>>>> + *		The temporary cursor is internal unless explicitly supplied by
>>>>>>> + *		the caller.
>>>>>>> + * head:	the head for your list.
>>>>>>> + */
>>>>>>> +#define list_for_each_mutable(pos, ...)					\
>>>>>>> +	CONCATENATE(__list_for_each_mutable, COUNT_ARGS(__VA_ARGS__))	\
>>>>>>> +		(pos, __VA_ARGS__)  
>>>>>>
>>>>>> The variable argument count logic really just slows down compilation.
>>>>>> Maybe there aren't enough copies of this code to make that significant.
>>>>>> But just because you can do it doesn't mean it is a gooD idea.
>>>>>> I'm also not sure it really adds anything to the readability.
>>>>>>
>>>>>> And, it you are going to make the middle argument optional there is
>>>>>> no need to change the macro name.  
>>>>>
>>>>> Christian König and Jani Nikula also disagree with the variadic-argument
>>>>> implementation approach. If we abandon that method, it means we will
>>>>> inevitably need to add some new macros. If mutable is not a good name,
>>>>> suggestions for better alternatives would be welcome; coming up with a
>>>>> suitable name is indeed rather tricky.  
>>>>
>>>> I don't think you need to add a new macro for the specific use case that people want to modify the next element of the iteration.
>>>>
>>>> If I remember your numbers correctly that is a really corner case and keeping using the existing *_safe() macros for that sounds perfectly fine to me.
>>>
>>> IIRC currently you have a choice of either:
>>> 	define               Item that can't be deleted
>>> 	list_for_each()	     The current item.
>>> 	list_for_each_safe() The next item.
>>> There is also likely to be code that updates the variables to allow
>>> for other scenarios.
>>>
>>> Note that if increase a reference count and release a lock then list_for_each()
>>> is likely safer than list_for_each_safe() :-)
>>>
>>> list.h has 9 variants of the 'safe' loop.
>>> The bloat of another 9 is getting excessive.
>>>
>>> It has to be said that this is one of my least favourite type of list...
>>
>> Hi Christian König, David Laight, Jani Nikula, David Hildenbrand,
>> Andy Shevchenko, Alexei Starovoitov
>>
>> For ease of discussion, I need to summarize the currently possible
>> approaches and briefly describe their respective pros and cons,
>> using the list_for_each_entry* interfaces as examples.
>>
>> 1. Add list_for_each_entry_mutable, while keeping list_for_each_entry
>> and list_for_each_entry_safe unchanged. list_for_each_entry_mutable
>> would be used specifically for safe deletion scenarios that do not
>> need to expose the temporary cursor externally. The code can refer to
>> the v1 version.
>>
>> Pros: Does not depend on immediate per-subsystem adaptation and can be
>>       merged directly.
>> Cons: Requires adding a whole set of mutable interfaces, which makes the
>>       code somewhat redundant.
> 
> Seems fine, and the original _safe naming is ambiguous anyway.
> 
>> 2. Directly optimize away the temporary cursor in list_for_each_entry_safe
>> and define it inside the loop instead, changing the interface from four
>> arguments to three.
>>
>> Pros: Does not add redundant interfaces.
>> Cons: (1) Users need to manually update special cases that use the
>>       traversal variable of list_for_each_entry_safe, the new
>>       list_for_each_entry_safe would no longer apply there and would
>>       need to be open-coded.
>>       (2) Because the macro arguments changes, all list_for_each_entry_safe
>>       callers would need to be modified and merged together, making it
>>       difficult to merge such a large amount of code at once.
> 
> This won't fly because there are literally thousands of
> list_for_each_entry_safe() users.
> 
>> 3. Use a variadic macro approach to optimize list_for_each_entry_safe,
>> so that it supports both three and four arguments.
>>
>> Pros: (1) Does not add redundant interfaces.
>>       (2) Does not depend on immediate per-subsystem adaptation and can
>>       be merged directly.
>> Cons: (1) Increases compile time.
>>       (2) Makes the interface harder for users to use.
> 
> Basically I'm against any variadic macro tricks where the optional
> argument is not the last argument. That's just way too surprising, and
> goes against common practice in just about all other languages.
> 
>> 4. Optimize list_for_each_entry by defining the temporary cursor internally,
>> making it compatible with the functionality of list_for_each_entry_safe.
>> The code can refer to the v2 version.
>>
>> Pros: (1) Does not add redundant interfaces.
>>       (2) The number of externally visible arguments of list_for_each_entry
>>       remains unchanged, still three.
>> Cons: (1) list_for_each_entry and list_for_each_entry_safe would be merged
>>       into one, and list_for_each_entry_safe would gradually be deprecated.
>>       (2) Users need to manually update special cases that use the traversal
>>       variable of list_for_each_entry, the new list_for_each_entry would no
>>       longer apply there and would need to be open-coded. There are 15 such
>>       cases in total.
> 
> This sounds good to me, though I take it there's some code size increase
> and/or performance penalty?
> 
> Maybe the 15 cases are questionable anyway?
> 
>> 5. Use a variadic macro approach to optimize list_for_each_entry, so that
>> it supports both three and four arguments.
>>
>> Pros: (1) Does not add redundant interfaces.
>>       (2) Does not depend on immediate per-subsystem adaptation and can be
>>       merged directly.
>> Cons: (1) Increases compile time.
>>       (2) list_for_each_entry and list_for_each_entry_safe would be merged
>>       into one, and list_for_each_entry_safe would gradually be deprecated.
> 
> Please don't do the macro tricks.
> 
>> 6. Make no changes, keep the current logic unchanged, and close the current
>> email discussion.
> 
> I like hiding the temporary stuff when possible.
> 
> BR,
> Jani.

Hi all,
If there are no objections, I will make the changes using the first approach.


Hi David Laight,
You previously expressed a different opinion. Do you have any further comments
on the current proposed approach?

-- 
Thanks
Kaitao Cheng