bonding: simplify and use RCU protection for 3ad xmit path

The commit 278b20837511776dc9d5f6ee1c7fabd5479838bb
(bonding: initial RCU conversion) has convert the roundrobin, active-backup, broadcast and xor xmit path to rcu protection, the performance will be better for these mode, so this time, convert xmit path for 3ad mode.

Suggested-by Nikolay Aleksandrov <[email protected]>

Suggested-by Veaceslav Falico <[email protected]>

Signed-off-by Ding Tianhong <[email protected]>

Signed-off-by Wang Yufen <[email protected]>

Cc Nikolay Aleksandrov <[email protected]>

Cc Veaceslav Falico <[email protected]>

---
 drivers/net/bonding/bond_3ad.c | 32 ++++++++++++++------------------
 drivers/net/bonding/bonding.h  | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 19 deletions(-)

drivers/net/bonding/bond_3ad.c +14 -18

index 0d8f427..13f1deb 100644
@@ -143,7 +143,7 @@ static inline struct bonding *__get_bond_by_port(struct port *port)
  */
 static inline struct port *__get_first_port(struct bonding *bond)
 {
-	struct slave *first_slave = bond_first_slave(bond);
+	struct slave *first_slave = bond_first_slave_rcu(bond);
 
 	return first_slave ? &(SLAVE_AD_INFO(first_slave).port) : NULL;
 }

@@ -163,7 +163,7 @@ static inline struct port *__get_next_port(struct port *port)
 	// If there's no bond for this port, or this is the last slave
 	if (bond == NULL)
 		return NULL;
-	slave_next = bond_next_slave(bond, slave);
+	slave_next = bond_next_slave_rcu(bond, slave);
 	if (!slave_next || bond_is_first_slave(bond, slave_next))
 		return NULL;
 

@@ -2417,16 +2417,14 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
 
 int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 {
-	struct slave *slave, *start_at;
 	struct bonding *bond = netdev_priv(dev);
+	struct slave *slave;
 	int slave_agg_no;
 	int slaves_in_agg;
 	int agg_id;
-	int i;
 	struct ad_info ad_info;
 	int res = 1;
 
-	read_lock(&bond->lock);
 	if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
 		pr_debug("%s: Error: __bond_3ad_get_active_agg_info failed\n",
 			 dev->name);

@@ -2444,13 +2442,17 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 
 	slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg);
 
-	bond_for_each_slave(bond, slave) {
+	bond_for_each_slave_rcu(bond, slave) {
 		struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
 
 		if (agg && (agg->aggregator_identifier == agg_id)) {
-			slave_agg_no--;
-			if (slave_agg_no < 0)
-				break;
+			if (--slave_agg_no < 0) {
+				if (SLAVE_IS_OK(slave)) {
+					res = bond_dev_queue_xmit(bond,
+						skb, slave->dev);
+					goto out;
+				}
+			}
 		}
 	}
 

@@ -2460,23 +2462,17 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 		goto out;
 	}
 
