Fw: Frequent Oops on Shutdown 2.6.10

Fw: Frequent Oops on Shutdown 2.6.10

[Andrew Morton]

Begin forwarded message:

Date: Sun, 20 Feb 2005 09:56:04 EST
From: AndyLiebman@aol.com
To: linux-kernel@vger.kernel.org
Subject: Frequent Oops on Shutdown 2.6.10


Hi, 
I compiled the 2.6.10 kernel with HyperThreading optimization (I'm  running a 
3.06 Ghz single Xeon processor with HT enabled). More or less, I'm  running 
Mandrake 10 Official, but with my own kernel. Can anybody help explain  why I'm 
getting this Oops on shutdown? It doesn't happen all the time -- about  50 
percent of the time. Never happens with the 2.6.6 kernel. I configured the  
2.6.10 kernel with mostly the same settings -- saying "no" to everything new,  
except optimizing for P4/Xeon processor, enabling HT optimization, and NOT  
enabling lots of "ham radio" and "ISDN" stuff that seemed to be enabled in the  
Mandrake kernel. 
 
Some relevant things about the machine: 
Single Xeon 3.06 processor
2 GB ECC RAM
2x 3ware 9500S-8 SATA RAID Cards
16 SATA drives
2 built-in GigE ports on motherboard
2 Intel 1000 MT Server Adapters on each of the two 133 Mhz  PCI-X slots
 
Here's the output from the Oops


*pde = 00000000
Oops:  0000 [#1]
SMP
Modules linked in: raid) appletalk xfs sd_mod sg sr_mod  3w_9xxx scsi_mod 
nfsd exportfs ipv6 af_packet raw ide_floppy ide_tape ide_cd  cdrom e1000 uhci_hcd 
usbcore rtc ext3 jbd
CPU: 1
EIP:  0060:[<c018b600>] Not tainted VLI
EFLAGS: 00010246  (2.6.10es-feb06)
EIP is at remove_proc_entry+0x2a/0x166
eax: 00000000 ebx:  f66a4e00 ecx: ffffffff edx: f6da1300
esi: f7cfb000 edi: 00000005 ebp:  c2183eb4 esp: c2183e94
ds: 007b es: 007b ss: 0068
Process swapper (pid: 0,  threadinfo=c2182000 task=c214e520)
Stack: c043402c c043402c 00000000 c2024980  00000005 f66a4e00 f7cfb000 
00000000 c2183ec8 f8c4f051 00000005 f6da1300 f66a4e00  c2183ee8 f8c2cc7b f66a4e00 
c2024980 00000002 00000000 f6da7e80 f6da6080 c2183f04  c0289967 f66a4e00
Call Trace: 
[<c0103352>]  show_stack+0xaf/0xb7
[<c01034d9>]  show_registers+0x15f/0x1d2
[<c01036dd>]  die+0xfa/0x180
[<c011365e>]  do_page_fault+0x464/0x646
[<c0102fbf>]  error_code+0x2b/0x30
[<f8c4f051>] snmp6_unregister_dev+0x41/0x57  [ipv6]
[<f8c2cc7b>] in6_dev_finish_destroy+0x35/0xb6  [ipv6]
[<c0289967>] dst_destroy+0xa2/0xcd
[<c028969a>]  dst_run_gc+0x72/0xfb
[<c0123584>]  run_timer_softirq+0xc4/0x185
[<c011f631>]  __do_softirq+0x65/0xd3
[<c011f6d0>]  do_softirq+0x31/0x33
[<c0102f18>]  apic_timer_interrupt+0x1c/0x24
[<c0100747>]  cpu_idle+0x31/0x3f
[<00000000>] 0x0
[<c2183fbc>]  0xc2183fbc
Code: e2 55 89 ef 83 ec 20 8b 55 0c 8b 4d 08 89 5d f4 85 d2 89 75  f8 89 7d 
fc 89 4d f0 0f 84 b0 00 00 00 8b 7d f0 31 c0 b9 ff ff ff ff <f2>  ae f7 d1 49 
8b 42 34 8d 5a 34 85 c0 89 ce 0f 84 84 00 00 00 
<0>Kernel  panic ___ not syncing: Fatal exception in interrupt
 

Re: Frequent Oops on Shutdown 2.6.10

[YOSHIFUJI Hideaki / 吉藤英明]

Hello.

