The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Kirill Tkhai <ktkhai@parallels.com>
Cc: Sasha Levin <sasha.levin@oracle.com>,
	mingo@kernel.org, torvalds@linux-foundation.org,
	paulmck@linux.vnet.ibm.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sched: Use RCU read lock on all calls to dl_bw_of()
Date: Mon, 29 Sep 2014 19:00:22 +0200	[thread overview]
Message-ID: <20140929170022.GA18926@worktop.programming.kicks-ass.net> (raw)
In-Reply-To: <20140929165418.GQ5430@worktop>

On Mon, Sep 29, 2014 at 06:54:18PM +0200, Peter Zijlstra wrote:
> On Mon, Sep 29, 2014 at 08:43:47PM +0400, Kirill Tkhai wrote:
> > Thanks for your report. It looks like your fix is not enough, because
> > we check for rcu_read_lock_sched_held() in dl_bw_of(). It still warns
> > even if rcu_read_lock() is held.
> > 
> > I used rcu_read_lock_sched_held() because we free root_domain using
> > call_rcu_sched(). So, it's necessary to held rcu_read_lock_sched(),
> > and my initial commit has this problem too.
> > 
> > It looks like we should fix it in a way like this:
> > 
> > [PATCH]sched: Use dl_bw_of() under rcu_read_lock_sched()
> > 
> > rq->rd is freed using call_rcu_sched(), and it's accessed with preemption
> > disabled in the most cases.
> > 
> > So in other places we should use rcu_read_lock_sched() to access it to fit
> > the scheme:
> > 
> > rcu_read_lock_sched() or preempt_disable() <==> call_rcu_sched().
> 
> Hmm, sad that. I cannot remember why that is rcu_sched, I suspect
> because we rely on it someplace but I cannot remember where.
> 
> We could of course do a double take on that and use call_rcu after
> call_rcu_sched(), such that either or both are sufficient.
> 
> I would very much prefer not to add extra preempt_disable()s if
> possible.

Ah wait, if we simply move that preempt_disable() inside the
for_each_cpu() loop there's no harm done. Having them outside is painful
though.

  reply	other threads:[~2014-09-29 17:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-29 15:19 [PATCH] sched: Use RCU read lock on all calls to dl_bw_of() Sasha Levin
2014-09-29 16:43 ` Kirill Tkhai
2014-09-29 16:54   ` Peter Zijlstra
2014-09-29 17:00     ` Peter Zijlstra [this message]
2014-09-30  8:23       ` Kirill Tkhai
2014-10-02  9:24         ` Peter Zijlstra
2014-10-03  5:28         ` [tip:sched/core] sched/dl: Use dl_bw_of() under rcu_read_lock_sched() tip-bot for Kirill Tkhai

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=20140929170022.GA18926@worktop.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=ktkhai@parallels.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=sasha.levin@oracle.com \
    --cc=torvalds@linux-foundation.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox