netdev - Re: [PATCH net-next v2 6/10] bonding: rebuild the lock use for bond_activebackup_arp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <527F06A2.2070407@gmail.com>
Date:	Sun, 10 Nov 2013 12:08:02 +0800
From:	Ding Tianhong <dthxman@...il.com>
To:	Nikolay Aleksandrov <nikolay@...hat.com>
CC:	Ding Tianhong <dingtianhong@...wei.com>,
	Jay Vosburgh <fubar@...ibm.com>,
	Andy Gospodarek <andy@...yhouse.net>,
	"David S. Miller" <davem@...emloft.net>,
	Veaceslav Falico <vfalico@...hat.com>,
	Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v2 6/10] bonding: rebuild the lock use for bond_activebackup_arp_mon()


>> +	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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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