All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qais Yousef <qais.yousef@arm.com>
To: "Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Patrick Bellasi <patrick.bellasi@matbug.net>,
	Tejun Heo <tj@kernel.org>, Quentin Perret <qperret@google.com>,
	Wei Wang <wvw@google.com>, Yun Hsiang <hsiang023167@gmail.com>,
	linux-kernel@vger.kernel.org, Qais Yousef <qais.yousef@arm.com>
Subject: [PATCH RESEND 2/2] sched/uclamp: Fix locking around cpu_util_update_eff()
Date: Mon, 10 May 2021 15:50:32 +0100	[thread overview]
Message-ID: <20210510145032.1934078-3-qais.yousef@arm.com> (raw)
In-Reply-To: <20210510145032.1934078-1-qais.yousef@arm.com>

cpu_cgroup_css_online() calls cpu_util_update_eff() without holding the
uclamp_mutex or rcu_read_lock() like other call sites, which is
a mistake.

The uclamp_mutex is required to protect against concurrent reads and
writes that could update the cgroup hierarchy.

The rcu_read_lock() is required to traverse the cgroup data structures
in cpu_util_update_eff().

Surround the caller with the required locks and add some asserts to
better document the dependency in cpu_util_update_eff().

Fixes: 7226017ad37a ("sched/uclamp: Fix a bug in propagating uclamp value in new cgroups")
Reported-by: Quentin Perret <qperret@google.com>
Signed-off-by: Qais Yousef <qais.yousef@arm.com>
---

There was no real failure observed because of this. Quentin just observed the
oddity and reported it.

 kernel/sched/core.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index b61329299379..efa15287d09e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8690,7 +8690,11 @@ static int cpu_cgroup_css_online(struct cgroup_subsys_state *css)
 
 #ifdef CONFIG_UCLAMP_TASK_GROUP
 	/* Propagate the effective uclamp value for the new group */
+	mutex_lock(&uclamp_mutex);
+	rcu_read_lock();
 	cpu_util_update_eff(css);
+	rcu_read_unlock();
+	mutex_unlock(&uclamp_mutex);
 #endif
 
 	return 0;
@@ -8780,6 +8784,9 @@ static void cpu_util_update_eff(struct cgroup_subsys_state *css)
 	enum uclamp_id clamp_id;
 	unsigned int clamps;
 
+	lockdep_assert_held(&uclamp_mutex);
+	SCHED_WARN_ON(!rcu_read_lock_held());
+
 	css_for_each_descendant_pre(css, top_css) {
 		uc_parent = css_tg(css)->parent
 			? css_tg(css)->parent->uclamp : NULL;
-- 
2.25.1


  parent reply	other threads:[~2021-05-10 14:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-10 14:50 [PATCH RESEND 0/2] Uclamp cgroup fixes Qais Yousef
2021-05-10 14:50 ` [PATCH RESEND 1/2] sched/uclamp: Fix wrong implementation of cpu.uclamp.min Qais Yousef
2021-05-19  8:09   ` [tip: sched/core] " tip-bot2 for Qais Yousef
2021-05-19  9:02   ` tip-bot2 for Qais Yousef
2021-05-10 14:50 ` Qais Yousef [this message]
2021-05-19  8:09   ` [tip: sched/core] sched/uclamp: Fix locking around cpu_util_update_eff() tip-bot2 for Qais Yousef
2021-05-19  9:02   ` tip-bot2 for Qais Yousef
2021-05-13 14:13 ` [PATCH RESEND 0/2] Uclamp cgroup fixes Peter Zijlstra

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=20210510145032.1934078-3-qais.yousef@arm.com \
    --to=qais.yousef@arm.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=hsiang023167@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=patrick.bellasi@matbug.net \
    --cc=peterz@infradead.org \
    --cc=qperret@google.com \
    --cc=tj@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=wvw@google.com \
    /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.