netdev - Re: [PATCH net-next v3 3/4] net: bonding: skip the 2nd trylock when first one fail

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <10FF7526-38C4-4776-BA00-7ECF6E7E143D@bamaicloud.com>
Date: Mon, 22 Dec 2025 22:15:07 +0800
From: Tonghao Zhang <tonghao@...aicloud.com>
To: Hangbin Liu <liuhangbin@...il.com>
Cc: netdev@...r.kernel.org,
 Jay Vosburgh <jv@...sburgh.net>,
 "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>,
 Jonathan Corbet <corbet@....net>,
 Andrew Lunn <andrew+netdev@...n.ch>,
 Nikolay Aleksandrov <razor@...ckwall.org>,
 Jason Xing <kerneljasonxing@...il.com>
Subject: Re: [PATCH net-next v3 3/4] net: bonding: skip the 2nd trylock when
 first one fail



> On Dec 1, 2025, at 15:35, Hangbin Liu <liuhangbin@...il.com> wrote:
> 
> On Sun, Nov 30, 2025 at 03:48:45PM +0800, Tonghao Zhang wrote:
>> After the first trylock fail, retrying immediately is
>> not advised as there is a high probability of failing
>> to acquire the lock again. This optimization makes sense.
>> 
>> Cc: Jay Vosburgh <jv@...sburgh.net>
>> Cc: "David S. Miller" <davem@...emloft.net>
>> Cc: Eric Dumazet <edumazet@...gle.com>
>> Cc: Jakub Kicinski <kuba@...nel.org>
>> Cc: Paolo Abeni <pabeni@...hat.com>
>> Cc: Simon Horman <horms@...nel.org>
>> Cc: Jonathan Corbet <corbet@....net>
>> Cc: Andrew Lunn <andrew+netdev@...n.ch>
>> Cc: Nikolay Aleksandrov <razor@...ckwall.org>
>> Cc: Hangbin Liu <liuhangbin@...il.com>
>> Cc: Jason Xing <kerneljasonxing@...il.com>
>> Signed-off-by: Tonghao Zhang <tonghao@...aicloud.com>
>> ---
>> v1:
>> - splitted from: https://patchwork.kernel.org/project/netdevbpf/patch/20251118090431.35654-1-tonghao@bamaicloud.com/
>> - this patch only skip the 2nd rtnl lock.
>> - add this patch to series
>> ---
>> drivers/net/bonding/bond_main.c | 16 +++++++++-------
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 1b16c4cd90e0..025ca0a45615 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -3756,7 +3756,7 @@ static bool bond_ab_arp_probe(struct bonding *bond)
>> 
>> static void bond_activebackup_arp_mon(struct bonding *bond)
>> {
>> - bool should_notify_rtnl = false;
>> + bool should_notify_rtnl;
>> int delta_in_ticks;
>> 
>> delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval);
>> @@ -3784,13 +3784,11 @@ static void bond_activebackup_arp_mon(struct bonding *bond)
>> should_notify_rtnl = bond_ab_arp_probe(bond);
>> rcu_read_unlock();
>> 
>> -re_arm:
>> - if (bond->params.arp_interval)
>> - queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
>> -
>> if (bond->send_peer_notif || should_notify_rtnl) {
>> - if (!rtnl_trylock())
>> - return;
>> + if (!rtnl_trylock()) {
>> + delta_in_ticks = 1;
>> + goto re_arm;
>> + }
>> 
>> if (bond->send_peer_notif) {
>> if (bond_should_notify_peers(bond))
>> @@ -3805,6 +3803,10 @@ static void bond_activebackup_arp_mon(struct bonding *bond)
>> 
>> rtnl_unlock();
>> }
>> +
>> +re_arm:
>> + if (bond->params.arp_interval)
>> + queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
>> }
>> 
>> static void bond_arp_monitor(struct work_struct *work)
>> -- 
>> 2.34.1
>> 
> 
> Maybe this patch should be merged together with patch 02, since the issue
> was introduced there. Before patch 02, both should_notify_peers and
> should_notify_rtnl would be false when the first rtnl_trylock() failed,
> so the second trylock() would never be called.
Yes, but Paolo suggested that put it in a separate patch file, because this code is unrelated from patch02. It's all good to me.
“”"
The above skips the 2nd trylock attempt when the first one fail, which
IMHO makes sense, but its unrelated from the rest of the change here. I
think this specific bits should go in a separate patch.
“"

https://patchwork.kernel.org/project/netdevbpf/patch/20251118090431.35654-1-tonghao@bamaicloud.com/
> 
> Thanks
> Hangbin
>