In article <20050220220304.1f2bd42f.akpm@osdl.org> (at Sun, 20 Feb 2005 22:03:04 -0800), Andrew Morton <akpm@osdl.org> says:

> EIP is at remove_proc_entry+0x2a/0x166
:
> [<f8c4f051>] snmp6_unregister_dev+0x41/0x57  [ipv6]
> [<f8c2cc7b>] in6_dev_finish_destroy+0x35/0xb6  [ipv6]
> [<c0289967>] dst_destroy+0xa2/0xcd

Would you test this patch, please?
Thanks.

------
[IPV6] Don't remove dev_snmp6 procfs entry until all users gone.

Signed-off-by: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>

===== net/ipv6/proc.c 1.25 vs edited =====
--- 1.25/net/ipv6/proc.c	2004-07-08 07:17:29 +09:00
+++ edited/net/ipv6/proc.c	2005-02-21 16:05:49 +09:00
@@ -31,7 +31,9 @@
 #include <net/ipv6.h>
 
 #ifdef CONFIG_PROC_FS
-static struct proc_dir_entry *proc_net_devsnmp6;
+struct proc_dir_entry *proc_net_devsnmp6;
+static DECLARE_MUTEX(proc_net_devsnmp6_sem);
+static unsigned int proc_net_devsnmp6_users;
 
 static int fold_prot_inuse(struct proto *proto)
 {
@@ -211,13 +213,26 @@
 			   __alignof__(struct icmpv6_mib)) < 0)
 		goto err_icmp;
 
