Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755374AbaJ2Sdd (ORCPT ); Wed, 29 Oct 2014 14:33:33 -0400 Received: from www.linutronix.de ([62.245.132.108]:58134 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751226AbaJ2Sdb (ORCPT ); Wed, 29 Oct 2014 14:33:31 -0400 Date: Wed, 29 Oct 2014 19:33:00 +0100 (CET) From: Thomas Gleixner To: Peter Zijlstra cc: Sabrina Dubroca , 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 In-Reply-To: <20141029180734.GQ12706@worktop.programming.kicks-ass.net> Message-ID: References: <20141029155620.GA4886@kria> <20141029180734.GQ12706@worktop.programming.kicks-ass.net> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 29 Oct 2014, Peter Zijlstra wrote: > On Wed, Oct 29, 2014 at 04:56:20PM +0100, Sabrina Dubroca wrote: > > commit e22b886a8a43b ("sched/wait: Add might_sleep() checks") included > > in today's linux-next added a check that fires on e1000 with netpoll: > > > > > > BUG: sleeping function called from invalid context at kernel/irq/manage.c:104 > > in_atomic(): 1, irqs_disabled(): 1, pid: 1, name: systemd > > no locks held by systemd/1. > > irq event stamp: 10102965 > > hardirqs last enabled at (10102965): [] vprintk_emit+0x2dd/0x6a0 > > hardirqs last disabled at (10102964): [] vprintk_emit+0x77/0x6a0 > > softirqs last enabled at (10102342): [] __do_softirq+0x27a/0x6f0 > > softirqs last disabled at (10102337): [] irq_exit+0x56/0xe0 > > Preemption disabled at:[] printk_emit+0x31/0x33 > > > > CPU: 1 PID: 1 Comm: systemd Not tainted 3.18.0-rc2-next-20141029-dirty #222 > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140617_173321-var-lib-archbuild-testing-x86_64-tobias 04/01/2014 > > ffffffff81a82291 ffff88001e743978 ffffffff817df31d 0000000000000000 > > 0000000000000000 ffff88001e7439a8 ffffffff8108dfa2 ffff88001e7439a8 > > ffffffff81a82291 0000000000000068 0000000000000000 ffff88001e7439d8 > > Call Trace: > > [] dump_stack+0x4f/0x7c > > [] ___might_sleep+0x182/0x2b0 > > [] __might_sleep+0x3a/0xc0 > > [] synchronize_irq+0x38/0xa0 > > [] disable_irq+0x20/0x30 > > [] e1000_netpoll+0x23/0x60 > > [] netpoll_poll_dev+0x72/0x3a0 > > [] netpoll_send_skb_on_dev+0x1b7/0x2e0 > > [] netpoll_send_udp+0x2e3/0x490 > > Oh cute.. not entirely sure what to do there. This only works if you > _know_ desc->threads_active will never be !0. > > The best I can come up with is something like this, which avoids the > might_sleep() in the one special case. > > Thomas? 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. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/