All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: "Linus Torvalds" <torvalds@linux-foundation.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	"Ingo Molnar" <mingo@kernel.org>,
	"Peter Hurley" <peter@hurleysoftware.com>,
	"Davidlohr Bueso" <dave@stgolabs.net>,
	"Bruno Prémont" <bonbons@linux-vserver.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Ilya Dryomov" <ilya.dryomov@inktank.com>,
	"Mike Galbraith" <umgwanakikbuti@gmail.com>
Subject: Re: Linux 3.19-rc5
Date: Sun, 1 Feb 2015 20:43:06 +0100	[thread overview]
Message-ID: <20150201194306.GA29993@redhat.com> (raw)
In-Reply-To: <20150131201601.GZ2896@worktop.programming.kicks-ass.net>

On 01/31, Peter Zijlstra wrote:
>
> On Sat, Jan 31, 2015 at 10:32:23AM -0800, Linus Torvalds wrote:
> > On Fri, Jan 30, 2015 at 7:47 AM, Oleg Nesterov <oleg@redhat.com> wrote:
> > >
> > > Perhaps sched_annotate_sleep() shouldn't depend on CONFIG_DEBUG_ATOMIC_SLEEP
> > > too...
> >
> > Ugh. That thing is horrible. The naming doesn't make it obvious at all
> > that it's actually making sure that we have state set to TASK_RUNNING,
> > and I could easily imagine that it would cause similar "busy-loops
> > while scheduling" issues if anybody ever uses it in the wrong context.
>
> The alternative was putting unconditional __set_task_state(TASK_RUNNING)
> things in a few code paths. I didn't want to cause the extra code in
> case we didn't need them. Particularly the include/net/sock.h one, as I
> know the network people are cycle counters.

And personally I agree. sched_annotate_sleep() looks self-documented, it
is clear that it is used to suppress the warning.

Still. Can't we avoid this subtle change in behaviour DEBUG_ATOMIC_SLEEP
adds?

Oleg.


--- x/include/linux/kernel.h
+++ x/include/linux/kernel.h
@@ -176,7 +176,7 @@ extern int _cond_resched(void);
  */
 # define might_sleep() \
 	do { __might_sleep(__FILE__, __LINE__, 0); might_resched(); } while (0)
-# define sched_annotate_sleep()	__set_current_state(TASK_RUNNING)
+# define sched_annotate_sleep()	(current->task_state_change = 0)
 #else
   static inline void ___might_sleep(const char *file, int line,
 				   int preempt_offset) { }
--- x/kernel/sched/core.c
+++ x/kernel/sched/core.c
@@ -7292,7 +7292,7 @@ void __might_sleep(const char *file, int
 	 * 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,
+	if (WARN_ONCE(current->state != TASK_RUNNING && current->task_state_change,
 			"do not call blocking ops when !TASK_RUNNING; "
 			"state=%lx set at [<%p>] %pS\n",
 			current->state,


  parent reply	other threads:[~2015-02-01 19:45 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-18  9:17 Linux 3.19-rc5 Linus Torvalds
2015-01-19 18:02 ` Bruno Prémont
2015-01-20  6:24   ` Linus Torvalds
2015-01-21 20:37     ` Bruno Prémont
2015-01-21 21:37       ` Bruno Prémont
2015-01-21 22:12         ` Davidlohr Bueso
2015-01-21 22:54           ` Peter Hurley
2015-01-30  1:22             ` Rafael J. Wysocki
2015-01-30  1:12               ` Linus Torvalds
2015-01-30  1:25                 ` Linus Torvalds
2015-01-30  1:35                   ` Linus Torvalds
2015-01-30  2:06                   ` Rafael J. Wysocki
2015-01-30 15:47                   ` Oleg Nesterov
2015-01-31 18:32                     ` Linus Torvalds
2015-01-31 20:16                       ` Peter Zijlstra
2015-01-31 21:54                         ` Linus Torvalds
2015-02-02 13:19                           ` Peter Zijlstra
2015-02-01 19:43                         ` Oleg Nesterov [this message]
2015-02-01 20:09                           ` Linus Torvalds
2015-02-01 20:19                             ` Oleg Nesterov
2015-02-02 15:34                             ` Peter Zijlstra
2015-01-31  9:16                   ` Bruno Prémont
2015-01-31  9:48                   ` Peter Zijlstra
2015-02-03 11:18                     ` Ingo Molnar
2015-02-05 21:14                   ` Bruno Prémont
2015-02-06 11:50                     ` Peter Zijlstra
2015-02-06 16:02                       ` Linus Torvalds
2015-02-06 16:39                         ` Peter Zijlstra
2015-02-06 16:46                           ` Linus Torvalds
2015-01-30  1:49                 ` Rafael J. Wysocki
2015-02-02  9:48                   ` Zdenek Kabelac

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=20150201194306.GA29993@redhat.com \
    --to=oleg@redhat.com \
    --cc=bonbons@linux-vserver.org \
    --cc=dave@stgolabs.net \
    --cc=ilya.dryomov@inktank.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peter@hurleysoftware.com \
    --cc=peterz@infradead.org \
    --cc=rjw@rjwysocki.net \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=umgwanakikbuti@gmail.com \
    /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.