* [lttng-dev] Query on the rculist
2013-01-24 5:22 ` Liu Yuan
@ 2013-01-24 17:04 ` Mathieu Desnoyers
0 siblings, 0 replies; 5+ messages in thread
From: Mathieu Desnoyers @ 2013-01-24 17:04 UTC (permalink / raw)
* Liu Yuan (namei.unix at gmail.com) wrote:
> On 01/22/2013 10:24 PM, Lai Jiangshan wrote:
> > Add a trivial comment.
> >
> > There is no list_del_init_rcu(), "init" hurts "rcu".
> > If you need a re-initialized object after deletion, you should use
> > synchronzie_rcu():
> >
> > list_del_rcu();
> > synchronize_rcu();
> > CDS_INIT_LIST_HEAD();
>
> Thanks, note taken. Mathieu, could you add this as comment for
> rculist.h? It will save people from finding list_dei_init_rcu().
Sure, done. Here is the updated proposed patch:
commit 82bffe25ba06e2ab8c872640efdf40d1f17f0e85
Author: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
Date: Mon Jan 21 15:53:41 2013 -0500
expand and cleanup list.h and rculist.h
Cleanup urcu/list.h and urcu/rculist.h: adopt Userspace RCU project
coding style, comment each function thoroughly.
Add the following API members:
- cds_list_for_each_safe()
- cds_list_for_each_entry_reverse_safe()
- cds_list_add_tail_rcu()
Document that cds_list_empty() can be used on RCU lists without
protection.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
CC: Liu Yuan <namei.unix at gmail.com>
CC: Lai Jiangshan <laijs at cn.fujitsu.com>
CC: Paul E. McKenney <paulmck at linux.vnet.ibm.com>
diff --git a/urcu/list.h b/urcu/list.h
index 5d04394..61e3298 100644
--- a/urcu/list.h
+++ b/urcu/list.h
@@ -5,7 +5,7 @@
*
* Copyright (C) 2009 Pierre-Marc Fournier
* Conversion to RCU list.
- * Copyright (C) 2010 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
+ * Copyright (C) 2010-2013 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -25,86 +25,124 @@
#ifndef _CDS_LIST_H
#define _CDS_LIST_H 1
-/* The definitions of this file are adopted from those which can be
- found in the Linux kernel headers to enable people familiar with
- the latter find their way in these sources as well. */
-
+/*
+ * The definitions of this file are adopted from those which can be
+ * found in the Linux kernel headers to enable people familiar with the
+ * latter find their way in these sources as well.
+ */
-/* Basic type for the double-link list. */
-struct cds_list_head
-{
- struct cds_list_head *next;
- struct cds_list_head *prev;
+/* Basic type for the double-link list. */
+struct cds_list_head {
+ struct cds_list_head *next;
+ struct cds_list_head *prev;
};
+/* Define a variable with the head and tail of the list. */
+#define CDS_LIST_HEAD(name) \
+ struct cds_list_head name = CDS_LIST_HEAD_INIT(name)
-/* Define a variable with the head and tail of the list. */
-#define CDS_LIST_HEAD(name) \
- struct cds_list_head name = { &(name), &(name) }
-
-/* Initialize a new list head. */
-#define CDS_INIT_LIST_HEAD(ptr) \
- (ptr)->next = (ptr)->prev = (ptr)
-
-#define CDS_LIST_HEAD_INIT(name) { .prev = &(name), .next = &(name) }
+#define CDS_LIST_HEAD_INIT(name) { .prev = &(name), .next = &(name) }
-/* Add new element at the head of the list. */
-static inline void
-cds_list_add (struct cds_list_head *newp, struct cds_list_head *head)
+/*
+ * @CSD_INIT_LIST_HEAD: Initialize a new list head.
+ * @head: head of list to initialize.
+ */
+static inline
+void CDS_INIT_LIST_HEAD(struct cds_list_head *head)
{
- head->next->prev = newp;
- newp->next = head->next;
- newp->prev = head;
- head->next = newp;
+ head->next = head;
+ head->prev = head;
}
-
-/* Add new element at the tail of the list. */
-static inline void
-cds_list_add_tail (struct cds_list_head *newp, struct cds_list_head *head)
+/*
+ * cds_list_add: Add new element at the head of the list.
+ * @newp: new element.
+ * @head: list head.
+ *
+ * Mutual exclusion against other updates and reads to list is required.
+ */
+static inline
+void cds_list_add(struct cds_list_head *newp, struct cds_list_head *head)
{
- head->prev->next = newp;
- newp->next = head;
- newp->prev = head->prev;
- head->prev = newp;
+ head->next->prev = newp;
+ newp->next = head->next;
+ newp->prev = head;
+ head->next = newp;
}
+/*
+ * cds_list_add_tail: Add new element at the tail of the list.
+ * @newp: new element.
+ * @head: list head.
+ *
+ * Mutual exclusion against other updates and reads to list is required.
+ */
+static inline
+void cds_list_add_tail(struct cds_list_head *newp, struct cds_list_head *head)
+{
+ head->prev->next = newp;
+ newp->next = head;
+ newp->prev = head->prev;
+ head->prev = newp;
+}
-/* Remove element from list. */
-static inline void
-__cds_list_del (struct cds_list_head *prev, struct cds_list_head *next)
+static inline
+void __cds_list_del(struct cds_list_head *prev, struct cds_list_head *next)
{
- next->prev = prev;
- prev->next = next;
+ next->prev = prev;
+ prev->next = next;
}
-/* Remove element from list. */
-static inline void
-cds_list_del (struct cds_list_head *elem)
+/*
+ * cds_list_del: Remove element from list.
+ * @elem: element to remove.
+ *
+ * Mutual exclusion against other updates and reads to list is required.
+ */
+static inline
+void cds_list_del(struct cds_list_head *elem)
{
- __cds_list_del (elem->prev, elem->next);
+ __cds_list_del(elem->prev, elem->next);
}
-/* Remove element from list, initializing the element's list pointers. */
-static inline void
-cds_list_del_init (struct cds_list_head *elem)
+/*
+ * cds_list_del_init: Remove element from list, init element.
+ * @elem: element to remove.
+ *
+ * Remove element from list, initializing the element's list pointers.
+ * Mutual exclusion against other updates and reads to list is required.
+ */
+static inline
+void cds_list_del_init(struct cds_list_head *elem)
{
cds_list_del(elem);
CDS_INIT_LIST_HEAD(elem);
}
-/* delete from list, add to another list as head */
-static inline void
-cds_list_move (struct cds_list_head *elem, struct cds_list_head *head)
+/*
+ * cds_list_move: Delete from list, add to another list as head.
+ * @elem: element to move.
+ * @head: head of list to move into.
+ *
+ * Mutual exclusion against other updates and reads to both lists is
+ * required.
+ */
+static inline
+void cds_list_move(struct cds_list_head *elem, struct cds_list_head *head)
{
- __cds_list_del (elem->prev, elem->next);
- cds_list_add (elem, head);
+ __cds_list_del(elem->prev, elem->next);
+ cds_list_add(elem, head);
}
-/* replace an old entry.
+/*
+ * cds_list_replace: replace an old entry.
+ * @old: old entry.
+ * @_new: new entry.
+ *
+ * Mutual exclusion against other updates and reads to list is required.
*/
-static inline void
-cds_list_replace(struct cds_list_head *old, struct cds_list_head *_new)
+static inline
+void cds_list_replace(struct cds_list_head *old, struct cds_list_head *_new)
{
_new->next = old->next;
_new->prev = old->prev;
@@ -112,75 +150,191 @@ cds_list_replace(struct cds_list_head *old, struct cds_list_head *_new)
_new->next->prev = _new;
}
-/* Join two lists. */
-static inline void
-cds_list_splice (struct cds_list_head *add, struct cds_list_head *head)
+/*
+ * cds_list_empty: returns whether list is empty
+ *
+ * Return nonzero if list is empty, zero otherwise.
+ * cds_list_empty() can be used on RCU lists. RCU read-side lock is not
+ * required.
+ */
+static inline
+int cds_list_empty(struct cds_list_head *head)
{
- /* Do nothing if the list which gets added is empty. */
- if (add != add->next)
- {
- add->next->prev = head;
- add->prev->next = head->next;
- head->next->prev = add->prev;
- head->next = add->next;
- }
+ return head == head->next;
}
-/* Get typed element from list at a given position. */
-#define cds_list_entry(ptr, type, member) \
- ((type *) ((char *) (ptr) - (unsigned long) (&((type *) 0)->member)))
-
+/*
+ * cds_list_splice: Join two lists.
+ * @add: head of list to merge into @head.
+ * @head: head of list to be merged into.
+ *
+ * Merge @add into @head.
+ * Mutual exclusion against other updates and reads to both lists is
+ * required.
+ */
+static inline
+void cds_list_splice(struct cds_list_head *add, struct cds_list_head *head)
+{
+ if (cds_list_empty(add))
+ return;
+ add->next->prev = head;
+ add->prev->next = head->next;
+ head->next->prev = add->prev;
+ head->next = add->next;
+}
-/* Get first entry from a list. */
-#define cds_list_first_entry(ptr, type, member) \
- cds_list_entry((ptr)->next, type, member)
+/*
+ * cds_list_replace_init: replace old entry by new one, init new entry.
+ * @old: old entry to replace.
+ * @_new: new entry to replace with.
+ *
+ * Mutual exclusion against other updates and reads to list is required.
+ */
+static inline
+void cds_list_replace_init(struct cds_list_head *old,
+ struct cds_list_head *_new)
+{
+ struct cds_list_head *head = old->next;
+ cds_list_del(old);
+ cds_list_add_tail(_new, head);
+ CDS_INIT_LIST_HEAD(old);
+}
+/*
+ * cds_list_entry: Get typed element from list at a given position.
+ * @ptr: node pointer (struct cds_list_head *).
+ * @type: type containing the list node field.
+ * @member: field name of list node within containing type.
+ */
+#define cds_list_entry(ptr, type, member) \
+ ((type *) ((char *) (ptr) - (unsigned long) (&((type *) 0)->member)))
-/* Iterate forward over the elements of the list. */
-#define cds_list_for_each(pos, head) \
- for (pos = (head)->next; pos != (head); pos = pos->next)
+/*
+ * cds_list_first_entry: Get first entry from a list.
+ * @head: list head (struct cds_list_head *).
+ * @type: type containing the list node field.
+ * @member: field name of list node within containing type.
+ *
+ * This should *not* be used on an empty list. Test for list emptiness
+ * before using @cds_list_first_entry.
+ */
+#define cds_list_first_entry(head, type, member) \
+ cds_list_entry((head)->next, type, member)
+/*
+ * cds_list_for_each: Iterate forward over the elements of the list.
+ * @pos: pointer to use as iterator (struct cds_list_head *).
+ * @head: list head (struct cds_list_head *).
+ *
+ * Mutual exclusion against updates to list is required.
+ * It is not safe to remove elements within loop iteration.
+ */
+#define cds_list_for_each(pos, head) \
+ for (pos = (head)->next; pos != (head); pos = pos->next)
-/* Iterate backward over the elements of the list. */
-#define cds_list_for_each_prev(pos, head) \
- for (pos = (head)->prev; pos != (head); pos = pos->prev)
+/*
+ * cds_list_for_each_safe: Iterate forward, safe against removal.
+ * @pos: pointer to use as iterator (struct cds_list_head *).
+ * @p: temporary pointer to store @pos copy (struct cds_list_head *).
+ * @head: list head (struct cds_list_head *).
+ *
+ * Mutual exclusion against updates to list is required.
+ * The list elements can be removed from the list within loop iteration.
+ */
+#define cds_list_for_each_safe(pos, p, head) \
+ for (pos = (head)->next, p = pos->next; \
+ pos != (head); \
+ pos = p; p = pos->next)
+/*
+ * cds_list_for_each_prev: Iterate backward over the elements of the list.
+ * @pos: pointer to use as iterator (struct cds_list_head *).
+ * @head: list head (struct cds_list_head *).
+ *
+ * Mutual exclusion against updates to list is required.
+ * It is not safe to remove elements within loop iteration.
+ */
+#define cds_list_for_each_prev(pos, head) \
+ for (pos = (head)->prev; pos != (head); pos = pos->prev)
-/* Iterate backwards over the elements list. The list elements can be
- removed from the list while doing this. */
-#define cds_list_for_each_prev_safe(pos, p, head) \
- for (pos = (head)->prev, p = pos->prev; \
- pos != (head); \
- pos = p, p = pos->prev)
+/*
+ * cds_list_for_each_prev_safe: Iterate backwards, safe against removal.
+ * @pos: pointer to use as iterator (struct cds_list_head *).
+ * @p: temporary pointer to store @pos copy (struct cds_list_head *).
+ * @head: list head (struct cds_list_head *).
+ *
+ * Mutual exclusion against updates to list is required.
+ * The list elements can be removed from the list within loop iteration.
+ */
+#define cds_list_for_each_prev_safe(pos, p, head) \
+ for (pos = (head)->prev, p = pos->prev; \
+ pos != (head); \
+ pos = p, p = pos->prev)
-#define cds_list_for_each_entry(pos, head, member) \
+/*
+ * cds_list_for_each_entry: Iterate forward over entries of the list.
+ * @pos: pointer to use as iterator (type of list node container).
+ * @head: list head (struct cds_list_head *).
+ * @member: field name of list node field within container type.
+ *
+ * Mutual exclusion against updates to list is required.
+ * It is not safe to remove elements within loop iteration.
+ */
+#define cds_list_for_each_entry(pos, head, member) \
for (pos = cds_list_entry((head)->next, __typeof__(*pos), member); \
- &pos->member != (head); \
- pos = cds_list_entry(pos->member.next, __typeof__(*pos), member))
+ &pos->member != (head); \
+ pos = cds_list_entry(pos->member.next, \
+ __typeof__(*pos), member))
-#define cds_list_for_each_entry_reverse(pos, head, member) \
+/*
+ * cds_list_for_each_entry_reverse: Iterate backwards over entries of the list.
+ * @pos: pointer to use as iterator (type of list node container).
+ * @head: list head (struct cds_list_head *).
+ * @member: field name of list node field within container type.
+ *
+ * Mutual exclusion against updates to list is required.
+ * It is not safe to remove elements within loop iteration.
+ */
+#define cds_list_for_each_entry_reverse(pos, head, member) \
for (pos = cds_list_entry((head)->prev, __typeof__(*pos), member); \
- &pos->member != (head); \
- pos = cds_list_entry(pos->member.prev, __typeof__(*pos), member))
+ &pos->member != (head); \
+ pos = cds_list_entry(pos->member.prev, \
+ __typeof__(*pos), member))
-#define cds_list_for_each_entry_safe(pos, p, head, member) \
+/*
+ * cds_list_for_each_entry_safe: Iterate forward, safe against removal.
+ * @pos: pointer to use as iterator (pointer to list node container).
+ * @p: temporary pointer to store @pos copy (pointer to list node container).
+ * @head: list head (struct cds_list_head *).
+ * @member: field name of list node field within container type.
+ *
+ * Mutual exclusion against updates to list is required.
+ * The list elements can be removed from the list within loop iteration.
+ */
+#define cds_list_for_each_entry_safe(pos, p, head, member) \
for (pos = cds_list_entry((head)->next, __typeof__(*pos), member), \
- p = cds_list_entry(pos->member.next, __typeof__(*pos), member); \
- &pos->member != (head); \
- pos = p, p = cds_list_entry(pos->member.next, __typeof__(*pos), member))
+ p = cds_list_entry(pos->member.next, \
+ __typeof__(*pos), member); \
+ &pos->member != (head); \
+ pos = p, p = cds_list_entry(pos->member.next, \
+ __typeof__(*pos), member))
-static inline int cds_list_empty(struct cds_list_head *head)
-{
- return head == head->next;
-}
-
-static inline void cds_list_replace_init(struct cds_list_head *old,
- struct cds_list_head *_new)
-{
- struct cds_list_head *head = old->next;
- cds_list_del(old);
- cds_list_add_tail(_new, head);
- CDS_INIT_LIST_HEAD(old);
-}
+/*
+ * cds_list_for_each_entry_reverse_safe: Iterate backwards, safe for removal.
+ * @pos: pointer to use as iterator (pointer to list node container).
+ * @p: temporary pointer to store @pos copy (pointer to list node container).
+ * @head: list head (struct cds_list_head *).
+ * @member: field name of list node field within container type.
+ *
+ * Mutual exclusion against updates to list is required.
+ * The list elements can be removed from the list within loop iteration.
+ */
+#define cds_list_for_each_entry_reverse_safe(pos, p, head, member) \
+ for (pos = cds_list_entry((head)->prev, __typeof__(*pos), member), \
+ p = cds_list_entry(pos->member.prev, \
+ __typeof__(*pos), member); \
+ &pos->member != (head); \
+ pos = p, p = cds_list_entry(pos->member.prev, \
+ __typeof__(*pos), member))
#endif /* _CDS_LIST_H */
diff --git a/urcu/rculist.h b/urcu/rculist.h
index 3604a96..cca64e5 100644
--- a/urcu/rculist.h
+++ b/urcu/rculist.h
@@ -5,7 +5,7 @@
*
* Copyright (C) 2009 Pierre-Marc Fournier
* Conversion to RCU list.
- * Copyright (C) 2010 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
+ * Copyright (C) 2010-2013 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -29,9 +29,31 @@
#include <urcu/arch.h>
#include <urcu-pointer.h>
-/* Add new element at the head of the list.
+/*
+ * This header contains a RCU-safe list API. It uses the urcu/list.h
+ * struct cds_list_head, but note that unless explicitly stated here,
+ * none of the urcu/list.h API members should be considered safe to use
+ * concurrently with RCU traversals. In addition to the explanation
+ * provided for each API member below, clarifications about relationship
+ * to some urcu/list.h API members follow.
+ *
+ * cds_list_empty() from urcu/list.h can be used on RCU lists. RCU
+ * read-side lock is not required.
+ *
+ * There is no list_del_init_rcu(), since there would need to be a
+ * grace-period between the "del" operation and the "init" operation.
+ */
+
+/*
+ * cds_list_add_rcu: Add new element at the head of the list.
+ * @newp: new element.
+ * @head: list head.
+ *
+ * Mutual exclusion against other updates to the list is required.
+ * RCU-safe for concurrent traversals.
*/
-static inline void cds_list_add_rcu(struct cds_list_head *newp, struct cds_list_head *head)
+static inline
+void cds_list_add_rcu(struct cds_list_head *newp, struct cds_list_head *head)
{
newp->next = head->next;
newp->prev = head;
@@ -40,9 +62,36 @@ static inline void cds_list_add_rcu(struct cds_list_head *newp, struct cds_list_
head->next = newp;
}
-/* replace an old entry atomically.
+/*
+ * cds_list_add_tail_rcu: Add new element at the tail of the list.
+ * @newp: new element.
+ * @head: list head.
+ *
+ * Mutual exclusion against other updates to the list is required.
+ * RCU-safe for concurrent traversals.
+ */
+static inline
+void cds_list_add_tail_rcu(struct cds_list_head *newp,
+ struct cds_list_head *head)
+{
+ newp->next = head;
+ newp->prev = head->prev;
+ cmm_smp_wmb();
+ head->prev = newp;
+ head->prev->next = newp;
+}
+
+/*
+ * cds_list_replace_rcu: replace an old entry atomically.
+ * @old: old entry.
+ * @_new: new entry.
+ *
+ * Replace old entry atomically with respect to concurrent readers.
+ * Mutual exclusion against other updates to the list is required.
+ * RCU-safe for concurrent traversals.
*/
-static inline void cds_list_replace_rcu(struct cds_list_head *old, struct cds_list_head *_new)
+static inline
+void cds_list_replace_rcu(struct cds_list_head *old, struct cds_list_head *_new)
{
_new->next = old->next;
_new->prev = old->prev;
@@ -50,29 +99,48 @@ static inline void cds_list_replace_rcu(struct cds_list_head *old, struct cds_li
_new->next->prev = _new;
}
-/* Remove element from list. */
-static inline void cds_list_del_rcu(struct cds_list_head *elem)
+/*
+ * cds_list_del_rcu: Remove element from list.
+ * @elem: new element.
+ *
+ * Mutual exclusion against other updates to the list is required.
+ * RCU-safe for concurrent traversals.
+ */
+static inline
+void cds_list_del_rcu(struct cds_list_head *elem)
{
elem->next->prev = elem->prev;
elem->prev->next = elem->next;
}
/*
- * Iteration through all elements of the list must be done while rcu_read_lock()
- * is held.
+ * cds_list_for_each_rcu: RCU-safe iteration over the elements of the list.
+ * @pos: iteration position (struct cds_list_head *).
+ * @head: list head (struct cds_list_head *).
+ *
+ * Iteration direction: forward.
+ * Iteration through all elements of the list must be done while
+ * rcu_read_lock() is held.
*/
+#define cds_list_for_each_rcu(pos, head) \
+ for (pos = rcu_dereference((head)->next); pos != (head); \
+ pos = rcu_dereference(pos->next))
-/* Iterate forward over the elements of the list. */
-#define cds_list_for_each_rcu(pos, head) \
- for (pos = rcu_dereference((head)->next); pos != (head); \
- pos = rcu_dereference(pos->next))
-
-
-/* Iterate through elements of the list.
+/*
+ * cds_list_for_each_entry_rcu: RCU-safe iteration over entries of the list.
+ * @pos: iteration position (pointer to entry type).
+ * @head: list head (struct cds_list_head *).
+ * @member: field name of the list node within entry type.
+ *
+ * Iteration direction: forward.
+ * Iteration through all elements of the list must be done while
+ * rcu_read_lock() is held.
*/
-#define cds_list_for_each_entry_rcu(pos, head, member) \
- for (pos = cds_list_entry(rcu_dereference((head)->next), __typeof__(*pos), member); \
+#define cds_list_for_each_entry_rcu(pos, head, member) \
+ for (pos = cds_list_entry(rcu_dereference((head)->next), \
+ __typeof__(*pos), member); \
&pos->member != (head); \
- pos = cds_list_entry(rcu_dereference(pos->member.next), __typeof__(*pos), member))
+ pos = cds_list_entry(rcu_dereference(pos->member.next), \
+ __typeof__(*pos), member))
#endif /* _URCU_RCULIST_H */
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 5+ messages in thread