Messages in this thread Patch in this message | | | Date | Sat, 25 Jan 2020 23:59:37 +0000 | From | Qais Yousef <> | Subject | Re: [PATCH] [RFC] sched: restrict iowait boost for boosted task only |
| |
On 01/24/20 12:55, Wei Wang wrote: > > > So I'm pretty sure we *do* want tasks with the default clamps to get iowait > > > boost'd. What we don't want are background tasks driving up the frequency, > > > and that should be via uclamp.max (as Quentin is suggesting) rather than > > > uclamp.min (as is suggested in the patch). > > > > > > Now, whether that is overloading the usage of uclamp... I'm not sure. > > > One of the argument for uclamp was actually frequency selection, so if > > > we just make iowait boost respect that, IOW not boost further than > > > uclamp.max (which is a bit better than a simple on/off switch), that > > > wouldn't be too crazy I think. > > > > Capping iowait boost value in schedutil based on uclamp makes sense indeed. > > > > What didn't make sense to me is the use of uclamp as a switch to toggle iowait > > boost on/off. > > Sounds like we all agree on adding a new toggle, so will move forward > with that then.
Looking more closely at iowait boost, it's not actually a generic cpufreq attribute. Only schedutil and intel_pstate have it. Other governors might implement something similar but under a different name.
So I'm not sure how easy it'd be to implement a generic toggle for something that probably should be considered an implementation detail of a governor and userspace shouldn't care much about.
Of course, the maintainers might have a different opinion. So don't let mine discourage you from pursuing this further! :-)
> For capping iowait boost, it should be a seperate patch. I am not sure > if we want to apply what's the current max clamp on the rq but I do > see the per-task iowait boost makes sense.
It is true the 2 patches are orthogonal, but if you already cap the max frequencies the background task can use, by ensuring the iowait_boost in schedutil respects the uclamp restrictions then this should solve your problem too, no?
The patch below only compile tested.
background/cpu.uclamp.max = 200 # Cap background tasks max frequencies
---
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c index 9b8916fd00a2..a76c02eecdaf 100644 --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -421,7 +421,8 @@ static unsigned long sugov_iowait_apply(struct sugov_cpu *sg_cpu, u64 time, * into the same scale so we can compare. */ boost = (sg_cpu->iowait_boost * max) >> SCHED_CAPACITY_SHIFT; - return max(boost, util); + boost = max(boost, util); + return uclamp_util_with(cpu_rq(sg_cpu->cpu), boost, NULL); }
#ifdef CONFIG_NO_HZ_COMMON -- Qais Yousef
| |