-	if (!proc_net_devsnmp6) {
-		err = -ENOENT;
-		goto err_proc;
+	down(&proc_net_devsnmp6_sem);
+	if (!proc_net_devsnmp6_users) {
+		proc_net_devsnmp6 = proc_mkdir("dev_snmp6", proc_net);
+		if (!proc_net_devsnmp6) {
+			err = -ENOMEM;
+			printk(KERN_ERR "%s(): failed to create dev_snmp6.\n",
+			       __FUNCTION__);
+			goto err_proc;
+		}
 	}
 	p = create_proc_entry(idev->dev->name, S_IRUGO, proc_net_devsnmp6);
-	if (!p)
+	if (!p) {
+		if (!proc_net_devsnmp6_users)
+			proc_net_remove("dev_snmp6");
 		goto err_proc;
+	}
+
+	proc_net_devsnmp6_users++;
+	up(&proc_net_devsnmp6_sem);
+
 	p->data = idev;
 	p->proc_fops = &snmp6_seq_fops;
 
@@ -225,6 +240,7 @@
 	return 0;
 
 err_proc:
+	up(&proc_net_devsnmp6_sem);
 	snmp6_mib_free((void **)idev->stats.icmpv6);
 err_icmp:
 	return err;
@@ -232,12 +248,23 @@
 
 int snmp6_unregister_dev(struct inet6_dev *idev)
 {
-	if (!proc_net_devsnmp6)
+	down(&proc_net_devsnmp6_sem);
+	if (!proc_net_devsnmp6) {
+		up(&proc_net_devsnmp6_sem);
 		return -ENOENT;
-	if (!idev || !idev->stats.proc_dir_entry)
+	}
+	if (!idev || !idev->stats.proc_dir_entry) {
+		up(&proc_net_devsnmp6_sem);
 		return -EINVAL;
+	}
 	remove_proc_entry(idev->stats.proc_dir_entry->name,
 			  proc_net_devsnmp6);
+	if (!--proc_net_devsnmp6_users) {
+		proc_net_remove("dev_snmp6");
+		proc_net_devsnmp6 = NULL;
+	}
+	up(&proc_net_devsnmp6_sem);
+
 	snmp6_mib_free((void **)idev->stats.icmpv6);
 
 	return 0;
@@ -250,18 +277,12 @@
 	if (!proc_net_fops_create("snmp6", S_IRUGO, &snmp6_seq_fops))
 		goto proc_snmp6_fail;
 
-	proc_net_devsnmp6 = proc_mkdir("dev_snmp6", proc_net);
-	if (!proc_net_devsnmp6)
-		goto proc_dev_snmp6_fail;
-
 	if (!proc_net_fops_create("sockstat6", S_IRUGO, &sockstat6_seq_fops))
 		goto proc_sockstat6_fail;
 out:
 	return rc;
 
 proc_sockstat6_fail:
-	proc_net_remove("dev_snmp6");
-proc_dev_snmp6_fail:
 	proc_net_remove("snmp6");
 proc_snmp6_fail:
 	rc = -ENOMEM;
@@ -271,7 +292,6 @@
 void ipv6_misc_proc_exit(void)
 {
 	proc_net_remove("sockstat6");
-	proc_net_remove("dev_snmp6");
 	proc_net_remove("snmp6");
 }
 

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: Frequent Oops on Shutdown 2.6.10

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20050221.162241.24618885.yoshfuji@linux-ipv6.org> (at Mon, 21 Feb 2005 16:22:41 +0900 (JST)), YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org> says:

> [IPV6] Don't remove dev_snmp6 procfs entry until all users gone.
> 
> Signed-off-by: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> 
> ===== net/ipv6/proc.c 1.25 vs edited =====
> --- 1.25/net/ipv6/proc.c	2004-07-08 07:17:29 +09:00
> +++ edited/net/ipv6/proc.c	2005-02-21 16:05:49 +09:00
:
> -	if (!p)
> +	if (!p) {
> +		if (!proc_net_devsnmp6_users)
> +			proc_net_remove("dev_snmp6");
>  		goto err_proc;
> +	}

Oops, sorry, I made a mistake here.
Please try this instead.

Thanks.

------
[IPV6] Don't remove dev_snmp6 procfs entry until all users gone.

Signed-off-by: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>

===== net/ipv6/proc.c 1.25 vs edited =====
--- 1.25/net/ipv6/proc.c	2004-07-08 07:17:29 +09:00
+++ edited/net/ipv6/proc.c	2005-02-21 16:23:57 +09:00
@@ -31,7 +31,9 @@
 #include <net/ipv6.h>
 
 #ifdef CONFIG_PROC_FS
-static struct proc_dir_entry *proc_net_devsnmp6;
+struct proc_dir_entry *proc_net_devsnmp6;
+static DECLARE_MUTEX(proc_net_devsnmp6_sem);
+static unsigned int proc_net_devsnmp6_users;
 
 static int fold_prot_inuse(struct proto *proto)
 {
@@ -211,13 +213,28 @@
 			   __alignof__(struct icmpv6_mib)) < 0)
 		goto err_icmp;
 
-	if (!proc_net_devsnmp6) {
-		err = -ENOENT;
-		goto err_proc;
+	down(&proc_net_devsnmp6_sem);
+	if (!proc_net_devsnmp6_users) {
+		proc_net_devsnmp6 = proc_mkdir("dev_snmp6", proc_net);
+		if (!proc_net_devsnmp6) {
+			err = -ENOMEM;
+			printk(KERN_ERR "%s(): failed to create dev_snmp6.\n",
+			       __FUNCTION__);
+			goto err_proc;
+		}
 	}
 	p = create_proc_entry(idev->dev->name, S_IRUGO, proc_net_devsnmp6);
-	if (!p)
+	if (!p) {
+		if (!proc_net_devsnmp6_users) {
+			proc_net_remove("dev_snmp6");
+			proc_net_devsnmp6 = NULL;
+		}
 		goto err_proc;
+	}
+
+	proc_net_devsnmp6_users++;
+	up(&proc_net_devsnmp6_sem);
+
 	p->data = idev;
 	p->proc_fops = &snmp6_seq_fops;
 
@@ -225,6 +242,7 @@
 	return 0;
 
 err_proc:
+	up(&proc_net_devsnmp6_sem);
 	snmp6_mib_free((void **)idev->stats.icmpv6);
 err_icmp:
 	return err;
@@ -232,12 +250,23 @@
 
 int snmp6_unregister_dev(struct inet6_dev *idev)
 {
-	if (!proc_net_devsnmp6)
+	down(&proc_net_devsnmp6_sem);
+	if (!proc_net_devsnmp6) {
+		up(&proc_net_devsnmp6_sem);
 		return -ENOENT;
-	if (!idev || !idev->stats.proc_dir_entry)
+	}
+	if (!idev || !idev->stats.proc_dir_entry) {
+		up(&proc_net_devsnmp6_sem);
 		return -EINVAL;
+	}
 	remove_proc_entry(idev->stats.proc_dir_entry->name,
 			  proc_net_devsnmp6);
+	if (!--proc_net_devsnmp6_users) {
+		proc_net_remove("dev_snmp6");
+		proc_net_devsnmp6 = NULL;
+	}
+	up(&proc_net_devsnmp6_sem);
+
 	snmp6_mib_free((void **)idev->stats.icmpv6);
 
 	return 0;
@@ -250,18 +279,12 @@
 	if (!proc_net_fops_create("snmp6", S_IRUGO, &snmp6_seq_fops))
 		goto proc_snmp6_fail;
 
-	proc_net_devsnmp6 = proc_mkdir("dev_snmp6", proc_net);
-	if (!proc_net_devsnmp6)
-		goto proc_dev_snmp6_fail;
-
 	if (!proc_net_fops_create("sockstat6", S_IRUGO, &sockstat6_seq_fops))
 		goto proc_sockstat6_fail;
 out:
 	return rc;
 
 proc_sockstat6_fail:
-	proc_net_remove("dev_snmp6");
-proc_dev_snmp6_fail:
 	proc_net_remove("snmp6");
 proc_snmp6_fail:
 	rc = -ENOMEM;
@@ -271,7 +294,6 @@
 void ipv6_misc_proc_exit(void)
 {
 	proc_net_remove("sockstat6");
-	proc_net_remove("dev_snmp6");
 	proc_net_remove("snmp6");
 }
 

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: Frequent Oops on Shutdown 2.6.10

[Herbert Xu]

YOSHIFUJI Hideaki / ???? <yoshfuji@linux-ipv6.org> wrote:
> In article <20050221.162241.24618885.yoshfuji@linux-ipv6.org> (at Mon, 21 Feb 2005 16:22:41 +0900 (JST)), YOSHIFUJI Hideaki / ???? <yoshfuji@linux-ipv6.org> says:
> 
>> [IPV6] Don't remove dev_snmp6 procfs entry until all users gone.

Sorry, but I don't see how this patch explains the oops the
people saw.

I googled for this oops and found three distinct occurences.  They
share the property that idev->stats.proc_dir_entry->name contains
a bogus value.  In two cases it had 0x5 and 0xa respectively, in
the other it had 0x61696265.

The last one could be an important clue.

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

Re: Frequent Oops on Shutdown 2.6.10

[Herbert Xu]

On Tue, Feb 22, 2005 at 08:57:19PM +1100, Herbert Xu wrote:
> YOSHIFUJI Hideaki / ???? <yoshfuji@linux-ipv6.org> wrote:
> > In article <20050221.162241.24618885.yoshfuji@linux-ipv6.org> (at Mon, 21 Feb 2005 16:22:41 +0900 (JST)), YOSHIFUJI Hideaki / ???? <yoshfuji@linux-ipv6.org> says:
> > 
> >> [IPV6] Don't remove dev_snmp6 procfs entry until all users gone.
> 
> Sorry, but I don't see how this patch explains the oops the
> people saw.

OK, I think I see what you were trying to fix now.  Unfortunately
I think this patch doesn't quite cure the problem.

First of all you can't sleep in snmp6_unregister_dev so semaphores
are out.  More importantly, the race is still on.

Here is what happens:

CPU0					CPU1
ifdown eth0
	...
					ifup eth0
						snmp6_register_dev
							adds proc entry
in6_dev_finish_destroy
	snmp6_unregister_dev
		deletes new proc entry

The next ifdown may fail because snmp6_unregister_dev will retrieve the
name from a proc entry that's already been deleted.

I see two solutions:

1) Unregister the proc entry earlier.  In other words, do it in
addrconf_ifdown.  Since this is highly serialised it means that
we can't add the new proc entry before the old proc entry has
been deleted.

2) Fix procfs so that we delete by pointer instead of name.  This
makes sense from a semantic pointer of view.  However, for this
particular instance it means that we may have two "eth0" entries
for as long as the old idev entry sticks around.

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

Re: Frequent Oops on Shutdown 2.6.10

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20050222101526.GA5814@gondor.apana.org.au> (at Tue, 22 Feb 2005 21:15:26 +1100), Herbert Xu <herbert@gondor.apana.org.au> says:

> On Tue, Feb 22, 2005 at 08:57:19PM +1100, Herbert Xu wrote:
> > YOSHIFUJI Hideaki / ???? <yoshfuji@linux-ipv6.org> wrote:
> > > In article <20050221.162241.24618885.yoshfuji@linux-ipv6.org> (at Mon, 21 Feb 2005 16:22:41 +0900 (JST)), YOSHIFUJI Hideaki / ???? <yoshfuji@linux-ipv6.org> says:
> > > 
> > >> [IPV6] Don't remove dev_snmp6 procfs entry until all users gone.
> > 
> > Sorry, but I don't see how this patch explains the oops the
> > people saw.
:
> I see two solutions:
> 
> 1) Unregister the proc entry earlier.  In other words, do it in
> addrconf_ifdown.  Since this is highly serialised it means that
> we can't add the new proc entry before the old proc entry has
> been deleted.

