Skip to contentSkip to conversation

Remove double dev->flags checking when calling dev_close()

PATCH Applied Pavel Emelyanov 4 participants 5 messages Oct 9, 2007 – Oct 10, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Remove double dev->flags checking when calling dev_close()

The unregister_netdevice() and dev_change_net_namespace() both check for dev->flags to be IFF_UP before calling the dev_close(), but the dev_close() checks for IFF_UP itself, so remove those unneeded checks.

---

net/core/dev.c +2 -4
index e7e728a..1e169a5 100644
@@ -3893,8 +3893,7 @@ void unregister_netdevice(struct net_dev
 	BUG_ON(dev->reg_state != NETREG_REGISTERED);
 
 	/* If device is running, close it first. */
-	if (dev->flags & IFF_UP)
-		dev_close(dev);
+	dev_close(dev);
 
 	/* And unlink it from device chain. */
 	unlist_netdevice(dev);
@@ -4018,8 +4017,7 @@ int dev_change_net_namespace(struct net_
 	 */
 
 	/* If device is running close it first. */
-	if (dev->flags & IFF_UP)
-		dev_close(dev);
+	dev_close(dev);
 
 	/* And unlink it from device chain */
 	err = -ENODEV;
#1 lore raw
Quoted 21 lines · Pavel Emelyanov

The unregister_netdevice() and dev_change_net_namespace() both check for dev->flags to be IFF_UP before calling the dev_close(), but the dev_close() checks for IFF_UP itself, so remove those unneeded checks.

---

net/core/dev.c +1 -2
index e7e728a..1e169a5 100644
@@ -3893,8 +3893,7 @@ void unregister_netdevice(struct net_dev
 	BUG_ON(dev->reg_state != NETREG_REGISTERED);
 
 	/* If device is running, close it first. */
-	if (dev->flags & IFF_UP)
-		dev_close(dev);
+	dev_close(dev);
 
 	/* And unlink it from device chain. */
 	unlist_netdevice(dev);

net/core/dev.c int dev_change_net_namespace(struct net_ · line 4023

Quoted 9 lines
net/core/dev.c +1 -2
@@ -4018,8 +4017,7 @@ int dev_change_net_namespace(struct net_
 	 */
 
 	/* If device is running close it first. */
-	if (dev->flags & IFF_UP)
-		dev_close(dev);
+	dev_close(dev);
 
 	/* And unlink it from device chain */
 	err = -ENODEV;

One side effect of this patch: might_sleep() is now called unconditionally.

If that is irrelevant... ACK.

#2 lore raw
Quoted 32 lines · Jeff Garzik
Quoted 31 lines · Pavel Emelyanov

The unregister_netdevice() and dev_change_net_namespace() both check for dev->flags to be IFF_UP before calling the dev_close(), but the dev_close() checks for IFF_UP itself, so remove those unneeded checks.

---

net/core/dev.c +2 -4
index e7e728a..1e169a5 100644
@@ -3893,8 +3893,7 @@ void unregister_netdevice(struct net_dev
 	BUG_ON(dev->reg_state != NETREG_REGISTERED);
 
 	/* If device is running, close it first. */
-	if (dev->flags & IFF_UP)
-		dev_close(dev);
+	dev_close(dev);
 
 	/* And unlink it from device chain. */
 	unlist_netdevice(dev);
@@ -4018,8 +4017,7 @@ int dev_change_net_namespace(struct net_
 	 */
 
 	/* If device is running close it first. */
-	if (dev->flags & IFF_UP)
-		dev_close(dev);
+	dev_close(dev);
 
Quoted 4 lines

net/core/dev.c int dev_change_net_namespace(struct net_ · line 4023

Quoted 2 lines
net/core/dev.c +0 -0
 	/* And unlink it from device chain */
 	err = -ENODEV;

One side effect of this patch: might_sleep() is now called unconditionally.

That's not a big deal - see, the synchronize_net() is called further in both places unconditionally (!) and also calls might_sleep(), so this is OK.

Quoted 1 line

If that is irrelevant... ACK.

Thanks :)

Quoted 6 lines

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

#3 lore raw
In reply to Jeff Garzik
Quoted 2 lines · Jeff Garzik

One side effect of this patch: might_sleep() is now called unconditionally.

That would be seem to be a good thing :)

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt