All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Sabrina Dubroca <sd@queasysnail.net>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	jeffrey.t.kirsher@intel.com
Subject: Re: e1000_netpoll(): disable_irq() triggers might_sleep() on linux-next
Date: Wed, 29 Oct 2014 20:50:54 +0100	[thread overview]
Message-ID: <20141029195054.GH10501@worktop.programming.kicks-ass.net> (raw)
In-Reply-To: <alpine.DEB.2.11.1410292046270.5308@nanos>

On Wed, Oct 29, 2014 at 08:49:03PM +0100, Thomas Gleixner wrote:
> On Wed, 29 Oct 2014, Peter Zijlstra wrote:
> 
> > On Wed, Oct 29, 2014 at 07:33:00PM +0100, Thomas Gleixner wrote:
> > > Yuck. No. You are just papering over the problem.
> > > 
> > > What happens if you add 'threadirqs' to the kernel command line? Or if
> > > the interrupt line is shared with a real threaded interrupt user?
> > > 
> > > The proper solution is to have a poll_lock for e1000 which serializes
> > > the hardware interrupt against netpoll instead of using
> > > disable/enable_irq().
> > > 
> > > In fact that's less expensive than the disable/enable_irq() dance and
> > > the chance of contention is pretty low. If done right it will be a
> > > NOOP for the CONFIG_NET_POLL_CONTROLLER=n case.
> > > 
> > 
> > OK a little something like so then I suppose.. But I suspect most all
> > the network drivers will need this and maybe more, disable_irq() is a
> > popular little thing and we 'just' changed semantics on them.
> 
> We changed that almost 4 years ago :) What we 'just' did was to add a
> prominent warning into the code.

You know that is the same right... they didn't know it was broken
therefore it wasn't :-), but now they need to go actually do stuff about
it, an entirely different proposition.

  reply	other threads:[~2014-10-29 19:50 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-29 15:56 e1000_netpoll(): disable_irq() triggers might_sleep() on linux-next Sabrina Dubroca
2014-10-29 18:07 ` Peter Zijlstra
2014-10-29 18:33   ` Thomas Gleixner
2014-10-29 19:36     ` Peter Zijlstra
2014-10-29 19:40       ` Jeff Kirsher
2014-10-29 19:53         ` Thomas Gleixner
2014-10-29 19:49       ` Thomas Gleixner
2014-10-29 19:50         ` Peter Zijlstra [this message]
2014-10-29 20:07           ` Thomas Gleixner
2014-10-29 20:23             ` Thomas Gleixner
2014-10-29 20:51               ` Peter Zijlstra
2014-10-29 21:03                 ` Thomas Gleixner
2014-12-02 16:35       ` Sabrina Dubroca
2014-12-22 15:28         ` Bart Van Assche
2015-01-05 10:06         ` Bart Van Assche

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=20141029195054.GH10501@worktop.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=sd@queasysnail.net \
    --cc=tglx@linutronix.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.