From: Flavio Leitner <fbl@sysclose.org>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org, amwang@redhat.com, fubar@us.ibm.com,
mpm@selenic.com, gospo@redhat.com, nhorman@tuxdriver.com,
jmoyer@redhat.com, shemminger@linux-foundation.org,
linux-kernel@vger.kernel.org, bridge@lists.linux-foundation.org,
bonding-devel@lists.sourceforge.net
Subject: Re: [PATCH] netconsole: queue console messages to send later
Date: Mon, 7 Jun 2010 21:37:07 -0300 [thread overview]
Message-ID: <20100608003707.GA30604@sysclose.org> (raw)
In-Reply-To: <20100607.165024.135517125.davem@davemloft.net>
On Mon, Jun 07, 2010 at 04:50:24PM -0700, David Miller wrote:
> From: Flavio Leitner <fleitner@redhat.com>
> Date: Mon, 7 Jun 2010 16:24:52 -0300
>
> > There are some networking drivers that hold a lock in the
> > transmit path. Therefore, if a console message is printed
> > after that, netconsole will push it through the transmit path,
> > resulting in a deadlock.
> >
> > This patch fixes the re-injection problem by queuing the console
> > messages in a preallocated circular buffer and then scheduling a
> > workqueue to send them later with another context.
> >
> > Signed-off-by: Flavio Leitner <fleitner@redhat.com>
>
> You absolutely and positively MUST NOT do this. Otherwise netconsole
> becomes completely useless. Your idea has been proposed several times
> as far back as 6 years ago, it was unacceptable then and it's
> unacceptable now.
>
> The whole point of netconsole is that we may be deep in an interrupt
> or other atomic context, the machine is about to hard hang, and it's
> absolutely essential that we get out any and all kernel logging
> messages that we can, immediately.
Got it. I've never assumed that netconsole would work reliable on
such situations, so I thought as we have better ways now it would
be helpful. See another idea below.
> There may not be another timer or workqueue able to execute after the
> printk() we're trying to emit. We may never get to that point.
What if in the netpoll, before we push the skb to the driver, we check
for a bit saying that it's already pushing another skb. In this case,
queue the new skb inside of netpoll and soon as the first call returns
and try to clear the bit, it will send the next skb?
printk("message 1")
...
netconsole called
netpoll sets the flag bit
pushes to the bonding driver which does another printk("message 2")
netconsole called again
netpoll checks for the flag, queue the message, returns.
so, bonding can finish up to send the first message
netpoll is about to return, checks for new queued messages, and pushes them.
bonding finishes up to send the second message
....
No deadlocks, skbs are ordered and still under the same opportunity
to send something. Does it sound acceptable?
It's off the top of my head, so probably this idea has some problems.
> Fix the locking in the drivers or layers that cause the issue instead
> of breaking netconsole.
Someday, somewhere, I know because I did this before, someone will
use a debugging printk() and will see the entire box hanging with
absolutely no message in any console because of this problem.
I'm not saying that fixing driver isn't the right way to go but
it seems not enough to me.
--
Flavio
next prev parent reply other threads:[~2010-06-08 0:37 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-05 8:11 [v5 Patch 1/3] netpoll: add generic support for bridge and bonding devices Amerigo Wang
2010-05-05 8:11 ` [v5 Patch 2/3] bridge: make bridge support netpoll Amerigo Wang
2010-05-05 8:11 ` [v5 Patch 3/3] bonding: make bonding " Amerigo Wang
2010-05-06 2:05 ` [v5 Patch 1/3] netpoll: add generic support for bridge and bonding devices Matt Mackall
2010-05-06 7:44 ` David Miller
2010-05-07 3:24 ` Cong Wang
2010-05-27 18:05 ` Flavio Leitner
2010-05-27 20:35 ` David Miller
2010-05-27 21:25 ` Flavio Leitner
2010-05-28 2:47 ` Cong Wang
2010-05-28 19:40 ` Flavio Leitner
2010-05-31 5:56 ` Cong Wang
2010-05-31 19:08 ` Flavio Leitner
2010-06-01 9:57 ` Cong Wang
2010-06-01 18:42 ` Jay Vosburgh
2010-06-02 10:04 ` Cong Wang
2010-06-04 19:18 ` Andy Gospodarek
2010-06-07 9:57 ` Cong Wang
2010-06-07 10:01 ` David Miller
2010-06-08 8:36 ` Cong Wang
2010-06-07 13:03 ` Andy Gospodarek
2010-06-08 8:38 ` Cong Wang
2010-06-07 19:24 ` [PATCH] netconsole: queue console messages to send later Flavio Leitner
2010-06-07 19:50 ` Matt Mackall
2010-06-07 20:00 ` Stephen Hemminger
2010-06-07 20:21 ` Matt Mackall
2010-06-07 23:52 ` David Miller
2010-06-07 23:50 ` David Miller
2010-06-08 0:37 ` Flavio Leitner [this message]
2010-06-08 8:59 ` Cong Wang
2010-05-28 8:16 ` [v5 Patch 1/3] netpoll: add generic support for bridge and bonding devices Cong Wang
2010-05-28 20:42 ` Flavio Leitner
2010-05-28 21:03 ` Jay Vosburgh
2010-05-31 5:29 ` Cong Wang
2010-05-31 5:37 ` Cong Wang
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=20100608003707.GA30604@sysclose.org \
--to=fbl@sysclose.org \
--cc=amwang@redhat.com \
--cc=bonding-devel@lists.sourceforge.net \
--cc=bridge@lists.linux-foundation.org \
--cc=davem@davemloft.net \
--cc=fubar@us.ibm.com \
--cc=gospo@redhat.com \
--cc=jmoyer@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mpm@selenic.com \
--cc=netdev@vger.kernel.org \
--cc=nhorman@tuxdriver.com \
--cc=shemminger@linux-foundation.org \
/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;
as well as URLs for NNTP newsgroup(s).