Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751273AbbBEVOu (ORCPT ); Thu, 5 Feb 2015 16:14:50 -0500 Received: from hygieia.santi-shop.eu ([78.46.175.2]:41219 "EHLO hygieia.santi-shop.eu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750823AbbBEVOs convert rfc822-to-8bit (ORCPT ); Thu, 5 Feb 2015 16:14:48 -0500 Date: Thu, 5 Feb 2015 22:14:36 +0100 From: Bruno =?UTF-8?B?UHLDqW1vbnQ=?= To: Linus Torvalds Cc: "Rafael J. Wysocki" , Ingo Molnar , Peter Hurley , Davidlohr Bueso , Peter Zijlstra , Linux Kernel Mailing List , Thomas Gleixner , Ilya Dryomov , Mike Galbraith , Oleg Nesterov Subject: Re: Linux 3.19-rc5 Message-ID: <20150205221436.01bc24f2@neptune.home> In-Reply-To: References: <1421878320.4903.17.camel@stgolabs.net> <54C02E08.4080405@hurleysoftware.com> <1861286.x5DC37NGWz@vostro.rjw.lan> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; i686-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4115 Lines: 92 On Thu, 29 January 2015 Linus Torvalds wrote: > On Thu, Jan 29, 2015 at 5:12 PM, Linus Torvalds wrote: > > > > The WARN() was already changed to a WARN_ONCE(). > > Oh, but I notice that the "__set_current_state(TASK_RUNNING) ends up > always happening. > > So I think the right fix is to: > > - warn once like we do > > - but *not* do that __set_current_state() which was always total crap anyway > > Why do I say "total crap"? Because of two independent issues: > > (a) it actually changes behavior for a debug vs non-debug kernel, > which is a really bad idea to begin with > > (b) it's really wrong. The whole "nested sleep" case was never a > major bug to begin with, just a possible inefficiency where constant > nested sleeps would possibly make the outer sleep not sleep. But that > "could possibly make" case was the unlikely case, and the debug patch > made it happen *all* the time by explicitly setting things running. > > So I think the proper patch is the attached. > > The comment is also crap. The comment says > > "Blocking primitives will set (and therefore destroy) current->state [...]" > > but the reality is that they *may* set it, and only in the unlikely > slow-path where they actually block. > > So doing this in "__may_sleep()" is just bogus and horrible horrible > crap. It turns the "harmless ugliness" into a real *harmful* bug. The > key word of "__may_sleep()" is that "MAY" part. It's a debug thing to > make relatively rare cases show up. > > PeterZ, please don't make "debugging" patches like this. Ever again. > Because this was just stupid, and it took me too long to realize that > despite the warning being shut up, the debug patch was still actively > doing bad bad things. > > Ingo, maybe you'd want to apply this through the scheduler tree, the > way you already did the WARN_ONCE() thing. > > Bruno, does this finally actually fix your pccard thing? Tested the variant that was applied by running rc7 and it fixes the endless loop. The loop is now replaced by a single WARN() trace - I guess expected: [ 3.083647] ------------[ cut here ]------------ [ 3.087477] WARNING: CPU: 0 PID: 67 at /usr/src/linux-git/kernel/sched/core.c:7300 __might_sleep+0x79/0x90() [ 3.091357] do not call blocking ops when !TASK_RUNNING; state=1 set at [] pccardd+0xa0/0x3e0 [ 3.095232] Modules linked in: [ 3.099020] CPU: 0 PID: 67 Comm: pccardd Not tainted 3.19.0-rc7-00003-g67288c4 #17 [ 3.102760] Hardware name: Acer TravelMate 660/TravelMate 660, BIOS 3A19 01/14/2004 [ 3.106504] c212def4 c212def4 c212deb4 c16caf23 c212dee4 c10416fd c1907334 c212df10 [ 3.110315] 00000043 c1907380 00001c84 c105a099 c105a099 c1442040 00000001 f5f54bc0 [ 3.114143] c212defc c104176e 00000009 c212def4 c1907334 c212df10 c212df30 c105a099 [ 3.117960] Call Trace: [ 3.121703] [] dump_stack+0x16/0x18 [ 3.125447] [] warn_slowpath_common+0x7d/0xc0 [ 3.129172] [] ? __might_sleep+0x79/0x90 [ 3.132868] [] ? __might_sleep+0x79/0x90 [ 3.136500] [] ? pccardd+0xa0/0x3e0 [ 3.140092] [] warn_slowpath_fmt+0x2e/0x30 [ 3.143657] [] __might_sleep+0x79/0x90 [ 3.147209] [] ? pccardd+0xa0/0x3e0 [ 3.150747] [] ? pccardd+0xa0/0x3e0 [ 3.154256] [] mutex_lock+0x17/0x2a [ 3.157734] [] pccardd+0xe9/0x3e0 [ 3.161207] [] ? pcmcia_socket_uevent+0x30/0x30 [ 3.164660] [] kthread+0xa0/0xc0 [ 3.168059] [] ret_from_kernel_thread+0x20/0x30 [ 3.171436] [] ? kthread_worker_fn+0x140/0x140 [ 3.174796] ---[ end trace c3f708b642e3d8f0 ]--- >From my reading of the thread fixing pccardd/sched TASK_RUNNING usage/check is another issue left for the future. Thanks, Bruno > Linus -- 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/