From: Andrew Morton <akpm@digeo.com>
To: "David S. Miller" <davem@redhat.com>
Cc: lists@mdiehl.de, linux-kernel@vger.kernel.org, jt@hpl.hp.com
Subject: Re: [2.5.69] rtnl-deadlock with usermodehelper and keventd
Date: Thu, 15 May 2003 18:12:11 -0700 [thread overview]
Message-ID: <20030515181211.5853fd18.akpm@digeo.com> (raw)
In-Reply-To: <20030515.175348.45895365.davem@redhat.com>
"David S. Miller" <davem@redhat.com> wrote:
>
>
> Way too invasive, and this adds bugs to the ipmr.c code.
Thought you'd say that.
My mail client munched the context. For reference:
> Martin Diehl <lists@mdiehl.de> wrote:
> >
> >
> > [ unregister_netdevice calls call_usermodehelper which waits for keventd to
> > pick up the subprocess_info, but keventd is blocked on rtnl_lock, which
> > unregister_netdev took ]
> >
>
> The nice way to fix this is to change unregister_netdev so it runs
> net_run_sbin_hotplug() outside rtnl_lock.
>
> Problem is, it's hard. I just knocked up the below patch, and it still
> needs work. Mainly because of the tremendously deep codepaths which call
> unregister_netdevice(), knowing that they are under rtnl_lock().
>
> It would be nice to clean all that up, and this patch actually contains
> good cleanups and a couple of bugfixes. But I don't think it's going to
> get there.
>
>
> The other way to fix it is to make call_usermodehelper() more async. That
> means kmallocing the sub_info, the work struct and the string arrays and
> all the strings. This is more general, and will probably fix other
> keventd-related deadlocks. But it is unattractive.
>
>
> Or we could change linkwatch to not take rtnl_lock() by some means. That's
> even less general.
>
>
> David, any comments? You think the below approach shuld be pursued?
"David S. Miller" <davem@redhat.com> wrote:
>
> I'd much rather see /sbin/hotplug be able to handle things
> asynchonously.
Yeah, I'm inclined to agree. I'll take a look at it.
Meanwhile please take a look at the leftover cleanups. It fixes a bug in
drivers/net/hamradio/dmascc.c too.
25-akpm/drivers/net/hamradio/dmascc.c | 4 +---
25-akpm/drivers/net/irda/ali-ircc.c | 7 ++-----
25-akpm/drivers/net/irda/donauboe.c | 7 +------
25-akpm/drivers/net/irda/irda-usb.c | 10 ++++------
25-akpm/drivers/net/irda/irport.c | 7 ++-----
25-akpm/drivers/net/irda/irtty.c | 7 ++-----
25-akpm/drivers/net/irda/nsc-ircc.c | 7 ++-----
25-akpm/drivers/net/irda/sa1100_ir.c | 7 ++-----
25-akpm/drivers/net/irda/toshoboe.c | 8 ++------
25-akpm/drivers/net/irda/w83977af_ir.c | 7 ++-----
10 files changed, 20 insertions(+), 51 deletions(-)
diff -puN drivers/net/hamradio/dmascc.c~unregister_netdev-cleanup drivers/net/hamradio/dmascc.c
--- 25/drivers/net/hamradio/dmascc.c~unregister_netdev-cleanup Thu May 15 18:05:29 2003
+++ 25-akpm/drivers/net/hamradio/dmascc.c Thu May 15 18:05:29 2003
@@ -325,9 +325,7 @@ void cleanup_module(void) {
/* Unregister devices */
for (i = 0; i < 2; i++) {
if (info->dev[i].name)
- rtnl_lock();
- unregister_netdevice(&info->dev[i]);
- rtnl_unlock();
+ unregister_netdev(&info->dev[i]);
}
/* Reset board */
diff -puN drivers/net/irda/ali-ircc.c~unregister_netdev-cleanup drivers/net/irda/ali-ircc.c
--- 25/drivers/net/irda/ali-ircc.c~unregister_netdev-cleanup Thu May 15 18:05:29 2003
+++ 25-akpm/drivers/net/irda/ali-ircc.c Thu May 15 18:05:29 2003
@@ -390,11 +390,8 @@ static int __exit ali_ircc_close(struct
iobase = self->io.fir_base;
/* Remove netdevice */
- if (self->netdev) {
- rtnl_lock();
- unregister_netdevice(self->netdev);
- rtnl_unlock();
- }
+ if (self->netdev)
+ unregister_netdev(self->netdev);
/* Release the PORT that this driver is using */
IRDA_DEBUG(4, "%s(), Releasing Region %03x\n", __FUNCTION__, self->io.fir_base);
diff -puN drivers/net/irda/donauboe.c~unregister_netdev-cleanup drivers/net/irda/donauboe.c
--- 25/drivers/net/irda/donauboe.c~unregister_netdev-cleanup Thu May 15 18:05:29 2003
+++ 25-akpm/drivers/net/irda/donauboe.c Thu May 15 18:05:29 2003
@@ -1577,12 +1577,7 @@ toshoboe_close (struct pci_dev *pci_dev)
}
if (self->netdev)
- {
- /* Remove netdevice */
- rtnl_lock ();
- unregister_netdevice (self->netdev);
- rtnl_unlock ();
- }
+ unregister_netdev(self->netdev);
kfree (self->ringbuf);
self->ringbuf = NULL;
diff -puN drivers/net/irda/irda-usb.c~unregister_netdev-cleanup drivers/net/irda/irda-usb.c
--- 25/drivers/net/irda/irda-usb.c~unregister_netdev-cleanup Thu May 15 18:05:29 2003
+++ 25-akpm/drivers/net/irda/irda-usb.c Thu May 15 18:05:29 2003
@@ -1231,12 +1231,10 @@ static inline int irda_usb_close(struct
ASSERT(self != NULL, return -1;);
/* Remove netdevice */
- if (self->netdev) {
- rtnl_lock();
- unregister_netdevice(self->netdev);
- self->netdev = NULL;
- rtnl_unlock();
- }
+ if (self->netdev)
+ unregister_netdev(self->netdev);
+ self->netdev = NULL;
+
/* Remove the speed buffer */
if (self->speed_buff != NULL) {
kfree(self->speed_buff);
diff -puN drivers/net/irda/irport.c~unregister_netdev-cleanup drivers/net/irda/irport.c
--- 25/drivers/net/irda/irport.c~unregister_netdev-cleanup Thu May 15 18:05:29 2003
+++ 25-akpm/drivers/net/irda/irport.c Thu May 15 18:05:29 2003
@@ -256,11 +256,8 @@ int irport_close(struct irport_cb *self)
self->dongle = NULL;
/* Remove netdevice */
- if (self->netdev) {
- rtnl_lock();
- unregister_netdevice(self->netdev);
- rtnl_unlock();
- }
+ if (self->netdev)
+ unregister_netdev(self->netdev);
/* Release the IO-port that this driver is using */
IRDA_DEBUG(0 , "%s(), Releasing Region %03x\n",
diff -puN drivers/net/irda/irtty.c~unregister_netdev-cleanup drivers/net/irda/irtty.c
--- 25/drivers/net/irda/irtty.c~unregister_netdev-cleanup Thu May 15 18:05:29 2003
+++ 25-akpm/drivers/net/irda/irtty.c Thu May 15 18:05:29 2003
@@ -282,11 +282,8 @@ static void irtty_close(struct tty_struc
self->dongle = NULL;
/* Remove netdevice */
- if (self->netdev) {
- rtnl_lock();
- unregister_netdevice(self->netdev);
- rtnl_unlock();
- }
+ if (self->netdev)
+ unregister_netdev(self->netdev);
self = hashbin_remove(irtty, (int) self, NULL);
diff -puN drivers/net/irda/nsc-ircc.c~unregister_netdev-cleanup drivers/net/irda/nsc-ircc.c
--- 25/drivers/net/irda/nsc-ircc.c~unregister_netdev-cleanup Thu May 15 18:05:29 2003
+++ 25-akpm/drivers/net/irda/nsc-ircc.c Thu May 15 18:05:29 2003
@@ -391,11 +391,8 @@ static int __exit nsc_ircc_close(struct
iobase = self->io.fir_base;
/* Remove netdevice */
- if (self->netdev) {
- rtnl_lock();
- unregister_netdevice(self->netdev);
- rtnl_unlock();
- }
+ if (self->netdev)
+ unregister_netdev(self->netdev);
/* Release the PORT that this driver is using */
IRDA_DEBUG(4, "%s(), Releasing Region %03x\n",
diff -puN drivers/net/irda/sa1100_ir.c~unregister_netdev-cleanup drivers/net/irda/sa1100_ir.c
--- 25/drivers/net/irda/sa1100_ir.c~unregister_netdev-cleanup Thu May 15 18:05:29 2003
+++ 25-akpm/drivers/net/irda/sa1100_ir.c Thu May 15 18:05:29 2003
@@ -1122,11 +1122,8 @@ static void __exit sa1100_irda_exit(void
{
struct net_device *dev = dev_get_drvdata(&sa1100ir_device.dev);
- if (dev) {
- rtnl_lock();
- unregister_netdevice(dev);
- rtnl_unlock();
- }
+ if (dev)
+ unregister_netdev(dev);
sys_device_unregister(&sa1100ir_device);
driver_unregister(&sa1100ir_driver);
diff -puN drivers/net/irda/toshoboe.c~unregister_netdev-cleanup drivers/net/irda/toshoboe.c
--- 25/drivers/net/irda/toshoboe.c~unregister_netdev-cleanup Thu May 15 18:05:29 2003
+++ 25-akpm/drivers/net/irda/toshoboe.c Thu May 15 18:05:29 2003
@@ -679,12 +679,8 @@ toshoboe_remove (struct pci_dev *pci_dev
self->recv_bufs[i] = NULL;
}
- if (self->netdev) {
- /* Remove netdevice */
- rtnl_lock();
- unregister_netdevice(self->netdev);
- rtnl_unlock();
- }
+ if (self->netdev)
+ unregister_netdev(self->netdev);
kfree (self->taskfilebuf);
self->taskfilebuf = NULL;
diff -puN drivers/net/irda/w83977af_ir.c~unregister_netdev-cleanup drivers/net/irda/w83977af_ir.c
--- 25/drivers/net/irda/w83977af_ir.c~unregister_netdev-cleanup Thu May 15 18:05:29 2003
+++ 25-akpm/drivers/net/irda/w83977af_ir.c Thu May 15 18:05:29 2003
@@ -299,11 +299,8 @@ static int w83977af_close(struct w83977a
#endif /* CONFIG_USE_W977_PNP */
/* Remove netdevice */
- if (self->netdev) {
- rtnl_lock();
- unregister_netdevice(self->netdev);
- rtnl_unlock();
- }
+ if (self->netdev)
+ unregister_netdev(self->netdev);
/* Release the PORT that this driver is using */
IRDA_DEBUG(0 , "%s(), Releasing Region %03x\n",
_
next prev parent reply other threads:[~2003-05-16 1:04 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <PAO-EX01Cv3uS7sBdxk00001183@pao-ex01.pao.digeo.com>
2003-05-16 0:53 ` David S. Miller
2003-05-16 1:12 ` Andrew Morton [this message]
2003-05-16 1:27 ` [2.5.69] rtnl-deadlock with usermodehelper and keventd Jean Tourrilhes
2003-05-23 7:06 ` Martin Diehl
2003-05-23 6:59 ` David S. Miller
2003-05-23 9:38 ` Martin Diehl
2003-05-23 9:43 ` David S. Miller
2003-05-23 14:42 ` Stian Jordet
2003-05-23 16:46 ` Jean Tourrilhes
2003-05-23 23:25 ` Martin Diehl
2003-05-15 13:14 Martin Diehl
2003-05-15 20:12 ` Jean Tourrilhes
2003-05-15 20:19 ` Greg KH
2003-05-15 20:25 ` Jean Tourrilhes
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=20030515181211.5853fd18.akpm@digeo.com \
--to=akpm@digeo.com \
--cc=davem@redhat.com \
--cc=jt@hpl.hp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lists@mdiehl.de \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox