Messages in this thread Patch in this message | | | Date | Fri, 24 Jul 2020 11:12:44 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v7 3/3] sched/uclamp: Fix a deadlock when enabling uclamp static key |
| |
On Thu, Jul 16, 2020 at 12:03:47PM +0100, Qais Yousef wrote:
I've trimmed the Changelog to read like:
--- Subject: sched/uclamp: Fix a deadlock when enabling uclamp static key From: Qais Yousef <qais.yousef@arm.com> Date: Thu, 16 Jul 2020 12:03:47 +0100
From: Qais Yousef <qais.yousef@arm.com>
The following splat was caught when setting uclamp value of a task:
BUG: sleeping function called from invalid context at ./include/linux/percpu-rwsem.h:49
cpus_read_lock+0x68/0x130 static_key_enable+0x1c/0x38 __sched_setscheduler+0x900/0xad8
Fix by ensuring we enable the key outside of the critical section in __sched_setscheduler()
Fixes: 46609ce22703 ("sched/uclamp: Protect uclamp fast path code with static key") Signed-off-by: Qais Yousef <qais.yousef@arm.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20200716110347.19553-4-qais.yousef@arm.com ---
And placed this patch first in the series
That said; don't we have a slight problem with enabling the key late like this? It means the uclamp will not actually take effect immediately and we'll have to wait for the next context switch ... whenever that might be.
Should we not have enabled the key early, instead of late?
something like so perhaps?
--- diff --git a/kernel/sched/core.c b/kernel/sched/core.c index a2a244af9a53..c6499b2696f5 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -1282,8 +1282,6 @@ static void __setscheduler_uclamp(struct task_struct *p, if (likely(!(attr->sched_flags & SCHED_FLAG_UTIL_CLAMP))) return; - static_branch_enable(&sched_uclamp_used); - if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP_MIN) { uclamp_se_set(&p->uclamp_req[UCLAMP_MIN], attr->sched_util_min, true); @@ -5074,6 +5072,7 @@ static int __sched_setscheduler(struct task_struct *p, retval = uclamp_validate(p, attr); if (retval) return retval; + static_branch_enable(&sched_uclamp_used); } if (pi)
| |