linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: DJBARROW@de.ibm.com
To: "David S. Miller" <davem@redhat.com>, alan@lxorguk.ukuu.org.uk
Cc: linux-kernel@vger.kernel.org
Subject: Re: bug in /net/core/dev.c
Date: Tue, 12 Jun 2001 16:17:48 +0200	[thread overview]
Message-ID: <C1256A69.004E9AF2.00@d12mta09.de.ibm.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 1565 bytes --]



Hi Alan/Dave
Here is a better version of the patch I think it cuts about 15 lines out of
dev.c  & I think logically it is more bulletproof than my previous attempt.

(See attached file: dev.c.2.4.5.v2.patch)

Hi Dave,

On 390 it is very easy to hotplug devices under VM.

We do it all the time using the channel device layer on s/390, so users
can reconfigure devices if they misconfigure them.

If it can be registered it should be able to be unregistered.

Cornelia Huck in our group in boeblingen noticed the bug too.



Please respond to "David S. Miller" <davem@redhat.com>

To:   Denis Joseph Barrow/Germany/Contr/IBM@IBMDE
cc:   linux-kernel@vger.kernel.org
Subject:  Re: bug in /net/core/dev.c





DJBARROW@de.ibm.com writes:
 > The dev->refcnt will go up to 2 ( it should be 1) unregister_netdevice
will
 > usually  loop forever
 > waiting for the refcnt to drop to 1 when an attempt to unregister is
done.

When will devices built statically into the kernel ever be
unregister'd?

Later,
David S. Miller
davem@redhat.com

Hi,
I found this bug in dev.c

It happens if register_netdevice is called before net_dev_init which can
happen from init functions,
when device drivers are compiled into the kernel.

The dev->refcnt will go up to 2 ( it should be 1) unregister_netdevice will
usually  loop forever
waiting for the refcnt to drop to 1 when an attempt to unregister is done.

The printk in the bootmessages early initialization of device <blah> is
deferred is evidence of this behaviour occuring.

The small patch is below ,hope it is okay for you.

[-- Attachment #2: dev.c.2.4.5.v2.patch --]
[-- Type: application/octet-stream, Size: 2269 bytes --]

--- net/core/dev.old.c	Wed May 30 14:24:47 2001
+++ net/core/dev.c	Tue Jun 12 15:37:58 2001
@@ -20,6 +20,10 @@
  *              Pekka Riikonen <priikone@poesidon.pspt.fi>
  *
  *	Changes:
+ *              D.J. Barrow     :       Fixed bug where dev->refcnt gets set to 2 
+ *                                      if register_netdev gets called before
+ *                                      net_dev_init & also removed a few lines
+ *                                      of code in the process.
  *		Alan Cox	:	device private ioctl copies fields back.
  *		Alan Cox	:	Transmit queue code does relevant stunts to
  *					keep the queue safe.
@@ -2382,6 +2386,7 @@
  *	will not get the same name.
  */
 
+int net_dev_init(void);
 int register_netdevice(struct net_device *dev)
 {
 	struct net_device *d, **dp;
@@ -2396,48 +2401,8 @@
 	dev->fastpath_lock=RW_LOCK_UNLOCKED;
 #endif
 
-	if (dev_boot_phase) {
-#ifdef CONFIG_NET_DIVERT
-		ret = alloc_divert_blk(dev);
-		if (ret)
-			return ret;
-#endif /* CONFIG_NET_DIVERT */
-		
-		/* This is NOT bug, but I am not sure, that all the
-		   devices, initialized before netdev module is started
-		   are sane. 
-
-		   Now they are chained to device boot list
-		   and probed later. If a module is initialized
-		   before netdev, but assumes that dev->init
-		   is really called by register_netdev(), it will fail.
-
-		   So that this message should be printed for a while.
-		 */
-		printk(KERN_INFO "early initialization of device %s is deferred\n", dev->name);
-
-		/* Check for existence, and append to tail of chain */
-		for (dp=&dev_base; (d=*dp) != NULL; dp=&d->next) {
-			if (d == dev || strcmp(d->name, dev->name) == 0) {
-				return -EEXIST;
-			}
-		}
-		dev->next = NULL;
-		write_lock_bh(&dev_base_lock);
-		*dp = dev;
-		dev_hold(dev);
-		write_unlock_bh(&dev_base_lock);
-
-		/*
-		 *	Default initial state at registry is that the
-		 *	device is present.
-		 */
-
-		set_bit(__LINK_STATE_PRESENT, &dev->state);
-
-		return 0;
-	}
-
+	if (dev_boot_phase)
+		net_dev_init();
 #ifdef CONFIG_NET_DIVERT
 	ret = alloc_divert_blk(dev);
 	if (ret)
@@ -2679,7 +2644,9 @@
 {
 	struct net_device *dev, **dp;
 	int i;
-
+	
+	if(!dev_boot_phase)
+		return 0;
 #ifdef CONFIG_NET_SCHED
 	pktsched_init();
 #endif

             reply	other threads:[~2001-06-12 14:20 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-06-12 14:17 DJBARROW [this message]
2001-06-12 14:57 ` David S. Miller
2001-06-12 15:34   ` Keith Owens
2001-06-12 15:46   ` David S. Miller
2001-06-12 16:11     ` Keith Owens
  -- strict thread matches above, loose matches on Subject: below --
2001-06-13 10:03 DJBARROW
2001-06-13 10:18 ` David S. Miller
2001-06-12 18:38 DJBARROW
2001-06-12 18:44 ` David S. Miller
2001-06-12 17:05 Ulrich.Weigand
2001-06-12 17:20 ` Keith Owens
2001-06-12 16:43 DJBARROW
2001-06-12 16:51 ` David S. Miller
2001-06-12 17:02 ` Keith Owens
2001-06-11 18:32 DJBARROW
2001-06-12  3:18 ` David S. Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=C1256A69.004E9AF2.00@d12mta09.de.ibm.com \
    --to=djbarrow@de.ibm.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=davem@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: bug in /net/core/dev.c' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).