All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>, LKML <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 2/2] x86: Make the x86-64 stacktrace code safely callable from scheduler
Date: Fri, 13 May 2011 06:48:26 -0400	[thread overview]
Message-ID: <1305283706.22280.40.camel@frodo> (raw)
In-Reply-To: <1305232326-9804-3-git-send-email-fweisbec@gmail.com>

On Thu, 2011-05-12 at 22:32 +0200, Frederic Weisbecker wrote:
> Avoid potential scheduler recursion and deadlock from the
> stacktrace code by avoiding rescheduling when we re-enable
> preemption.

I'm curious to where you saw this deadlock? As I have the function stack
tracer using preempt_disable_notrace and enable_notrace without any
issues, and it traces all functions in the kernel[*]. I have no issue
with using raw_local_irq_save/restore() if it is to protect the per_cpu
variable from interrupt corruption, but I don't see the problem with
recursion.

There's only one function I had to worry about with preempt disable, not
the entire scheduler. That was the function preempt_schedule(). This
function is called by preempt_enable() and that will cause an infinite
loop if you have something in preempt_schedule() call preempt_enable().

Remember that ftrace_preempt_disable/enable() crap that I did to try to
avoid the scheduler deadlock? I found it was complex and unnecessary
because the scheduler itself was not an issue, it was only
preempt_schedule(). I replaced all that crappy code with a single line
that added notrace to preempt_schedule() and everything just worked.

Thus, if you disable interrupts to protect the cpu data, that's fine,
and say so in the change log. I really like to know if you really saw
this deadlock. Yes enabling preemption in the scheduler may recurse, but
it will only do so once.

I still argue that interrupt enabling is slow. I've seen a large slow
down of the code by switching stack tracer from preempt disable to irq
disable. I used perf to see why, and it told me that disabling
interrupts as fine, but enabling interrupts can cost you quite a bit.

-- Steve

[*] of course function tracing does not trace other notrace functions.


  parent reply	other threads:[~2011-05-13 10:48 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-12 20:32 [GIT PULL] x86 stacktrace updates Frederic Weisbecker
2011-05-12 20:32 ` [PATCH 1/2] x86: Remove warning and warning_symbol from struct stacktrace_ops Frederic Weisbecker
2011-07-14 11:01   ` Mel Gorman
2011-07-14 13:50     ` Frederic Weisbecker
2011-05-12 20:32 ` [PATCH 2/2] x86: Make the x86-64 stacktrace code safely callable from scheduler Frederic Weisbecker
2011-05-12 20:40   ` Frederic Weisbecker
2011-05-12 21:28   ` Ingo Molnar
2011-05-12 21:43     ` Frederic Weisbecker
2011-05-12 21:55       ` Ingo Molnar
2011-05-13 10:48   ` Steven Rostedt [this message]
2011-05-13 12:48     ` Frederic Weisbecker
2011-05-13 13:19       ` Peter Zijlstra
2011-05-13 13:29         ` Frederic Weisbecker

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=1305283706.22280.40.camel@frodo \
    --to=rostedt@goodmis.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --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 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.