Messages in this thread | | | Date | Thu, 7 Oct 2021 23:40:51 +0000 | From | Wei Yang <> | Subject | Re: [PATCH] hashtable: remove a redundant check in hash_for_each_xxx() |
| |
On Thu, Oct 07, 2021 at 11:50:22AM +1100, NeilBrown wrote: >On Thu, 07 Oct 2021, Wei Yang wrote: >> >> Here is a draft patch based on you comment: >> >> diff --git a/include/linux/hashtable.h b/include/linux/hashtable.h >> index f6c666730b8c..2ff4cb5e6a22 100644 >> --- a/include/linux/hashtable.h >> +++ b/include/linux/hashtable.h >> @@ -116,6 +116,13 @@ static inline void hash_del_rcu(struct hlist_node *node) >> hlist_del_init_rcu(node); >> } >> >> +/** >> + * Note: the following three hash_for_each[_xxx] helpers introduce a new loop >> + * command that is constructed from 2 nested loops. It is safe to 'break' out >> + * of this loop even though it is a two nested loops. The 'obj == NULL' test >> + * ensures that when the inner loop is broken, the outer loop will break too. >> + */ >> + >> /** >> * hash_for_each - iterate over a hashtable >> * @name: hashtable to iterate >> >> >> If you feel good, I would like to add >> >> Sugguested-by: NeilBrown <neilb@suse.de> > >That's definitely an improvement. > >I'd probably put it in the kernel-doc comment for hash_for_each, >then in the other two just put the "it is safe" bit. Something like >the following. But I don't feel strongly about it. >I'm happy to say > Reviewed-by: NeilBrown <neilb@suse.de> >
Thanks for your detailed instruction :-)
>for your patch. > >Thanks, >NeilBrown > > >diff --git a/include/linux/hashtable.h b/include/linux/hashtable.h >index f6c666730b8c..61db940c9501 100644 >--- a/include/linux/hashtable.h >+++ b/include/linux/hashtable.h >@@ -122,6 +122,10 @@ static inline void hash_del_rcu(struct hlist_node *node) > * @bkt: integer to use as bucket loop cursor > * @obj: the type * to use as a loop cursor for each entry > * @member: the name of the hlist_node within the struct >+ * >+ * Note: It is safe to 'break' out of this loop even though it is a two >+ * nested loops. The 'obj == NULL' test ensures that when the inner loop >+ * is broken, the outer loop will break too. > */ > #define hash_for_each(name, bkt, obj, member) \ > for ((bkt) = 0, obj = NULL; obj == NULL && (bkt) < HASH_SIZE(name);\ >@@ -134,6 +138,8 @@ static inline void hash_del_rcu(struct hlist_node *node) > * @bkt: integer to use as bucket loop cursor > * @obj: the type * to use as a loop cursor for each entry > * @member: the name of the hlist_node within the struct >+ * >+ * It is safe to 'break' out of this loop. > */ > #define hash_for_each_rcu(name, bkt, obj, member) \ > for ((bkt) = 0, obj = NULL; obj == NULL && (bkt) < HASH_SIZE(name);\ >@@ -148,6 +154,8 @@ static inline void hash_del_rcu(struct hlist_node *node) > * @tmp: a &struct hlist_node used for temporary storage > * @obj: the type * to use as a loop cursor for each entry > * @member: the name of the hlist_node within the struct >+ * >+ * It is safe to 'break' out of this loop. > */ > #define hash_for_each_safe(name, bkt, tmp, obj, member) \ > for ((bkt) = 0, obj = NULL; obj == NULL && (bkt) < HASH_SIZE(name);\
-- Wei Yang Help you, Help me
| |