-	start_at = slave;
-
-	bond_for_each_slave_from(bond, slave, i, start_at) {
-		int slave_agg_id = 0;
+	bond_for_each_slave_rcu(bond, slave) {
 		struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
 
-		if (agg)
-			slave_agg_id = agg->aggregator_identifier;
-
-		if (SLAVE_IS_OK(slave) && agg && (slave_agg_id == agg_id)) {
+		if (SLAVE_IS_OK(slave) && agg &&
+			agg->aggregator_identifier == agg_id) {
 			res = bond_dev_queue_xmit(bond, skb, slave->dev);
 			break;
 		}
 	}
 
 out:
-	read_unlock(&bond->lock);
 	if (res) {
 		/* no suitable interface, frame not sent */
 		kfree_skb(skb);

drivers/net/bonding/bonding.h +13 -1

index f7ab161..419161d 100644
@@ -74,13 +74,35 @@
 /* slave list primitives */
 #define bond_to_slave(ptr) list_entry(ptr, struct slave, list)
 
+/* slave list primitives, Caller must hold rcu_read_lock */
+#define bond_to_slave_rcu(ptr) list_entry_rcu(ptr, struct slave, list)
+
+/* bond_is_empty return NULL if slave list is empty*/
+#define bond_is_empty(bond) \
+	(list_empty(&(bond)->slave_list))
+
+/* bond_is_empty_rcu return NULL if slave list is empty*/
+#define bond_is_empty_rcu(bond) \
+	(!list_first_or_null_rcu(&(bond)->slave_list, struct slave, list))
+
 /* IMPORTANT: bond_first/last_slave can return NULL in case of an empty list */
 #define bond_first_slave(bond) \
 	list_first_entry_or_null(&(bond)->slave_list, struct slave, list)
 #define bond_last_slave(bond) \
-	(list_empty(&(bond)->slave_list) ? NULL : \
+	(bond_is_empty(bond) ? NULL : \
 					   bond_to_slave((bond)->slave_list.prev))
 
+/**

drivers/net/bonding/bonding.h +4 -0

+#define bond_prev_slave_rcu(bond, pos) \
+	(bond_is_first_slave(bond, pos) ? bond_last_slave_rcu(bond) : \
+					  bond_to_slave_rcu((pos)->list.prev))
+
 /**
  * bond_for_each_slave_from - iterate the slaves list from a starting point
  * @bond:	the bond holding this list.

On Fri, Sep 06, 2013 at 03:28:07PM +0800, Ding Tianhong wrote:

(Also, mind the "likely(__ptr != __next)" - usually, and bonding is a very good example, we always have at least one element in the list.)

This way, I'd recommend you to do bond_last_slave_rcu() the same way as here - you, though, might omit saving the slave_list pointer, it's not needed in case of bonding AFAIK. Something like that (I'm writing it in my mail editor - so it's only for the reference):

#define bond_last_slave_rcu(bond) \
    ({struct list_head *__slave_ptr = list_next_rcu(&bond->slave_list); \
      likely(__slave_ptr != &bond->slave_list) ? \
        bond_to_slave_rcu(__slave_ptr) : NULL;})

Or, even better (from my POV), add a generic macro to rculist.h (again, didn't even compile it) - it can be used later on:

include/linux/rculist.h +1 -0

index f4b1001..37b49d1 100644
@@ -23,6 +23,7 @@
  * way, we must not access it directly
  */
 #define list_next_rcu(list)    (*((struct list_head __rcu
 **)(&(list)->next)))
+#define list_prev_rcu(list)    (*((struct list_head __rcu
 **)(&(list)->prev)))
 

/*
* Insert a new entry between two known consecutive entries.

include/linux/rculist.h +0 -0

 /**
  * list_for_each_entry_rcu    -    iterate over rcu list of given type

* @pos: the type * to use as a loop cursor.
------- END OF PATCH ------

Anyway, it's up to you.

Hope that helps.

Quoted 1 line · Veaceslav Falico

On Fri, Sep 06, 2013 at 03:28:07PM +0800, Ding Tianhong wrote:

Quoted 19 lines

include/linux/rculist.h +1 -0

index f4b1001..37b49d1 100644
@@ -23,6 +23,7 @@
  * way, we must not access it directly
  */
 #define list_next_rcu(list)    (*((struct list_head __rcu
 **)(&(list)->next)))
+#define list_prev_rcu(list)    (*((struct list_head __rcu
 **)(&(list)->prev)))
 

/*
* Insert a new entry between two known consecutive entries.

include/linux/rculist.h +0 -0

@@ -271,6 +272,12 @@ static inline void list_splice_init_rcu(struct
 list_head *list,
       likely(__ptr != __next) ? container_of(__next, type, member) : NULL; \
     })
 

include/linux/rculist.h static inline void list_splice_init_rcu(struct list_head *list, · line 280

Quoted 6 lines

include/linux/rculist.h +6 -0

+#define list_last_or_null_rcu(ptr, type, member) \
+    ({struct list_head *__ptr = (ptr); \
+      struct list_head __rcu *__last = list_prev_rcu(__ptr); \
+      likely(__ptr != __last) ? container_of(__prev, type, member) : NULL; \
+    })
+

Hi,
Actually I don't think you can dereference ->prev and use the standard list_del_rcu because it guarantees only the ->next ptr will be valid and ->prev is set to LIST_POISON2.
IMO, you'll need something like this: https://lkml.org/lkml/2012/7/25/193
with the bidir_del and all that.

But in any case I complete agree with Veaceslav here. Read all the documentation carefully :-)

Cheers,
Nik

Quoted 8 lines

include/linux/rculist.h +0 -0

 /**
  * list_for_each_entry_rcu    -    iterate over rcu list of given type

* @pos: the type * to use as a loop cursor.
------- END OF PATCH ------

Anyway, it's up to you.

Hope that helps.

Quoted 3 lines · Nikolay Aleksandrov

Quoted 1 line · Veaceslav Falico

On Fri, Sep 06, 2013 at 03:28:07PM +0800, Ding Tianhong wrote:

...snip...

Quoted 19 lines

Quoted 19 lines

include/linux/rculist.h +1 -0

index f4b1001..37b49d1 100644
@@ -23,6 +23,7 @@
 * way, we must not access it directly
 */
 #define list_next_rcu(list) (*((struct list_head __rcu
 **)(&(list)->next)))
