Skip to contentSkip to conversation

sky2: rtnl_lock out of loop will be faster

PATCH In review Wang Chen 3 participants 4 messages Dec 12, 2007 – Dec 14, 2007

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Wang Chen

author
#0 lore raw

PATCH 4/4 sky2: rtnl_lock out of loop will be faster

[PATCH 4/4] [NETDEV] sky2: rtnl_lock out of loop will be faster

Before this patch, it gets and releases the lock at each iteration of the loop. Changing unregister_netdev to unregister_netdevice and locking outside of the loop will be faster for this approach.

Signed-off-by Wang Chen <[email protected]>
---
 sky2.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletion(-)
drivers/net/sky2.c +3 -1
@@ -4270,8 +4270,10 @@ static void __devexit sky2_remove(struct
 	del_timer_sync(&hw->watchdog_timer);
 	cancel_work_sync(&hw->restart_work);
 
+	rtnl_lock();
 	for (i = hw->ports-1; i >= 0; --i)
-		unregister_netdev(hw->dev[i]);
+		unregister_netdevice(hw->dev[i]);
+	rtnl_unlock();
 
 	sky2_write32(hw, B0_IMSK, 0);
#1 lore raw
Quoted 16 lines · On Wed, 12 Dec 2007 16:50:09 +0800 Wang Chen <[email protected]> wrote:

[PATCH 4/4] [NETDEV] sky2: rtnl_lock out of loop will be faster

Before this patch, it gets and releases the lock at each iteration of the loop. Changing unregister_netdev to unregister_netdevice and locking outside of the loop will be faster for this approach.

Signed-off-by Wang Chen <[email protected]>
---
 sky2.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletion(-)
drivers/net/sky2.c +0 -0
@@ -4270,8 +4270,10 @@ static void __devexit sky2_remove(struct
 	del_timer_sync(&hw->watchdog_timer);

drivers/net/sky2.c static void __devexit sky2_remove(struct · line 4279

Quoted 10 lines
drivers/net/sky2.c +3 -1
 	cancel_work_sync(&hw->restart_work);
 
+	rtnl_lock();
 	for (i = hw->ports-1; i >= 0; --i)
-		unregister_netdev(hw->dev[i]);
+		unregister_netdevice(hw->dev[i]);
+	rtnl_unlock();
 
 	sky2_write32(hw, B0_IMSK, 0);

Umm, okay but it doesn't matter really, there can only be 2 ports and 90+% of the boards only have one port. So why bother??

Signature · 1 line
Stephen Hemminger <[email protected]>
author
#2 lore raw

Stephen Hemminger said the following on 2007-12-13 0:30:

Quoted 3 lines

Umm, okay but it doesn't matter really, there can only be 2 ports and 90+% of the boards only have one port. So why bother??

Thanks Stephen.
David, please ignore all of the four patches.

#3 lore raw
In reply to Wang Chen
Quoted 15 lines · Wang Chen

[PATCH 4/4] [NETDEV] sky2: rtnl_lock out of loop will be faster

Before this patch, it gets and releases the lock at each iteration of the loop. Changing unregister_netdev to unregister_netdevice and locking outside of the loop will be faster for this approach.

Signed-off-by Wang Chen <[email protected]>
---
 sky2.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletion(-)

drivers/net/sky2.c static void __devexit sky2_remove(struct · line 4276

Quoted 8 lines
drivers/net/sky2.c +3 -1
@@ -4270,8 +4270,10 @@ static void __devexit sky2_remove(struct
 	del_timer_sync(&hw->watchdog_timer);
 	cancel_work_sync(&hw->restart_work);
 
+	rtnl_lock();
 	for (i = hw->ports-1; i >= 0; --i)
-		unregister_netdev(hw->dev[i]);
+		unregister_netdevice(hw->dev[i]);
+	rtnl_unlock();

while true and correct, I don't see the remove path as needing this type of micro-optimization.

Removing and shutting down hardware is an operation that can take many seconds (an eternity, to a computer)... a very slow operation.

Thus, given that speed is not a priority here, I place more value on smaller, more compact, easily reviewable code -- the existing unpatched code in this case.

Jeff