Okay, it sounds reasonable.

What do you think of this?

Signed-off-by: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>

===== include/net/ipv6.h 1.42 vs edited =====
--- 1.42/include/net/ipv6.h	2005-01-15 06:30:07 +09:00
+++ edited/include/net/ipv6.h	2005-02-23 17:10:38 +09:00
@@ -149,6 +149,8 @@
 
 int snmp6_register_dev(struct inet6_dev *idev);
 int snmp6_unregister_dev(struct inet6_dev *idev);
+int snmp6_alloc_dev(struct inet6_dev *idev);
+int snmp6_free_dev(struct inet6_dev *idev);
 int snmp6_mib_init(void *ptr[2], size_t mibsize, size_t mibalign);
 void snmp6_mib_free(void *ptr[2]);
 
===== net/ipv6/addrconf.c 1.129 vs edited =====
--- 1.129/net/ipv6/addrconf.c	2005-01-18 06:13:31 +09:00
+++ edited/net/ipv6/addrconf.c	2005-02-23 17:59:56 +09:00
@@ -308,7 +308,7 @@
 		printk("Freeing alive inet6 device %p\n", idev);
 		return;
 	}
-	snmp6_unregister_dev(idev);
+	snmp6_free_dev(idev);
 	kfree(idev);
 }
 
