diff mbox

[1/1] bonding: restrict the data protected by rcu_read_lock

Message ID 1468308497-5384-1-git-send-email-zyjzyj2000@gmail.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Zhu Yanjun July 12, 2016, 7:28 a.m. UTC
In this function, origin is a pointer to struct aggregator.
No matter what agg is changed to, it has nothing to do with origin.

CC: Jay Vosburgh <jay.vosburgh@canonical.com>
Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
---
 drivers/net/bonding/bond_3ad.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

David Miller July 12, 2016, 6:03 p.m. UTC | #1
From: Zhu Yanjun <zyjzyj2000@gmail.com>
Date: Tue, 12 Jul 2016 15:28:17 +0800

> In this function, origin is a pointer to struct aggregator.
> No matter what agg is changed to, it has nothing to do with origin.
> 
> CC: Jay Vosburgh <jay.vosburgh@canonical.com>
> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>

I really hate changes like this.

Imagine if a inner function was called with RCU protection around
it, as a rule.

That's the same as what is happening here, the RCU locking is done
around the entire contents of the function and that is perfectly
fine even if one variable assignment or whatever is superfluously
contained inside of it.

I'm not applying this patch, sorry.
diff mbox

Patch

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index edc70ff..20afee3 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1624,8 +1624,8 @@  static void ad_agg_selection_logic(struct aggregator *agg,
 	struct slave *slave;
 	struct port *port;
 
-	rcu_read_lock();
 	origin = agg;
+	rcu_read_lock();
 	active = __get_active_agg(agg);
 	best = (active && agg_device_up(active)) ? active : NULL;