linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@techsingularity.net>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org,
	Matt Fleming <matt@codeblueprint.co.uk>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Subject: Re: [PATCH 2/2] sched/debug: fix deadlock when enabling sched events
Date: Wed, 8 Jun 2016 09:11:41 +0100	[thread overview]
Message-ID: <20160608081141.GF2527@techsingularity.net> (raw)
In-Reply-To: <50a0f7f2a5f8d69e0440c3430f06be7ccbe59903.1465322027.git.jpoimboe@redhat.com>

On Tue, Jun 07, 2016 at 02:43:17PM -0500, Josh Poimboeuf wrote:
> When enabling sched trace events via:
> 
>   echo 1 > /sys/kernel/debug/tracing/events/sched/enable
> 
> I see a hang, with the following BUG in the printk buffer:
> 

I was certain I had tested this case because I was monitoring to make
sure the stats were not updating until the tuning knob was used. It was
because the context was unsafe that any printing happened because at the
time trying to enable stats in that context blew up.

> In addition to the deadlock, I think this code has other issues:
> 
> 1. Instead of just warning and allowing the tracepoints to be broken,
>    I'd argue that it would be better to make them work by forcing
>    schedstats enabled and printing a warning about that, which is what's
>    already being done in other similar cases (latencytop and profiling).
>    Otherwise things like "perf sched record" won't have the intended
>    effect.  In fact the comment in the above code snippet seems to agree
>    with me, so maybe that was the original intent.
> 
> 2. It's called in the scheduler hot path from enqueue_entity().
> 
> So change the warning to a call to force_schedstat_enabled(), and do it
> only when the tracepoint gets enabled.
> 
> Fixes: cb2517653fcc ("sched/debug: Make schedstats a runtime tunable that is disabled by default")
> Cc: stable@vger.kernel.org
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

I like it! I wasn't aware there was an option to have a registration
callback.

Acked-by: Mel Gorman <mgorman@techsingularity.net>

-- 
Mel Gorman
SUSE Labs

      parent reply	other threads:[~2016-06-08  8:12 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-07 19:43 [PATCH 0/2] sched/debug: more schedstats fixes Josh Poimboeuf
2016-06-07 19:43 ` [PATCH 1/2] sched/debug: fix 'schedstats=enable' cmdline option Josh Poimboeuf
2016-06-08  8:06   ` Mel Gorman
2016-06-08 14:21   ` [tip:sched/core] sched/debug: Fix " tip-bot for Josh Poimboeuf
2016-06-07 19:43 ` [PATCH 2/2] sched/debug: fix deadlock when enabling sched events Josh Poimboeuf
2016-06-07 19:54   ` Peter Zijlstra
2016-06-07 21:02     ` Josh Poimboeuf
2016-06-08  8:13     ` Mel Gorman
2016-06-07 21:06   ` Josh Poimboeuf
2016-06-08  7:56   ` Ingo Molnar
2016-06-08 13:12     ` Josh Poimboeuf
2016-06-08 13:15       ` Ingo Molnar
2016-06-13  7:32         ` [PATCH v2] " Josh Poimboeuf
2016-06-14 11:35           ` [tip:sched/urgent] sched/debug: Fix " tip-bot for Josh Poimboeuf
2016-06-08  8:11   ` Mel Gorman [this message]

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=20160608081141.GF2527@techsingularity.net \
    --to=mgorman@techsingularity.net \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt@codeblueprint.co.uk \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=srikar@linux.vnet.ibm.com \
    --subject='Re: [PATCH 2/2] sched/debug: fix deadlock when enabling sched events' \
    /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

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).