Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

2007-12-14 Thread Krzysztof Halasa 
Wang Chen [EMAIL PROTECTED] 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
--
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

Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

2007-12-14 Thread David Miller 
From: Krzysztof Halasa [EMAIL PROTECTED]
Date: Fri, 14 Dec 2007 22:28:07 +0100

 Wang Chen [EMAIL PROTECTED] 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.
--
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

Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

2007-12-12 Thread David Miller 
From: Wang Chen [EMAIL PROTECTED]
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 [EMAIL PROTECTED]

Applied, thanks for finding and fixing this bug.
--
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

Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

2007-12-12 Thread Herbert Xu 
David Miller [EMAIL PROTECTED] wrote:
 From: Wang Chen [EMAIL PROTECTED]
 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 [EMAIL PROTECTED]

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 ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
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

Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

2007-12-12 Thread David Miller 
From: Herbert Xu [EMAIL PROTECTED]
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 [EMAIL PROTECTED]

Good idea, applied to net-2.6.25, thanks.
--
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