+#define list_prev_rcu(list) (*((struct list_head __rcu
 **)(&(list)->prev)))
 

/*
* Insert a new entry between two known consecutive entries.

include/linux/rculist.h +0 -0

@@ -271,6 +272,12 @@ static inline void list_splice_init_rcu(struct
 list_head *list,
 likely(__ptr != __next) ? container_of(__next, type, member) : NULL; \
 })
 

Quoted 13 lines

include/linux/rculist.h static inline void list_splice_init_rcu(struct list_head *list, · line 280

Quoted 7 lines

include/linux/rculist.h +6 -0

+#define list_last_or_null_rcu(ptr, type, member) \
+ ({struct list_head *__ptr = (ptr); \
+ struct list_head __rcu *__last = list_prev_rcu(__ptr); \
+ likely(__ptr != __last) ? container_of(__prev, type, member) : 
 NULL; \
+ })
+

Hi,
Actually I don't think you can dereference ->prev and use the standard list_del_rcu because it guarantees only the ->next ptr will be valid and ->prev is set to LIST_POISON2.
IMO, you'll need something like this: https://lkml.org/lkml/2012/7/25/193
with the bidir_del and all that.

Yeah, right, my bad - we can rely only on the ->next pointer, indeed, missed that part. RCU is hard :).

So it'll be a lot harder to implement bond_last_slave_rcu() in a 'straightforward' approach.

I'd rather go in the opposite direction here - i.e. drop the 'reverse' traversal completely, and all the use cases for bond_last_slave_rcu(). I've got some patches already - http://patchwork.ozlabs.org/patch/272076/ doing that, and hopefully will remove the whole 'backword' traversal completely in the future.

Quoted 7 lines

But in any case I complete agree with Veaceslav here. Read all the documentation carefully :-)

Cheers,
Nik

Quoted 9 lines

Quoted 8 lines

include/linux/rculist.h +0 -0

 /**

* list_for_each_entry_rcu - iterate over rcu list of given type
* @pos: the type * to use as a loop cursor.
------- END OF PATCH ------

Anyway, it's up to you.

Hope that helps.

Signature · 3 lines

To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 5 lines

Signature · 3 lines

To unsubscribe from this list: send the line "unsubsc ribe netdev" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

.

On 2013/9/8 14:05, Ding Tianhong wrote:

Hi Veaceslav and Nik:

please take a moment to reveiw the function just modify for bond_XXX_rcu, and give me some advice. thanks for the help again.:)

+
#define bond_is_first_slave(bond, pos) ((pos)->list.prev == &(bond)->slave_list) #define bond_is_last_slave(bond, pos) ((pos)->list.next == &(bond)->slave_list)

(quoted diff) +0 -0

@@ -93,6 +117,29 @@
   (bond_is_first_slave(bond, pos) ? bond_last_slave(bond) : \
                     bond_to_slave((pos)->list.prev))
 

(quoted diff) +7 -4

+
 
 
-#define bond_for_each_slave_from(bond, pos, cnt, start) \
-   for (cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \
-        cnt++, pos = bond_next_slave(bond, pos))
-
+#define bond_for_each_slave_from(bond, pos, start) \
+   for (pos = start; pos; (pos = bond_next_slave(bond, pos)) != start ? \
+       (pos) : (pos = NULL))
+
+#define bond_for_each_slave_from_rcu(bond, pos, start) \
+   for ({struct list_head *__start = &(start)->list; \

+

Best regards
Ding

Quoted 9 lines

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubsc ribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

.

Quoted 7 lines · Ding Tianhong

On 2013/9/8 14:05, Ding Tianhong wrote:

Hi Veaceslav and Nik:

please take a moment to reveiw the function just modify for bond_XXX_rcu, and give me some advice. thanks for the help again.:)

Review

Quoted 8 lines

(quoted diff) +8 -0

+#define bond_first_slave_rcu(bond) \
+ list_first_or_null_rcu(&(bond)->slave_list, struct slave, list);
+#define bond_last_slave_rcu(bond) \
+ ({struct list_head *__slave_list = &(bond)->slave_list; \
+ struct list_head __rcu *__prev = \
+ (*((struct list_head __rcu **)(&(__slave_list)->prev)));\
+ likely(__slave_list != __prev) ? \
+ container_of(__prev, struct slave, list) : NULL;})

Please take a look at Nikolay's reply to my RCU email -
http://www.spinics.net/lists/netdev/msg249805.html . And mine also, to his email. In short - RCU doesn't guarantee ->prev, so better take the approach of eliminating bond_last/prev_slave completely.

Quoted 8 lines

+
#define bond_is_first_slave(bond, pos) ((pos)->list.prev == &(bond)->slave_list) #define bond_is_last_slave(bond, pos) ((pos)->list.next == &(bond)->slave_list)

(quoted diff) +0 -0

@@ -93,6 +117,29 @@
 (bond_is_first_slave(bond, pos) ? bond_last_slave(bond) : \
 bond_to_slave((pos)->list.prev))
 

Quoted 11 lines

(quoted diff) +1 -0

+/* Since bond_first/last_slave_rcu can return NULL, these can return 

NULL too */
+#define bond_next_slave_rcu(bond, pos) \
+ ({struct list_head *__slave_list = &(bond)->slave_list; \
+ struct list_head __rcu *__next = list_next_rcu(__slave_list); \
+ struct list_head *__pos_list = &(pos)->list; \
+ struct list_head __rcu *__pos_next = list_next_rcu(__pos_list); \
+ likely(__pos_next != __slave_list) ? \
+ container_of(__pos_next, struct slave, list) : \
+ container_of(__next, struct slave, list); \
+ })