@@ -339,6 +339,16 @@
 		/* We refer to the device */
 		dev_hold(dev);
 
+		if (snmp6_alloc_dev(ndev) < 0) {
+			ADBG((KERN_WARNING
+				"%s(): cannot allocate memory for statistics; dev=%s.\n",
+				__FUNCTION__, dev->name));
+			neigh_parms_release(&nd_tbl, ndev->nd_parms);
+			ndev->dead = 1;
+			in6_dev_finish_destroy(ndev);
+			return NULL;
+		}
+
 		if (snmp6_register_dev(ndev) < 0) {
 			ADBG((KERN_WARNING
 				"%s(): cannot create /proc/net/dev_snmp6/%s\n",
@@ -2013,6 +2023,10 @@
 		dev->ip6_ptr = NULL;
 		idev->dead = 1;
 		write_unlock_bh(&addrconf_lock);
+
+		/* Step 1.5: remove snmp6 entry */
+		snmp6_unregister_dev(idev);
+
 	}
 
 	/* Step 2: clear hash table */
===== net/ipv6/proc.c 1.25 vs edited =====
--- 1.25/net/ipv6/proc.c	2004-07-08 07:17:29 +09:00
+++ edited/net/ipv6/proc.c	2005-02-23 18:05:27 +09:00
@@ -201,33 +201,23 @@
 
 int snmp6_register_dev(struct inet6_dev *idev)
 {
-	int err = -ENOMEM;
 	struct proc_dir_entry *p;
 
 	if (!idev || !idev->dev)
 		return -EINVAL;
 
-	if (snmp6_mib_init((void **)idev->stats.icmpv6, sizeof(struct icmpv6_mib),
-			   __alignof__(struct icmpv6_mib)) < 0)
-		goto err_icmp;
+	if (!proc_net_devsnmp6)
+		return -ENOENT;
 
-	if (!proc_net_devsnmp6) {
-		err = -ENOENT;
-		goto err_proc;
-	}
 	p = create_proc_entry(idev->dev->name, S_IRUGO, proc_net_devsnmp6);
 	if (!p)
-		goto err_proc;
+		return -ENOMEM;
+
 	p->data = idev;
 	p->proc_fops = &snmp6_seq_fops;
 
 	idev->stats.proc_dir_entry = p;
 	return 0;
-
-err_proc:
-	snmp6_mib_free((void **)idev->stats.icmpv6);
-err_icmp:
-	return err;
 }
 
 int snmp6_unregister_dev(struct inet6_dev *idev)
@@ -238,8 +228,6 @@
 		return -EINVAL;
 	remove_proc_entry(idev->stats.proc_dir_entry->name,
 			  proc_net_devsnmp6);
-	snmp6_mib_free((void **)idev->stats.icmpv6);
-
 	return 0;
 }
 
@@ -274,12 +262,21 @@
 	proc_net_remove("dev_snmp6");
 	proc_net_remove("snmp6");
 }
