Message ID | m1ljo61cya.fsf@fess.ebiederm.org |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Fri, 2009-06-05 at 16:24 -0700, Eric W. Biederman wrote: > The patch below fixes the one bug I know of in with just my changes > applied. I don't think it fixes everything with Herberts changes. > > I expect some of this will need to be moved to release to fix the > select problem on 2.6.30-rc8. It certainly seems to fix the BUG in unregister_netdevice() which was the failure mode I was seeing most often in 2.6.30-rc8. Thanks. Tested-by: David Woodhouse <David.Woodhouse@intel.com>
From: David Woodhouse <dwmw2@infradead.org> Date: Sat, 06 Jun 2009 08:06:11 +0100 > On Fri, 2009-06-05 at 16:24 -0700, Eric W. Biederman wrote: >> The patch below fixes the one bug I know of in with just my changes >> applied. I don't think it fixes everything with Herberts changes. >> >> I expect some of this will need to be moved to release to fix the >> select problem on 2.6.30-rc8. > > It certainly seems to fix the BUG in unregister_netdevice() which was > the failure mode I was seeing most often in 2.6.30-rc8. Thanks. > > Tested-by: David Woodhouse <David.Woodhouse@intel.com> Applied, thanks! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Eric W. Biederman <ebiederm@xmission.com> wrote: > > --- linux-2.6.28.x86_64-old/drivers/net/tun.c 2009-05-06 15:01:56.000000000 -0700 > +++ linux-2.6.28.x86_64/drivers/net/tun.c 2009-05-06 15:10:09.000000000 -0700 > @@ -1194,21 +1194,22 @@ > static int tun_chr_close(struct inode *inode, struct file *file) > { > struct tun_file *tfile = file->private_data; > - struct tun_struct *tun = __tun_get(tfile); > + struct tun_struct *tun; > > > + rtnl_lock(); > + tun = __tun_get(tfile); > if (tun) { > DBG(KERN_INFO "%s: tun_chr_close\n", tun->dev->name); > > - rtnl_lock(); > __tun_detach(tun); > > /* If desireable, unregister the netdevice. */ > if (!(tun->flags & TUN_PERSIST)) > unregister_netdevice(tun->dev); > > - rtnl_unlock(); > } > + rtnl_unlock(); Sorry, catching up with emails (hmm, maybe I should stop doing that and read some new emails :) This just turns a wide-open race into a less likely one (that's why it appears to fix the problem). The crux of the issue is that __tun_get(tfile) != NULL has nothing to do with whether the device has been unregistered. Cheers,
--- linux-2.6.28.x86_64-old/drivers/net/tun.c 2009-05-06 15:01:56.000000000 -0700 +++ linux-2.6.28.x86_64/drivers/net/tun.c 2009-05-06 15:10:09.000000000 -0700 @@ -1194,21 +1194,22 @@ static int tun_chr_close(struct inode *inode, struct file *file) { struct tun_file *tfile = file->private_data; - struct tun_struct *tun = __tun_get(tfile); + struct tun_struct *tun; + rtnl_lock(); + tun = __tun_get(tfile); if (tun) { DBG(KERN_INFO "%s: tun_chr_close\n", tun->dev->name); - rtnl_lock(); __tun_detach(tun); /* If desireable, unregister the netdevice. */ if (!(tun->flags & TUN_PERSIST)) unregister_netdevice(tun->dev); - rtnl_unlock(); } + rtnl_unlock(); put_net(tfile->net); kfree(tfile);