[PATCH] State limits to safety of _safe iterators

[PATCH] State limits to safety of _safe iterators

[Paul E. McKenney]

The _safe list iterators make a blanket statement about how they are
safe against removal.  This patch, inspired by private conversations
with people who unwisely but perhaps understandably took this blanket
statement at its word, adds comments stating limits to this safety.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---

 list.h |   42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff -urpNa -X dontdiff linux-2.6.22/include/linux/list.h linux-2.6.22-safedoc/include/linux/list.h
--- linux-2.6.22/include/linux/list.h	2007-07-08 16:32:17.000000000 -0700
+++ linux-2.6.22-safedoc/include/linux/list.h	2007-09-12 17:45:38.000000000 -0700
@@ -472,6 +472,12 @@ static inline void list_splice_init_rcu(
  * @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.
+ *
+ * Please note that this is safe only against removal by the code in
+ * this iterator's body (either directly or via function calls).  In
+ * particular, it is not safe against removal by other tasks unless
+ * you use appropriate locking, RCU, or other synchronization
+ * mechanism.
  */
 #define list_for_each_safe(pos, n, head) \
 	for (pos = (head)->next, n = pos->next; pos != (head); \
@@ -542,6 +548,12 @@ static inline void list_splice_init_rcu(
  * @n:		another type * to use as temporary storage
  * @head:	the head for your list.
  * @member:	the name of the list_struct within the struct.
+ *
+ * Please note that this is safe only against removal by the code in
+ * this iterator's body (either directly or via function calls).  In
+ * particular, it is not safe against removal by other tasks unless
+ * you use appropriate locking, RCU, or other synchronization
+ * mechanism.
  */
 #define list_for_each_entry_safe(pos, n, head, member)			\
 	for (pos = list_entry((head)->next, typeof(*pos), member),	\
@@ -558,6 +570,12 @@ static inline void list_splice_init_rcu(
  *
  * Iterate over list of given type, continuing after current point,
  * safe against removal of list entry.
+ *
+ * Please note that this is safe only against removal by the code in
+ * this iterator's body (either directly or via function calls).  In
+ * particular, it is not safe against removal by other tasks unless
+ * you use appropriate locking, RCU, or other synchronization
+ * mechanism.
  */
 #define list_for_each_entry_safe_continue(pos, n, head, member) 		\
 	for (pos = list_entry(pos->member.next, typeof(*pos), member), 		\
@@ -574,6 +592,12 @@ static inline void list_splice_init_rcu(
  *
  * Iterate over list of given type from current point, safe against
  * removal of list entry.
+ *
+ * Please note that this is safe only against removal by the code in
+ * this iterator's body (either directly or via function calls).  In
+ * particular, it is not safe against removal by other tasks unless
+ * you use appropriate locking, RCU, or other synchronization
+ * mechanism.
  */
 #define list_for_each_entry_safe_from(pos, n, head, member) 			\
 	for (n = list_entry(pos->member.next, typeof(*pos), member);		\
@@ -589,6 +613,12 @@ static inline void list_splice_init_rcu(
  *
  * Iterate backwards over list of given type, safe against removal
  * of list entry.
+ *
+ * Please note that this is safe only against removal by the code in
+ * this iterator's body (either directly or via function calls).  In
+ * particular, it is not safe against removal by other tasks unless
+ * you use appropriate locking, RCU, or other synchronization
+ * mechanism.
  */
 #define list_for_each_entry_safe_reverse(pos, n, head, member)		\
 	for (pos = list_entry((head)->prev, typeof(*pos), member),	\
@@ -623,6 +653,12 @@ static inline void list_splice_init_rcu(
  *
  * Iterate over an rcu-protected list, safe against removal of list entry.
  *
+ * Please note that this is safe only against removal by the code in
+ * this iterator's body (either directly or via function calls).  In
+ * particular, it is not safe against removal by other tasks unless
+ * you use appropriate locking, RCU, or other synchronization
+ * mechanism.
+ *
  * This list-traversal primitive may safely run concurrently with
  * the _rcu list-mutation primitives such as list_add_rcu()
  * as long as the traversal is guarded by rcu_read_lock().
@@ -942,6 +978,12 @@ static inline void hlist_add_after_rcu(s
  * @n:		another &struct hlist_node to use as temporary storage
  * @head:	the head for your list.
  * @member:	the name of the hlist_node within the struct.
+ *
+ * Please note that this is safe only against removal by the code in
+ * this iterator's body (either directly or via function calls).  In
+ * particular, it is not safe against removal by other tasks unless
+ * you use appropriate locking, RCU, or other synchronization
+ * mechanism.
  */
 #define hlist_for_each_entry_safe(tpos, pos, n, head, member) 		 \
 	for (pos = (head)->first;					 \

Re: [PATCH] State limits to safety of _safe iterators

[Matthew Helsley]

On Wed, 2007-09-12 at 18:01 -0700, Paul E. McKenney wrote:
> The _safe list iterators make a blanket statement about how they are
> safe against removal.  This patch, inspired by private conversations
> with people who unwisely but perhaps understandably took this blanket
> statement at its word, adds comments stating limits to this safety.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
> 
>  list.h |   42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff -urpNa -X dontdiff linux-2.6.22/include/linux/list.h linux-2.6.22-safedoc/include/linux/list.h
> --- linux-2.6.22/include/linux/list.h	2007-07-08 16:32:17.000000000 -0700
> +++ linux-2.6.22-safedoc/include/linux/list.h	2007-09-12 17:45:38.000000000 -0700
> @@ -472,6 +472,12 @@ static inline void list_splice_init_rcu(
>   * @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.
> + *
> + * Please note that this is safe only against removal by the code in

I'm not trying to be snarky but how far should we go before expecting
folks to read the macros? Depending on the answer you may also want to
mention that without additional additional code it's safe only against
removal of the list element at pos.

<snip>

Cheers,
	-Matt Helsley

Re: [PATCH] State limits to safety of _safe iterators

[Paul E. McKenney]

On Thu, Sep 13, 2007 at 02:22:45AM -0700, Matthew Helsley wrote:
> On Wed, 2007-09-12 at 18:01 -0700, Paul E. McKenney wrote:
> > The _safe list iterators make a blanket statement about how they are
> > safe against removal.  This patch, inspired by private conversations
> > with people who unwisely but perhaps understandably took this blanket
> > statement at its word, adds comments stating limits to this safety.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > ---
> > 
> >  list.h |   42 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> > 
> > diff -urpNa -X dontdiff linux-2.6.22/include/linux/list.h linux-2.6.22-safedoc/include/linux/list.h
> > --- linux-2.6.22/include/linux/list.h	2007-07-08 16:32:17.000000000 -0700
> > +++ linux-2.6.22-safedoc/include/linux/list.h	2007-09-12 17:45:38.000000000 -0700
> > @@ -472,6 +472,12 @@ static inline void list_splice_init_rcu(
> >   * @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.
> > + *
> > + * Please note that this is safe only against removal by the code in
> 
> I'm not trying to be snarky but how far should we go before expecting
> folks to read the macros? Depending on the answer you may also want to
> mention that without additional additional code it's safe only against
> removal of the list element at pos.

Good question.  In fact, I would have agreed with you before coming
across people who in my experience are generally reasonably well clued
in who were confused about this.

							Thanx, Paul

Re: [PATCH] State limits to safety of _safe iterators

[Andrew Morton]

On Thu, 13 Sep 2007 08:21:07 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:

> On Thu, Sep 13, 2007 at 02:22:45AM -0700, Matthew Helsley wrote:
> > On Wed, 2007-09-12 at 18:01 -0700, Paul E. McKenney wrote:
> > > The _safe list iterators make a blanket statement about how they are
> > > safe against removal.  This patch, inspired by private conversations
> > > with people who unwisely but perhaps understandably took this blanket
> > > statement at its word, adds comments stating limits to this safety.
> > > 
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > ---
> > > 
> > >  list.h |   42 ++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 42 insertions(+)
> > > 
> > > diff -urpNa -X dontdiff linux-2.6.22/include/linux/list.h linux-2.6.22-safedoc/include/linux/list.h
> > > --- linux-2.6.22/include/linux/list.h	2007-07-08 16:32:17.000000000 -0700
> > > +++ linux-2.6.22-safedoc/include/linux/list.h	2007-09-12 17:45:38.000000000 -0700
> > > @@ -472,6 +472,12 @@ static inline void list_splice_init_rcu(
> > >   * @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.
> > > + *
> > > + * Please note that this is safe only against removal by the code in
> > 
> > I'm not trying to be snarky but how far should we go before expecting
> > folks to read the macros? Depending on the answer you may also want to
> > mention that without additional additional code it's safe only against
> > removal of the list element at pos.
> 
> Good question.  In fact, I would have agreed with you before coming
> across people who in my experience are generally reasonably well clued
> in who were confused about this.
> 

hmm, yes, I must say, one would need to be fairly thick to expect a little
helper macro to protect you from activity on other CPUs.

Re: [PATCH] State limits to safety of _safe iterators

[Paul E. McKenney]

On Fri, Sep 14, 2007 at 05:21:51PM -0700, Andrew Morton wrote:
> On Thu, 13 Sep 2007 08:21:07 -0700
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > On Thu, Sep 13, 2007 at 02:22:45AM -0700, Matthew Helsley wrote:
> > > On Wed, 2007-09-12 at 18:01 -0700, Paul E. McKenney wrote:
> > > > The _safe list iterators make a blanket statement about how they are
> > > > safe against removal.  This patch, inspired by private conversations
> > > > with people who unwisely but perhaps understandably took this blanket
> > > > statement at its word, adds comments stating limits to this safety.
> > > > 
> > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > > ---
> > > > 
> > > >  list.h |   42 ++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 42 insertions(+)
> > > > 
> > > > diff -urpNa -X dontdiff linux-2.6.22/include/linux/list.h linux-2.6.22-safedoc/include/linux/list.h
> > > > --- linux-2.6.22/include/linux/list.h	2007-07-08 16:32:17.000000000 -0700
> > > > +++ linux-2.6.22-safedoc/include/linux/list.h	2007-09-12 17:45:38.000000000 -0700
> > > > @@ -472,6 +472,12 @@ static inline void list_splice_init_rcu(
> > > >   * @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.
> > > > + *
> > > > + * Please note that this is safe only against removal by the code in
> > > 
> > > I'm not trying to be snarky but how far should we go before expecting
> > > folks to read the macros? Depending on the answer you may also want to
> > > mention that without additional additional code it's safe only against
> > > removal of the list element at pos.
> > 
> > Good question.  In fact, I would have agreed with you before coming
> > across people who in my experience are generally reasonably well clued
> > in who were confused about this.
> > 
> 
> hmm, yes, I must say, one would need to be fairly thick to expect a little
> helper macro to protect you from activity on other CPUs.

Or distracted or tired or whatever.

In any case, I don't feel all that strongly about this, so if the general
consensus is that it is not required, no problem...

							Thanx, Paul