Re: [PATCH v3 1/7] list: Add mutable iterator variants
Kaitao Cheng <[email protected]> Wed, 24 Jun 2026 21:14:50 +0800
| Newsgroups | org.kernel.vger.audit,dev.linux.lists.damon,dev.linux.lists.llvm,dev.linux.lists.sched-ext,dev.linux.lists.virtualization,org.freedesktop.lists.dri-devel,org.infradead.lists.kexec,org.kernel.vger.bpf,org.kernel.vger.cgroups,org.kernel.vger.io-uring,org.kernel.vger.linux-block,org.kernel.vger.linux-crypto,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-modules,org.kernel.vger.linux-perf-users,org.kernel.vger.linux-pm,org.kernel.vger.linux-trace-kernel,org.kernel.vger.live-patching,org.kernel.vger.netdev,org.kernel.vger.rcu,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
在 2026/6/22 16:42, David Laight 写道: > On Mon, 22 Jun 2026 12:05:31 +0800 > Kaitao Cheng <[email protected]> wrote: > >> From: Kaitao Cheng <[email protected]> >> >> 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 <[email protected]> >> --- >> 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. -- Thanks Kaitao Cheng