lkml.org 
[lkml]   [2013]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v2] rcu: fix a race in hlist_nulls_for_each_entry_rcu macro
    Hi, Paul!

    > On 25.05.2013 15:37, Paul E. McKenney wrote:
    >> Again, I believe that your retry logic needs to extend back into the
    >> calling function for your some_func() example above.

    And what do you think about the following approach (diff below)?

    It seems to me, it's enough clear (especially with good accompanying comments)
    and produces a good binary code (without significant overhead).
    Also, we will remove a hidden reef in using rcu-protected (h)list traverses with restarts.

    diff --git a/include/linux/rculist.h b/include/linux/rculist.h
    index 6f95e24..8d8b1eb 100644
    --- a/include/linux/rculist.h
    +++ b/include/linux/rculist.h
    @@ -269,7 +269,8 @@ static inline void list_splice_init_rcu(struct list_head *list,
    * as long as the traversal is guarded by rcu_read_lock().
    */
    #define list_for_each_entry_rcu(pos, head, member) \
    - for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
    + for (ACCESS_ONCE(*(head)), \
    + pos = list_entry_rcu((head)->next, typeof(*pos), member); \
    &pos->member != (head); \
    pos = list_entry_rcu(pos->member.next, typeof(*pos), member))

    @@ -459,7 +460,8 @@ static inline void hlist_add_after_rcu(struct hlist_node *prev,
    * as long as the traversal is guarded by rcu_read_lock().
    */
    #define hlist_for_each_entry_rcu(tpos, pos, head, member) \
    - for (pos = rcu_dereference_raw(hlist_first_rcu(head)); \
    + for (ACCESS_ONCE(*(head)), \
    + pos = rcu_dereference_raw(hlist_first_rcu(head)); \
    pos && \
    ({ tpos = hlist_entry(pos, typeof(*tpos), member); 1; }); \
    pos = rcu_dereference_raw(hlist_next_rcu(pos)))
    @@ -476,7 +478,7 @@ static inline void hlist_add_after_rcu(struct hlist_node *prev,
    * as long as the traversal is guarded by rcu_read_lock().
    */
    #define hlist_for_each_entry_rcu_bh(tpos, pos, head, member) \
    - for (pos = rcu_dereference_bh((head)->first); \
    + for (ACCESS_ONCE(*(head)), pos = rcu_dereference_bh((head)->first); \
    pos && \
    ({ tpos = hlist_entry(pos, typeof(*tpos), member); 1; }); \
    pos = rcu_dereference_bh(pos->next))
    diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
    index 2ae1371..4af5ee5 100644
    --- a/include/linux/rculist_nulls.h
    +++ b/include/linux/rculist_nulls.h
    @@ -107,7 +107,8 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
    *
    */
    #define hlist_nulls_for_each_entry_rcu(tpos, pos, head, member) \
    - for (pos = rcu_dereference_raw(hlist_nulls_first_rcu(head)); \
    + for (ACCESS_ONCE(*(head)), \
    + pos = rcu_dereference_raw(hlist_nulls_first_rcu(head)); \
    (!is_a_nulls(pos)) && \
    ({ tpos = hlist_nulls_entry(pos, typeof(*tpos), member); 1; }); \
    pos = rcu_dereference_raw(hlist_nulls_next_rcu(pos)))
    ---
    Thanks!

    Regards,
    Roman



    \
     
     \ /
      Last update: 2013-05-27 20:41    [W:2.618 / U:0.232 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site