-
 #else	/* CONFIG_PROC_FS */
 
-
 int snmp6_register_dev(struct inet6_dev *idev)
 {
+	return 0;
+}
+
+int snmp6_unregister_dev(struct inet6_dev *idev)
+{
+	return 0;
+}
+#endif	/* CONFIG_PROC_FS */
+
+int snmp6_alloc_dev(struct inet6_dev *idev)
+{
 	int err = -ENOMEM;
 
 	if (!idev || !idev->dev)
@@ -295,11 +292,10 @@
 	return err;
 }
 
-int snmp6_unregister_dev(struct inet6_dev *idev)
+int snmp6_free_dev(struct inet6_dev *idev)
 {
 	snmp6_mib_free((void **)idev->stats.icmpv6);
 	return 0;
 }
 
-#endif
 

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: Frequent Oops on Shutdown 2.6.10

[Herbert Xu]

On Wed, Feb 23, 2005 at 06:35:55PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
> 
> What do you think of this?

Thanks, this looks great.  There is just one technical detail
to patch up.
 
> -int snmp6_unregister_dev(struct inet6_dev *idev)
> +int snmp6_free_dev(struct inet6_dev *idev)
>  {
>  	snmp6_mib_free((void **)idev->stats.icmpv6);
>  	return 0;
>  }

You need to check whether icmpv6[0] is NULL either here or in
snmp6_mib_free.  Otherwise when snmp6_alloc_dev fails we'll
wind up here and then call free_percpu on a pair of NULL pointers.

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

Re: Frequent Oops on Shutdown 2.6.10

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20050223095144.GB16820@gondor.apana.org.au> (at Wed, 23 Feb 2005 20:51:44 +1100), Herbert Xu <herbert@gondor.apana.org.au> says:

> You need to check whether icmpv6[0] is NULL either here or in
> snmp6_mib_free.  Otherwise when snmp6_alloc_dev fails we'll
> wind up here and then call free_percpu on a pair of NULL pointers.

Ok, right...

Here's the (hopefuly) final one.


Signed-off-by: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>

===== include/net/ipv6.h 1.42 vs edited =====
--- 1.42/include/net/ipv6.h	2005-01-15 06:30:07 +09:00
+++ edited/include/net/ipv6.h	2005-02-23 17:10:38 +09:00
@@ -149,6 +149,8 @@
 
 int snmp6_register_dev(struct inet6_dev *idev);
 int snmp6_unregister_dev(struct inet6_dev *idev);
+int snmp6_alloc_dev(struct inet6_dev *idev);
+int snmp6_free_dev(struct inet6_dev *idev);
 int snmp6_mib_init(void *ptr[2], size_t mibsize, size_t mibalign);
 void snmp6_mib_free(void *ptr[2]);
 
===== net/ipv6/addrconf.c 1.129 vs edited =====
--- 1.129/net/ipv6/addrconf.c	2005-01-18 06:13:31 +09:00
+++ edited/net/ipv6/addrconf.c	2005-02-23 17:59:56 +09:00
@@ -308,7 +308,7 @@
 		printk("Freeing alive inet6 device %p\n", idev);
 		return;
 	}
-	snmp6_unregister_dev(idev);
+	snmp6_free_dev(idev);
 	kfree(idev);
 }
 
@@ -339,6 +339,16 @@
 		/* We refer to the device */
 		dev_hold(dev);
 