Nice, but can be shortened - we know that pos won't go away.

Quoted 9 lines

(quoted diff) +7 -0

+
+#define bond_prev_slave_rcu(bond, pos) \
+ ({struct list_head *__slave_list = &(bond)->slave_list; \
+ struct list_head __rcu *__prev = \
+ (*((struct list_head __rcu **)(&(__slave_list)->prev)));\
+ struct list_head *__pos_list = &(pos)->list; \
+ struct list_head __rcu *__pos_prev = (__pos_list->prev 

!=LIST_POISON2) ? \

Quoted 6 lines

+ (*((struct list_head __rcu **)(&(__pos_list)->prev))) : NULL; \
+ likely(__pos_prev != __slave_list) ? \
+ ((__pos_prev) ? list_entry_rcu(__pos_prev, struct slave, list) :
NULL;) : \
+ (list_entry_rcu(__prev, struct slave, list)); \
+ })

Same remark as above about prev.

Quoted 3 lines

+

Review

Quoted 9 lines

(quoted diff) +5 -4

-#define bond_for_each_slave_from(bond, pos, cnt, start) \
- for (cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \
- cnt++, pos = bond_next_slave(bond, pos))
-
+#define bond_for_each_slave_from(bond, pos, start) \
+ for (pos = start; pos; (pos = bond_next_slave(bond, pos)) != start ? \
+ (pos) : (pos = NULL))
+
+#define bond_for_each_slave_from_rcu(bond, pos, start) \

Quoted 4 lines

+ for ({struct list_head *__start = &(start)->list; \
+ struct list_head *__slave_list = &(bond)->slave_list; \
+ pos = list_entry_rcu(__start, struct slave, list);}; \
+ pos; \

Quoted 3 lines

+ {struct list_head __rcu *__next = list_next_rcu(pos->next); \
+ __next != __slave_list ? \
+ __next : __next = list_next_rcu(__next->next); \

Quoted 4 lines

+ __next != __start ? \
+ pos = list_entry_rcu(__next, struct slave, list) : \
+ pos = NULL; \

Quoted 1 line

+ })

Jeez, I don't even want to review it. It's too complex and too hard to maintain, even if it works. Can you please make something shorter/easier to understand?

