netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
@ 2007-12-12  2:35 Wang Chen
  2007-12-12 18:46 ` David Miller
  2007-12-14 21:28 ` Krzysztof Halasa
  0 siblings, 2 replies; 6+ messages in thread
From: Wang Chen @ 2007-12-12  2:35 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Krzysztof Halasa, netdev, David S. Miller

[PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

Since the caller and the upper caller doesn't hod the rtnl semaphore.
We should use unregister_netdev instead of unregister_netdevice.

Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>
---
 hdlc_fr.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

--- linux-2.6.24.rc5.org/drivers/net/wan/hdlc_fr.c	2007-10-10 04:31:38.000000000 +0800
+++ linux-2.6.24.rc5/drivers/net/wan/hdlc_fr.c	2007-12-12 10:25:13.000000000 +0800
@@ -1177,7 +1177,7 @@ static int fr_del_pvc(hdlc_device *hdlc,
 	if (dev->flags & IFF_UP)
 		return -EBUSY;		/* PVC in use */
 
-	unregister_netdevice(dev); /* the destructor will free_netdev(dev) */
+	unregister_netdev(dev); /* the destructor will free_netdev(dev) */
 	*get_dev_p(pvc, type) = NULL;
 
 	if (!pvc_is_used(pvc)) {
@@ -1202,10 +1202,10 @@ static void fr_destroy(struct net_device
 		pvc_device *next = pvc->next;
 		/* destructors will free_netdev() main and ether */
 		if (pvc->main)
-			unregister_netdevice(pvc->main);
+			unregister_netdev(pvc->main);
 
 		if (pvc->ether)
-			unregister_netdevice(pvc->ether);
+			unregister_netdev(pvc->ether);
 
 		kfree(pvc);
 		pvc = next;


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
  2007-12-12  2:35 [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice Wang Chen
@ 2007-12-12 18:46 ` David Miller
  2007-12-13  3:13   ` Herbert Xu
  2007-12-14 21:28 ` Krzysztof Halasa
  1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2007-12-12 18:46 UTC (permalink / raw)
  To: wangchen; +Cc: jgarzik, khc, netdev

From: Wang Chen <wangchen@cn.fujitsu.com>
Date: Wed, 12 Dec 2007 10:35:56 +0800

> [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
> 
> Since the caller and the upper caller doesn't hod the rtnl semaphore.
> We should use unregister_netdev instead of unregister_netdevice.
> 
> Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>

Applied, thanks for finding and fixing this bug.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
  2007-12-12 18:46 ` David Miller
@ 2007-12-13  3:13   ` Herbert Xu
  2007-12-13  3:22     ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2007-12-13  3:13 UTC (permalink / raw)
  To: David Miller; +Cc: wangchen, jgarzik, khc, netdev

David Miller <davem@davemloft.net> wrote:
> From: Wang Chen <wangchen@cn.fujitsu.com>
> Date: Wed, 12 Dec 2007 10:35:56 +0800
> 
>> [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
> 
> Applied, thanks for finding and fixing this bug.

Let's see who else is doing this:

[NET]: Check RTNL status in unregister_netdevice

The caller must hold the RTNL so let's check it in unregister_netdevice.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/net/core/dev.c b/net/core/dev.c
index 06615df..b254e52 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3967,6 +3967,8 @@ void synchronize_net(void)
 
 void unregister_netdevice(struct net_device *dev)
 {
+	ASSERT_RTNL();
+
 	rollback_registered(dev);
 	/* Finish processing unregister after unlock */
 	net_set_todo(dev);

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
  2007-12-13  3:13   ` Herbert Xu
@ 2007-12-13  3:22     ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2007-12-13  3:22 UTC (permalink / raw)
  To: herbert; +Cc: wangchen, jgarzik, khc, netdev

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 13 Dec 2007 11:13:44 +0800

> Let's see who else is doing this:
> 
> [NET]: Check RTNL status in unregister_netdevice
> 
> The caller must hold the RTNL so let's check it in unregister_netdevice.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Good idea, applied to net-2.6.25, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
  2007-12-12  2:35 [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice Wang Chen
  2007-12-12 18:46 ` David Miller
@ 2007-12-14 21:28 ` Krzysztof Halasa
  2007-12-14 21:36   ` David Miller
  1 sibling, 1 reply; 6+ messages in thread
From: Krzysztof Halasa @ 2007-12-14 21:28 UTC (permalink / raw)
  To: Wang Chen; +Cc: Jeff Garzik, netdev, David S. Miller

Wang Chen <wangchen@cn.fujitsu.com> writes:

> [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
>
> Since the caller and the upper caller doesn't hod the rtnl semaphore.
> We should use unregister_netdev instead of unregister_netdevice.

NAK, not-a-bug. The caller actually holds rtnl, it goes through
the netdev core ioctl dispatcher:

(unregister_netdevice+0x0/0x24) from (fr_ioctl+0x688/0x75c)
/* fr_del_pvc() and fr_add_pvc() optimized out by gcc */
(fr_ioctl+0x0/0x75c) from (hdlc_ioctl+0x4c/0x8c)
(hdlc_ioctl+0x0/0x8c) from (hss_ioctl+0x3c/0x324)
(hss_ioctl+0x0/0x324) from (dev_ifsioc+0x428/0x4e8)
(dev_ifsioc+0x0/0x4e8) from (dev_ioctl+0x5d8/0x664)
(dev_ioctl+0x0/0x664) from (sock_ioctl+0x90/0x254)
(sock_ioctl+0x0/0x254) from (do_ioctl+0x34/0x78)
(do_ioctl+0x0/0x78) from (vfs_ioctl+0x78/0x2a8)
(vfs_ioctl+0x0/0x2a8) from (sys_ioctl+0x40/0x64)
(sys_ioctl+0x0/0x64) from (ret_fast_syscall+0x0/0x2c)

The patch would make it deadlock.

Please note that sister fr_add_pvc() uses register_netdevice().
The same applies to fr_destroy().
-- 
Krzysztof Halasa

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
  2007-12-14 21:28 ` Krzysztof Halasa
@ 2007-12-14 21:36   ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2007-12-14 21:36 UTC (permalink / raw)
  To: khc; +Cc: wangchen, jgarzik, netdev

From: Krzysztof Halasa <khc@pm.waw.pl>
Date: Fri, 14 Dec 2007 22:28:07 +0100

> Wang Chen <wangchen@cn.fujitsu.com> writes:
> 
> > [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice
> >
> > Since the caller and the upper caller doesn't hod the rtnl semaphore.
> > We should use unregister_netdev instead of unregister_netdevice.
> 
> NAK, not-a-bug. The caller actually holds rtnl, it goes through
> the netdev core ioctl dispatcher:
> 
> (unregister_netdevice+0x0/0x24) from (fr_ioctl+0x688/0x75c)
> /* fr_del_pvc() and fr_add_pvc() optimized out by gcc */
> (fr_ioctl+0x0/0x75c) from (hdlc_ioctl+0x4c/0x8c)
> (hdlc_ioctl+0x0/0x8c) from (hss_ioctl+0x3c/0x324)
> (hss_ioctl+0x0/0x324) from (dev_ifsioc+0x428/0x4e8)
> (dev_ifsioc+0x0/0x4e8) from (dev_ioctl+0x5d8/0x664)
> (dev_ioctl+0x0/0x664) from (sock_ioctl+0x90/0x254)
> (sock_ioctl+0x0/0x254) from (do_ioctl+0x34/0x78)
> (do_ioctl+0x0/0x78) from (vfs_ioctl+0x78/0x2a8)
> (vfs_ioctl+0x0/0x2a8) from (sys_ioctl+0x40/0x64)
> (sys_ioctl+0x0/0x64) from (ret_fast_syscall+0x0/0x2c)
> 
> The patch would make it deadlock.

Ok, I'll drop this patch, thanks for checking.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-12-14 21:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-12-12  2:35 [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice Wang Chen
2007-12-12 18:46 ` David Miller
2007-12-13  3:13   ` Herbert Xu
2007-12-13  3:22     ` David Miller
2007-12-14 21:28 ` Krzysztof Halasa
2007-12-14 21:36   ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).