+		if (snmp6_alloc_dev(ndev) < 0) {
+			ADBG((KERN_WARNING
+				"%s(): cannot allocate memory for statistics; dev=%s.\n",
+				__FUNCTION__, dev->name));
+			neigh_parms_release(&nd_tbl, ndev->nd_parms);
+			ndev->dead = 1;
+			in6_dev_finish_destroy(ndev);
+			return NULL;
+		}
+
 		if (snmp6_register_dev(ndev) < 0) {
 			ADBG((KERN_WARNING
 				"%s(): cannot create /proc/net/dev_snmp6/%s\n",
@@ -2013,6 +2023,10 @@
 		dev->ip6_ptr = NULL;
 		idev->dead = 1;
 		write_unlock_bh(&addrconf_lock);
+
+		/* Step 1.5: remove snmp6 entry */
+		snmp6_unregister_dev(idev);
+
 	}
 
 	/* Step 2: clear hash table */
===== net/ipv6/af_inet6.c 1.88 vs edited =====
--- 1.88/net/ipv6/af_inet6.c	2005-01-14 13:41:05 +09:00
+++ edited/net/ipv6/af_inet6.c	2005-02-24 00:58:11 +09:00
@@ -652,8 +652,10 @@
 {
 	if (ptr == NULL)
 		return;
-	free_percpu(ptr[0]);
-	free_percpu(ptr[1]);
+	if (ptr[0])
+		free_percpu(ptr[0]);
+	if (ptr[1])
+		free_percpu(ptr[1]);
 	ptr[0] = ptr[1] = NULL;
 }
 
===== net/ipv6/proc.c 1.25 vs edited =====
--- 1.25/net/ipv6/proc.c	2004-07-08 07:17:29 +09:00
+++ edited/net/ipv6/proc.c	2005-02-24 01:00:44 +09:00
@@ -201,33 +201,23 @@
 
 int snmp6_register_dev(struct inet6_dev *idev)
 {
-	int err = -ENOMEM;
 	struct proc_dir_entry *p;
 
 	if (!idev || !idev->dev)
 		return -EINVAL;
 
-	if (snmp6_mib_init((void **)idev->stats.icmpv6, sizeof(struct icmpv6_mib),
-			   __alignof__(struct icmpv6_mib)) < 0)
-		goto err_icmp;
+	if (!proc_net_devsnmp6)
+		return -ENOENT;
 
-	if (!proc_net_devsnmp6) {
-		err = -ENOENT;
-		goto err_proc;
-	}
 	p = create_proc_entry(idev->dev->name, S_IRUGO, proc_net_devsnmp6);
 	if (!p)
-		goto err_proc;
+		return -ENOMEM;
+
 	p->data = idev;
 	p->proc_fops = &snmp6_seq_fops;
 
 	idev->stats.proc_dir_entry = p;
 	return 0;
-
-err_proc:
-	snmp6_mib_free((void **)idev->stats.icmpv6);
-err_icmp:
-	return err;
 }
 
 int snmp6_unregister_dev(struct inet6_dev *idev)
@@ -238,8 +228,6 @@
 		return -EINVAL;
 	remove_proc_entry(idev->stats.proc_dir_entry->name,
 			  proc_net_devsnmp6);
-	snmp6_mib_free((void **)idev->stats.icmpv6);
-
 	return 0;
 }
 
@@ -280,6 +268,17 @@
 
 int snmp6_register_dev(struct inet6_dev *idev)
 {
+	return 0;
+}
+
+int snmp6_unregister_dev(struct inet6_dev *idev)
+{
+	return 0;
+}
+#endif	/* CONFIG_PROC_FS */
+
+int snmp6_alloc_dev(struct inet6_dev *idev)
+{
 	int err = -ENOMEM;
 
 	if (!idev || !idev->dev)
@@ -295,11 +294,10 @@
 	return err;
 }
 
-int snmp6_unregister_dev(struct inet6_dev *idev)
+int snmp6_free_dev(struct inet6_dev *idev)
 {
 	snmp6_mib_free((void **)idev->stats.icmpv6);
 	return 0;
 }
 
-#endif
 


-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: Frequent Oops on Shutdown 2.6.10

[Herbert Xu]

On Thu, Feb 24, 2005 at 01:41:48AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
> 
> Here's the (hopefuly) final one.

Looks good to me.

> Signed-off-by: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>

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

Thanks,
-- 
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

Re: Frequent Oops on Shutdown 2.6.10

[David S. Miller]

On Thu, 24 Feb 2005 09:48:27 +1100
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Thu, Feb 24, 2005 at 01:41:48AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
> > 
> > Here's the (hopefuly) final one.
> 
> Looks good to me.
> 
> > Signed-off-by: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied to 2.6.11, thanks everyone.