Quoted 17 lines

+

Best regards
Ding

Quoted 9 lines

Quoted 5 lines

Signature · 3 lines

To unsubscribe from this list: send the line "unsubsc ribe netdev" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

.

Signature · 3 lines

To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 8 lines · Veaceslav Falico

Quoted 7 lines · Ding Tianhong

On 2013/9/8 14:05, Ding Tianhong wrote:

Hi Veaceslav and Nik:

please take a moment to reveiw the function just modify for bond_XXX_rcu, and give me some advice. thanks for the help again.:)

Quoted 14 lines

Review

Quoted 8 lines

(quoted diff) +8 -0

+#define bond_first_slave_rcu(bond) \
+ list_first_or_null_rcu(&(bond)->slave_list, struct slave, list);
+#define bond_last_slave_rcu(bond) \
+ ({struct list_head *__slave_list = &(bond)->slave_list; \
+ struct list_head __rcu *__prev = \
+ (*((struct list_head __rcu **)(&(__slave_list)->prev)));\
+ likely(__slave_list != __prev) ? \
+ container_of(__prev, struct slave, list) : NULL;})

Please take a look at Nikolay's reply to my RCU email -
http://www.spinics.net/lists/netdev/msg249805.html . And mine also, to his email. In short - RCU doesn't guarantee ->prev, so better take the approach of eliminating bond_last/prev_slave completely.

yes, I see the message, the list_del_rcu will make the slave->list
->prev = LIST_POISON2,
the bond->slave_list will not be set to the messae, the prev will point a slave->list or itself, so I think it will be ok here, please correct me if I miss something.

Best Regards
Ding

Quoted 8 lines

Quoted 8 lines

+
#define bond_is_first_slave(bond, pos) ((pos)->list.prev == &(bond)->slave_list) #define bond_is_last_slave(bond, pos) ((pos)->list.next == &(bond)->slave_list)

(quoted diff) +0 -0

@@ -93,6 +117,29 @@
 (bond_is_first_slave(bond, pos) ? bond_last_slave(bond) : \
 bond_to_slave((pos)->list.prev))
 

Quoted 13 lines

Quoted 11 lines

(quoted diff) +1 -0

+/* Since bond_first/last_slave_rcu can return NULL, these can 

return NULL too */
+#define bond_next_slave_rcu(bond, pos) \
+ ({struct list_head *__slave_list = &(bond)->slave_list; \
+ struct list_head __rcu *__next = list_next_rcu(__slave_list); \
+ struct list_head *__pos_list = &(pos)->list; \
+ struct list_head __rcu *__pos_next = list_next_rcu(__pos_list); \
+ likely(__pos_next != __slave_list) ? \
+ container_of(__pos_next, struct slave, list) : \
+ container_of(__next, struct slave, list); \
+ })

Nice, but can be shortened - we know that pos won't go away.

OK, clean it soon.

Quoted 9 lines

Quoted 9 lines

(quoted diff) +7 -0

+
+#define bond_prev_slave_rcu(bond, pos) \
+ ({struct list_head *__slave_list = &(bond)->slave_list; \
+ struct list_head __rcu *__prev = \
+ (*((struct list_head __rcu **)(&(__slave_list)->prev)));\
+ struct list_head *__pos_list = &(pos)->list; \
+ struct list_head __rcu *__pos_prev = (__pos_list->prev 

!=LIST_POISON2) ? \

yes, the pos->list will be set to LIST_POISON2 by list_del_rcu, so I add a check for it, But take the approach of eliminating bond_last/prev_slave completely is a wise decision, I agree.

Quoted 12 lines

Quoted 6 lines

+ (*((struct list_head __rcu **)(&(__pos_list)->prev))) : NULL; \
+ likely(__pos_prev != __slave_list) ? \
+ ((__pos_prev) ? list_entry_rcu(__pos_prev, struct slave, list) : NULL;) : \
+ (list_entry_rcu(__prev, struct slave, list)); \
+ })

Same remark as above about prev.

Quoted 3 lines

+

Quoted 9 lines

Review

Quoted 9 lines

(quoted diff) +5 -4

