On Fri, Oct 14, 2005 at 07:03:25PM -0700, Paul E. McKenney wrote:
>
> > diff --git a/include/linux/list.h b/include/linux/list.h
> > --- a/include/linux/list.h
> > +++ b/include/linux/list.h
> > @@ -442,12 +442,15 @@ static inline void list_splice_init(stru
> > * as long as the traversal is guarded by rcu_read_lock().
> > */
> > #define list_for_each_rcu(pos, head) \
> > - for (pos = (head)->next; prefetch(pos->next), pos != (head); \
> > - pos = rcu_dereference(pos->next))
> > + for (pos = (head)->next; \
> > + pos = rcu_dereference(pos), \
> > + prefetch(pos->next), pos != (head); \
> > + pos = pos->next)
>
> Why not something like the following? Seems a bit simpler to me.
>
> #define list_for_each_rcu(pos, head) \
> for (pos = rcu_dereference((head)->next); \
> prefetch(pos->next), pos != (head); \
> pos = rcu_dereference(pos->next))
In this case your version is indeed more concise. However, in most of
the other for_each macros having it in the loop conditional looks more
natural.
So in order to be consistent throughout list.h, I'd like to keep the
rcu_dereference in the loop conditional.
> > @@ -492,8 +496,10 @@ static inline void list_splice_init(stru
> > * as long as the traversal is guarded by rcu_read_lock().
> > */
> > #define list_for_each_continue_rcu(pos, head) \
> > - for ((pos) = (pos)->next; prefetch((pos)->next), (pos) != (head); \
> > - (pos) = rcu_dereference((pos)->next))
> > + for ((pos) = (pos)->next; \
> > + (pos) = rcu_dereference((pos)), \
> > + prefetch((pos)->next), (pos) != (head); \
> > + (pos) = (pos)->next)
>
> The above hurts my head -- childhood trauma due to having to use a
> FORTRAN compiler that required "I=I" at odd intervals in order to
> generate correct code... How about the following?
>
> #define list_for_each_continue_rcu(pos, head) \
> for ((pos) = (pos)->next; \
> prefetch(rcu_dereference(pos)->next), (pos) != (head); \
> (pos) = (pos)->next)
I chose to keep it out of prefetch because normally the argument to
prefetch does not have any side-effects. Even though today's prefetch
is an inline function which does respect side-effects, there is always
a possibility that someone somewhere might decide to implement prefetch
as a macro.
Besides, the expression
i = foo(i)
where foo has side-effects is pretty normal.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
