Thread (19 messages) 19 messages, 3 authors, 2025-12-24

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

From: Hangbin Liu <hidden>
Date: 2025-12-24 06:00:54

On Mon, Dec 22, 2025 at 10:15:07PM +0800, Tonghao Zhang wrote:
quoted
On Dec 1, 2025, at 15:35, Hangbin Liu [off-list ref] wrote:

On Sun, Nov 30, 2025 at 03:48:45PM +0800, Tonghao Zhang wrote:
quoted
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@jvosburgh.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Cc: Nikolay Aleksandrov <razor@blackwall.org>
Cc: Hangbin Liu <redacted>
Cc: Jason Xing <redacted>
Signed-off-by: Tonghao Zhang <redacted>
---
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.
“"
OK, then let's follow Paolo's suggestion.

Hangbin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help