-#define bond_for_each_slave_from(bond, pos, cnt, start) \
- for (cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \
- cnt++, pos = bond_next_slave(bond, pos))
-
+#define bond_for_each_slave_from(bond, pos, start) \
+ for (pos = start; pos; (pos = bond_next_slave(bond, pos)) != start ? \
+ (pos) : (pos = NULL))
+
+#define bond_for_each_slave_from_rcu(bond, pos, start) \

yes, it is a little tedious. I think it could be more easier and shorter.

Quoted 4 lines

Quoted 4 lines

+ for ({struct list_head *__start = &(start)->list; \
+ struct list_head *__slave_list = &(bond)->slave_list; \
+ pos = list_entry_rcu(__start, struct slave, list);}; \
+ pos; \

the only way to get out of the loop is that pos is NULL.

Quoted 3 lines

Quoted 3 lines

+ {struct list_head __rcu *__next = list_next_rcu(pos->next); \
+ __next != __slave_list ? \
+ __next : __next = list_next_rcu(__next->next); \

first, check whether the pos->next is the last one in the slave_list, if it does, get the first slave of the bond->slave_list.

Quoted 4 lines

Quoted 4 lines

+ __next != __start ? \
+ pos = list_entry_rcu(__next, struct slave, list) : \
+ pos = NULL; \

second, check whether the pos is reach the start, if not, continue, otherwise, the pos will be set to NULL, so break the loop.

Quoted 6 lines

Quoted 1 line

+ })

Jeez, I don't even want to review it. It's too complex and too hard to maintain, even if it works. Can you please make something shorter/easier to understand?

Best Regards.
Ding

Quoted 22 lines

Quoted 17 lines

+

Best regards
Ding

Quoted 9 lines

Quoted 5 lines

Signature · 3 lines

To unsubscribe from this list: send the line "unsubsc ribe netdev" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

.

Signature · 3 lines

To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 3 lines · Nikolay Aleksandrov

Quoted 1 line · Veaceslav Falico

On Fri, Sep 06, 2013 at 03:28:07PM +0800, Ding Tianhong wrote:

...snip...

Quoted 19 lines

Quoted 19 lines

include/linux/rculist.h +1 -0

index f4b1001..37b49d1 100644
@@ -23,6 +23,7 @@
 * way, we must not access it directly
 */
 #define list_next_rcu(list)    (*((struct list_head __rcu
 **)(&(list)->next)))
+#define list_prev_rcu(list)    (*((struct list_head __rcu
 **)(&(list)->prev)))
 

/*
* Insert a new entry between two known consecutive entries.

include/linux/rculist.h +0 -0

@@ -271,6 +272,12 @@ static inline void list_splice_init_rcu(struct
 list_head *list,
      likely(__ptr != __next) ? container_of(__next, type, member) : NULL; \
    })
 

Quoted 12 lines

include/linux/rculist.h static inline void list_splice_init_rcu(struct list_head *list, · line 280

Quoted 6 lines

include/linux/rculist.h +6 -0

+#define list_last_or_null_rcu(ptr, type, member) \
+    ({struct list_head *__ptr = (ptr); \
+      struct list_head __rcu *__last = list_prev_rcu(__ptr); \
+      likely(__ptr != __last) ? container_of(__prev, type, member) : NULL; \
+    })
+

Hi,
Actually I don't think you can dereference ->prev and use the standard list_del_rcu because it guarantees only the ->next ptr will be valid and ->prev is set to LIST_POISON2.
IMO, you'll need something like this: https://lkml.org/lkml/2012/7/25/193
with the bidir_del and all that.

Yeah, right, my bad - we can rely only on the ->next pointer, indeed, missed that part. RCU is hard :).

So it'll be a lot harder to implement bond_last_slave_rcu() in a 'straightforward' approach.

I'd rather go in the opposite direction here - i.e. drop the 'reverse' traversal completely, and all the use cases for bond_last_slave_rcu(). I've got some patches already - http://patchwork.ozlabs.org/patch/272076/ doing that, and hopefully will remove the whole 'backword' traversal completely in the future.

Quoted 15 lines

But in any case I complete agree with Veaceslav here. Read all the documentation carefully :-)

Cheers,
Nik

Quoted 8 lines

include/linux/rculist.h +0 -0

 /**
 * list_for_each_entry_rcu    -    iterate over rcu list of given type

* @pos: the type * to use as a loop cursor.
------- END OF PATCH ------

Anyway, it's up to you.

Hope that helps.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html