Re: [PATCH net-next v2 6/10] bonding: rebuild the lock use for bond_activebackup_arp_mon()

[Ding Tianhong <dthxman@gmail.com>]

>> +	rcu_read_unlock();
>>   
>>   re_arm:
>>   	if (bond->params.arp_interval)
>>   		queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
>>   
>> -	read_unlock(&bond->lock);
>> -
>>   	if (should_notify_peers) {
>>   		if (!rtnl_trylock())
>>   			return;
>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>> index deb9738..90b745c 100644
>> --- a/drivers/net/bonding/bonding.h
>> +++ b/drivers/net/bonding/bonding.h
>> @@ -97,6 +97,13 @@
>>   		netdev_adjacent_get_private(bond_slave_list(bond)->prev) : \
>>   		NULL)
>>   
>> +#define bond_first_slave_rcu(bond) \
>> +	({struct list_head *__ptr = (bond_slave_list(bond)); \
>> +	 struct list_head *__next = ACCESS_ONCE(__ptr->next); \
>> +	 likely(__ptr != __next) ? \
>> +	 netdev_adjacent_get_private_rcu(__next) : NULL; \
>> +	 })
>> +
> Honestly, I don't like this, it sure can be re-written in a more
> straight-forward manner.

I have re-write the function by 2 ways, the first one just like 
list_first_or_null_rcu,
the second one just used the exist function 
netdev_lower_get_next_private_rcu,
I think the first one is better, it is more exactly.

1:

+#define bond_first_slave_rcu(bond) \
+	({struct list_head *__ptr = (bond_slave_list(bond)); \
+	 struct list_head *__next = ACCESS_ONCE(__ptr->next); \
+	 likely(__ptr != __next) ? \
+	 (list_entry_rcu(__next, struct netdev_adjacent, list))->private : NULL; \
+	 })
+

2:

+#define bond_first_slave_rcu(bond) \
+	({struct list_head *iter = (bond_slave_list(bond)); \
+	 netdev_lower_get_next_private_rcu(bond->dev, &iter) ? : NULL; \
+	})


what do you think about is, maybe you have more wonderful idea, pls 
remind me,
thanks

Regards
Ding

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