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

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

在 2026/6/10 22:43, Andy Shevchenko 写道:
> On Wed, Jun 10, 2026 at 02:14:06PM +0800, Kaitao Cheng wrote:
>> 在 2026/6/9 18:33, Christian König 写道:
>>> On 6/9/26 08:13, Kaitao Cheng wrote:
>>>>
>>>> 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.
> 
> Ah, I think the distinct macro families is that what we want.
> But the hiding of the parameter can be done inside list_for_each_*_safe().
> You can do a treewide change with coccinelle.
> 
> Sorry if I didn't get the whole idea from your previous contributions.
> 
> Note, even cases that would need a temporary cursor may be switched to
> new list_for_each_*_safe(), see how PCI macros for iterating over resources
> are implemented (include/linux/pci.h).

Thanks for your suggestions. I've written a demo based on your feedback.
Could you please review it and share your thoughts on this approach?


diff --git a/include/linux/list.h b/include/linux/list.h
index 9df84a56a789..306554ab1841 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>

@@ -911,20 +912,34 @@ static inline size_t list_count_nodes(struct list_head *head)
        for (; !list_entry_is_head(pos, head, member);                  \
             pos = list_prev_entry(pos, member))

+#define __list_for_each_entry_safe_internal(pos, next, head, member)   \
+       for (typeof(pos) next = list_next_entry(pos =                   \
+               list_first_entry(head, typeof(*pos), member), member);  \
+            !list_entry_is_head(pos, head, member);                    \
+            pos = next, next = list_next_entry(next, member))
+
+#define __list_for_each_entry_safe2(pos, head, member)                 \
+       __list_for_each_entry_safe_internal(pos, __UNIQUE_ID(next), head, member)
+
+#define __list_for_each_entry_safe3(pos, next, head, member)           \
+       for (pos = list_first_entry(head, typeof(*pos), member),        \
+               next = list_next_entry(pos, member);                    \
+            !list_entry_is_head(pos, head, member);                    \
+            pos = next, next = list_next_entry(next, member))
+
 /**
  * list_for_each_entry_safe - iterate over list of given type safe against removal of list entry
  * @pos:       the type * to use as a loop cursor.
- * @n:         another type * to use as temporary storage
- * @head:      the head for your list.
- * @member:    the name of the list_head within the struct.
+ * @...:       either (head, member) or (next, head, member)
+ *     @next:  another type * to use as optional temporary storage. The temporary
+ *             cursor is internal unless explicitly supplied by the caller.
+ *     @head:  the head for your list.
+ *     @member:the name of the list_head within the struct.
  *
  */
-#define list_for_each_entry_safe(pos, n, head, member)                 \
-       for (pos = list_first_entry(head, typeof(*pos), member),        \
-               n = list_next_entry(pos, member);                       \
-            !list_entry_is_head(pos, head, member);                    \
-            pos = n, n = list_next_entry(n, member))
+#define list_for_each_entry_safe(pos, ...)                             \
+       CONCATENATE(__list_for_each_entry_safe, COUNT_ARGS(__VA_ARGS__))\
+               (pos, __VA_ARGS__)

 /**
  * list_for_each_entry_safe_continue - continue list iteration safe against removal

>> 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.
>>
>>>> The final patch changes include/linux/list.h to keep a private cursor in
>>>> the common entry iterators while preserving the public macro interface.
>>>> The safe variants remain available when callers need the temporary
>>>> cursor explicitly or have stronger mutation requirements.
> 
> 

-- 
Thanks
Kaitao Cheng