lkml.org 
[lkml]   [2015]   [Jan]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: Linux 3.19-rc5
From
On Thu, Jan 29, 2015 at 5:12 PM, Linus Torvalds
<torvalds@linux-foundation.org> 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?

Linus
kernel/sched/core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c0accc00566e..76aaf5c61114 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -7292,13 +7292,12 @@ void __might_sleep(const char *file, int line, int preempt_offset)
* since we will exit with TASK_RUNNING make sure we enter with it,
* otherwise we will destroy state.
*/
- if (WARN_ONCE(current->state != TASK_RUNNING,
+ WARN_ONCE(current->state != TASK_RUNNING,
"do not call blocking ops when !TASK_RUNNING; "
"state=%lx set at [<%p>] %pS\n",
current->state,
(void *)current->task_state_change,
- (void *)current->task_state_change))
- __set_current_state(TASK_RUNNING);
+ (void *)current->task_state_change);

___might_sleep(file, line, preempt_offset);
}
\
 
 \ /
  Last update: 2015-01-30 02:41    [W:0.278 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site