All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Qais Yousef <qais.yousef@arm.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	Valentin Schneider <valentin.schneider@arm.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Patrick Bellasi <patrick.bellasi@matbug.net>,
	Chris Redpath <chris.redpath@arm.com>,
	Lukasz Luba <lukasz.luba@arm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 2/2] sched/uclamp: Protect uclamp fast path code with static key
Date: Tue, 30 Jun 2020 21:06:43 +0200	[thread overview]
Message-ID: <20200630190643.GC4817@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20200630175502.otw4seymlynghje7@e107158-lin.cambridge.arm.com>

On Tue, Jun 30, 2020 at 06:55:02PM +0100, Qais Yousef wrote:
> On 06/30/20 19:07, Peter Zijlstra wrote:

> > There's a fun race described in 9107c89e269d ("perf: Fix race between
> > event install and jump_labels"), are we sure this isn't also susceptible
> > to something similar?
> > 
> > I suspect not, but I just wanted to make sure.
> 
> IIUC, the worry is that not all CPUs might have observed the change in the
> static key state; hence could not be running the patched
> enqueue/dequeue_task(), so we could end up with some CPUs accounting for
> uclamp in the enqueue/dequeue path but not others?
> 
> I was hoping this synchronization is guaranteed by the static_branch_*() call.

It is, that isn't quite the the problem. Looking at it more I think
commit 1dbb6704de91 ("jump_label: Fix concurrent
static_key_enable/disable()") fixed some of it.

From what I can remember there were two parts to this problem, one being
fixed by the above commit, the other being that if we enable while a
task is running we miss the switch-in event (exactly how in this patch
we miss the enqueue).

Due to the missing switch-in, the state is 'weird' and the subsequent
IPI to install a remote event didn't quite work.

So I put that sync_sched() call in to guarantee all CPUs have done a
schedule() cycle after having the key switched. This makes sure that
every running task has seen the switch-in and thus the state is as
expected.

But like I said, I think we're good, that one extra branch deals with
the half-state.

  reply	other threads:[~2020-06-30 19:07 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-30 11:21 [PATCH v6 0/2] sched: Optionally skip uclamp logic in fast path Qais Yousef
2020-06-30 11:21 ` [PATCH v6 1/2] sched/uclamp: Fix initialization of struct uclamp_rq Qais Yousef
2020-07-09  8:45   ` [tip: sched/core] " tip-bot2 for Qais Yousef
2020-06-30 11:21 ` [PATCH v6 2/2] sched/uclamp: Protect uclamp fast path code with static key Qais Yousef
2020-06-30 17:07   ` Peter Zijlstra
2020-06-30 17:55     ` Qais Yousef
2020-06-30 19:06       ` Peter Zijlstra [this message]
2020-06-30 19:28         ` Qais Yousef
2020-07-09  8:45   ` [tip: sched/core] " tip-bot2 for Qais Yousef
2020-07-01 16:32 ` [PATCH v6 0/2] sched: Optionally skip uclamp logic in fast path Lukasz Luba
2020-07-03 12:09 ` Vincent Guittot
2020-07-06 10:41   ` Qais Yousef
2020-07-07 12:29     ` Vincent Guittot
2020-07-07 13:11       ` Qais Yousef

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=20200630190643.GC4817@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=bsegall@google.com \
    --cc=chris.redpath@arm.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukasz.luba@arm.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=patrick.bellasi@matbug.net \
    --cc=qais.yousef@arm.com \
    --cc=rostedt@goodmis.org \
    --cc=valentin.schneider@arm.com \
    --cc=vincent.guittot@linaro.org \
    /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.