linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Laurent Dufour <ldufour@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@osdl.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: RFC: SysRq nice-all-RT-tasks is broken
Date: Wed, 8 Mar 2017 12:40:12 -0500	[thread overview]
Message-ID: <20170308124012.496a75cf@gandalf.local.home> (raw)
In-Reply-To: <22b0e5fe-2aa1-72de-4d00-29d768d226ca@linux.vnet.ibm.com>


[
  Added Peter

   Update: Laurent noticed that sysrq 'n' (nice-all-RT-tasks) calls
   __sched_setscheduler() form interrupt context. At the start of that
   function, there's a BUG_ON(in_interrupt()). The reason for that was
   due to the rt mutex pi code calling wait_lock. Which was not irq
   safe. Now it is, but that's not good enough.
]

On Wed, 8 Mar 2017 18:03:55 +0100
Laurent Dufour <ldufour@linux.vnet.ibm.com> wrote:

> On 08/03/2017 17:57, Steven Rostedt wrote:
> > On Wed, 8 Mar 2017 11:51:14 -0500
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> >   
> >> Hmm, that commit was added in 2.6.18, and you're right, a lot has
> >> changed since then. Have you tried removing it and running it under
> >> lockdep, and see if it triggers any warnings?  
> > 
> > I did a little digging, and it appears that its the rt mutex wait lock
> > that the comment was referring to. Today that spin lock is irq safe. I
> > believe its safe to remove the BUG_ON(). Want me to send a patch?  
> 
> Sure, go ahead ;)
>

Actually, it's still not safe :-/

I just noticed this in the call path:

	raw_spin_unlock_irq(&task->pi_lock);

As well as other raw_spin_unlock_irq()s.

Which would enable interrupts regardless of the previous state.

One solution is to change all those to irqsave() but that seems to be a
big step for something that is rarely done (how many years has it been
since 2.6.18?).

I wonder if we should just have a special flag sent by that sysrq
trigger. Since it is causing all tasks to go "nice" there's no need to
do the pi chain walk in __sched_setscheduler().

-- Steve

  reply	other threads:[~2017-03-08 17:40 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-08 15:23 RFC: SysRq nice-all-RT-tasks is broken Laurent Dufour
2017-03-08 16:51 ` Steven Rostedt
2017-03-08 16:57   ` Steven Rostedt
2017-03-08 17:03     ` Laurent Dufour
2017-03-08 17:40       ` Steven Rostedt [this message]
2017-03-08 17:46         ` Steven Rostedt
2017-03-09  9:02           ` Laurent Dufour
2017-05-23  8:46           ` [tip:sched/core] sched/core: Allow __sched_setscheduler() in interrupts when PI is not used tip-bot for Steven Rostedt (VMware)

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=20170308124012.496a75cf@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=akpm@osdl.org \
    --cc=ldufour@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).