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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.