| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a3bba801-5253-5ab1-4dce-43c5b7cb407c@ssi.bg>
Date: Wed, 16 Aug 2023 19:02:39 +0300 (EEST)
From: Julian Anastasov <ja@....bg>
To: "Paul E. McKenney" <paulmck@...nel.org>
cc: Simon Horman <horms@...ge.net.au>, lvs-devel@...r.kernel.org,
netfilter-devel@...r.kernel.org, netdev@...r.kernel.org,
rcu@...r.kernel.org, Dust Li <dust.li@...ux.alibaba.com>,
Jiejian Wu <jiejian@...ux.alibaba.com>,
Jiri Wiesner <jwiesner@...e.de>
Subject: Re: [PATCH RFC net-next 01/14] rculist_bl: add
hlist_bl_for_each_entry_continue_rcu
Hello,
On Tue, 15 Aug 2023, Paul E. McKenney wrote:
> On Tue, Aug 15, 2023 at 08:30:18PM +0300, Julian Anastasov wrote:
> > Add hlist_bl_for_each_entry_continue_rcu and hlist_bl_next_rcu
> >
> > Signed-off-by: Julian Anastasov <ja@....bg>
> > ---
> > include/linux/rculist_bl.h | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/include/linux/rculist_bl.h b/include/linux/rculist_bl.h
> > index 0b952d06eb0b..93a757793d83 100644
> > --- a/include/linux/rculist_bl.h
> > +++ b/include/linux/rculist_bl.h
> > @@ -24,6 +24,10 @@ static inline struct hlist_bl_node *hlist_bl_first_rcu(struct hlist_bl_head *h)
> > ((unsigned long)rcu_dereference_check(h->first, hlist_bl_is_locked(h)) & ~LIST_BL_LOCKMASK);
> > }
> >
> > +/* return the next element in an RCU protected list */
> > +#define hlist_bl_next_rcu(node) \
> > + (*((struct hlist_bl_node __rcu **)(&(node)->next)))
> > +
> > /**
> > * hlist_bl_del_rcu - deletes entry from hash list without re-initialization
> > * @n: the element to delete from the hash list.
> > @@ -98,4 +102,17 @@ static inline void hlist_bl_add_head_rcu(struct hlist_bl_node *n,
> > ({ tpos = hlist_bl_entry(pos, typeof(*tpos), member); 1; }); \
> > pos = rcu_dereference_raw(pos->next))
> >
> > +/**
> > + * hlist_bl_for_each_entry_continue_rcu - iterate over a list continuing after
> > + * current point
>
> Please add a comment to the effect that the element continued from
> must have been either: (1) Iterated to within the same RCU read-side
> critical section or (2) Nailed down using some lock, reference count,
> or whatever suffices to keep the continued-from element from being freed
> in the meantime.
I created 2nd version which has more changes. I'm not sure
what are the desired steps for this patch, should I keep it as part
of my patchset if accepted? Or should I post it separately? Here it
is v2 for comments.
[PATCHv2 RFC] rculist_bl: add hlist_bl_for_each_entry_continue_rcu
Change the old hlist_bl_first_rcu to hlist_bl_first_rcu_dereference
to indicate that it is a RCU dereference.
Add hlist_bl_next_rcu and hlist_bl_first_rcu to use RCU pointers
and use them to fix sparse warnings.
Add hlist_bl_for_each_entry_continue_rcu.
Signed-off-by: Julian Anastasov <ja@....bg>
---
include/linux/rculist_bl.h | 49 +++++++++++++++++++++++++++++++-------
1 file changed, 40 insertions(+), 9 deletions(-)
diff --git a/include/linux/rculist_bl.h b/include/linux/rculist_bl.h
index 0b952d06eb0b..36363b876e53 100644
--- a/include/linux/rculist_bl.h
+++ b/include/linux/rculist_bl.h
@@ -8,21 +8,31 @@
#include <linux/list_bl.h>
#include <linux/rcupdate.h>
+/* return the first ptr or next element in an RCU protected list */
+#define hlist_bl_first_rcu(head) \
+ (*((struct hlist_bl_node __rcu **)(&(head)->first)))
+#define hlist_bl_next_rcu(node) \
+ (*((struct hlist_bl_node __rcu **)(&(node)->next)))
+
static inline void hlist_bl_set_first_rcu(struct hlist_bl_head *h,
struct hlist_bl_node *n)
{
LIST_BL_BUG_ON((unsigned long)n & LIST_BL_LOCKMASK);
LIST_BL_BUG_ON(((unsigned long)h->first & LIST_BL_LOCKMASK) !=
LIST_BL_LOCKMASK);
- rcu_assign_pointer(h->first,
+ rcu_assign_pointer(hlist_bl_first_rcu(h),
(struct hlist_bl_node *)((unsigned long)n | LIST_BL_LOCKMASK));
}
-static inline struct hlist_bl_node *hlist_bl_first_rcu(struct hlist_bl_head *h)
-{
- return (struct hlist_bl_node *)
- ((unsigned long)rcu_dereference_check(h->first, hlist_bl_is_locked(h)) & ~LIST_BL_LOCKMASK);
-}
+#define hlist_bl_first_rcu_dereference(head) \
+({ \
+ struct hlist_bl_head *__head = (head); \
+ \
+ (struct hlist_bl_node *) \
+ ((unsigned long)rcu_dereference_check(hlist_bl_first_rcu(__head), \
+ hlist_bl_is_locked(__head)) & \
+ ~LIST_BL_LOCKMASK); \
+})
/**
* hlist_bl_del_rcu - deletes entry from hash list without re-initialization
@@ -73,7 +83,7 @@ static inline void hlist_bl_add_head_rcu(struct hlist_bl_node *n,
{
struct hlist_bl_node *first;
- /* don't need hlist_bl_first_rcu because we're under lock */
+ /* don't need hlist_bl_first_rcu* because we're under lock */
first = hlist_bl_first(h);
n->next = first;
@@ -93,9 +103,30 @@ static inline void hlist_bl_add_head_rcu(struct hlist_bl_node *n,
*
*/
#define hlist_bl_for_each_entry_rcu(tpos, pos, head, member) \
- for (pos = hlist_bl_first_rcu(head); \
+ for (pos = hlist_bl_first_rcu_dereference(head); \
pos && \
({ tpos = hlist_bl_entry(pos, typeof(*tpos), member); 1; }); \
- pos = rcu_dereference_raw(pos->next))
+ pos = rcu_dereference_raw(hlist_bl_next_rcu(pos)))
+
+/**
+ * hlist_bl_for_each_entry_continue_rcu - continue iteration over list of given
+ * type
+ * @tpos: the type * to use as a loop cursor.
+ * @pos: the &struct hlist_bl_node to use as a loop cursor.
+ * @member: the name of the hlist_bl_node within the struct.
+ *
+ * Continue to iterate over list of given type, continuing after
+ * the current position which must have been in the list when the RCU read
+ * lock was taken.
+ * This would typically require either that you obtained the node from a
+ * previous walk of the list in the same RCU read-side critical section, or
+ * that you held some sort of non-RCU reference (such as a reference count)
+ * to keep the node alive *and* in the list.
+ */
+#define hlist_bl_for_each_entry_continue_rcu(tpos, pos, member) \
+ for (pos = rcu_dereference_raw(hlist_bl_next_rcu(&(tpos)->member)); \
+ pos && \
+ ({ tpos = hlist_bl_entry(pos, typeof(*tpos), member); 1; }); \
+ pos = rcu_dereference_raw(hlist_bl_next_rcu(pos)))
#endif
--
2.41.0
Regards
--
Julian Anastasov <ja@....bg>
Powered by blists - more mailing lists