| Message ID | 20240304201625.100619-3-christian.loehle@arm.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
| Series | Introduce per-task io utilization boost | expand |
On Mon, Mar 4, 2024 at 9:17 PM Christian Loehle <christian.loehle@arm.com> wrote: > > The previous commit provides a new cpu_util_cfs_boost_io interface for > schedutil which uses the io boosted utilization of the per-task > tracking strategy. Schedutil iowait boosting is therefore no longer > necessary so remove it. I'm wondering about the cases when schedutil is used without EAS. Are they still going to be handled as before after this change?
On 18/03/2024 14:07, Rafael J. Wysocki wrote: > On Mon, Mar 4, 2024 at 9:17 PM Christian Loehle > <christian.loehle@arm.com> wrote: >> >> The previous commit provides a new cpu_util_cfs_boost_io interface for >> schedutil which uses the io boosted utilization of the per-task >> tracking strategy. Schedutil iowait boosting is therefore no longer >> necessary so remove it. > > I'm wondering about the cases when schedutil is used without EAS. > > Are they still going to be handled as before after this change? Well they should still get boosted (under the new conditions) and according to my tests that does work. Anything in particular you're worried about? So in terms of throughput I see similar results with EAS and CAS+sugov. I'm happy including numbers in the cover letter for future versions, too. So far my intuition was that nobody would care enough to include them (as long as it generally still works). Kind Regards, Christian
On Mon, Mar 18, 2024 at 5:40 PM Christian Loehle <christian.loehle@arm.com> wrote: > > On 18/03/2024 14:07, Rafael J. Wysocki wrote: > > On Mon, Mar 4, 2024 at 9:17 PM Christian Loehle > > <christian.loehle@arm.com> wrote: > >> > >> The previous commit provides a new cpu_util_cfs_boost_io interface for > >> schedutil which uses the io boosted utilization of the per-task > >> tracking strategy. Schedutil iowait boosting is therefore no longer > >> necessary so remove it. > > > > I'm wondering about the cases when schedutil is used without EAS. > > > > Are they still going to be handled as before after this change? > > Well they should still get boosted (under the new conditions) and according > to my tests that does work. OK > Anything in particular you're worried about? It is not particularly clear to me how exactly the boost is taken into account without EAS. > So in terms of throughput I see similar results with EAS and CAS+sugov. > I'm happy including numbers in the cover letter for future versions, too. > So far my intuition was that nobody would care enough to include them > (as long as it generally still works). Well, IMV clear understanding of the changes is more important.
On 18/03/2024 17:08, Rafael J. Wysocki wrote: > On Mon, Mar 18, 2024 at 5:40 PM Christian Loehle > <christian.loehle@arm.com> wrote: >> >> On 18/03/2024 14:07, Rafael J. Wysocki wrote: >>> On Mon, Mar 4, 2024 at 9:17 PM Christian Loehle >>> <christian.loehle@arm.com> wrote: >>>> >>>> The previous commit provides a new cpu_util_cfs_boost_io interface for >>>> schedutil which uses the io boosted utilization of the per-task >>>> tracking strategy. Schedutil iowait boosting is therefore no longer >>>> necessary so remove it. >>> >>> I'm wondering about the cases when schedutil is used without EAS. >>> >>> Are they still going to be handled as before after this change? >> >> Well they should still get boosted (under the new conditions) and according >> to my tests that does work. > > OK > >> Anything in particular you're worried about? > > It is not particularly clear to me how exactly the boost is taken into > account without EAS. So a quick rundown for now, I'll try to include something along the lines in future versions then, too. Every task_struct carries an io_boost_level in the range of [0..8] with it. The boost is in units of utilization (w.r.t SCHED_CAPACITY_SCALE, independent of CPU the task might be currently enqueued on). The boost is taken into account for: 1. sugov frequency selection with io_boost = cpu_util_io_boost(sg_cpu->cpu); util = max(util, io_boost); The io boost of all tasks enqueued on the rq will be max-aggregated with the util here. (See cfs_rq->io_boost_tasks). 2. Task placement, for EAS in feec(); Otherwise select_idle_sibling() / select_idle_capacity() to ensure the CPU satisfies the requested io_boost of the task to be enqueued. Determining the io_boost_level is a bit more involved than with sugov's implementation and happens in dequeue_io_boost(), hopefully that part is reasonably understandable from the code. Hope that helps. Kind Regards, Christian > >> So in terms of throughput I see similar results with EAS and CAS+sugov. >> I'm happy including numbers in the cover letter for future versions, too. >> So far my intuition was that nobody would care enough to include them >> (as long as it generally still works). > > Well, IMV clear understanding of the changes is more important.
On 03/18/24 18:08, Rafael J. Wysocki wrote: > On Mon, Mar 18, 2024 at 5:40 PM Christian Loehle > <christian.loehle@arm.com> wrote: > > > > On 18/03/2024 14:07, Rafael J. Wysocki wrote: > > > On Mon, Mar 4, 2024 at 9:17 PM Christian Loehle > > > <christian.loehle@arm.com> wrote: > > >> > > >> The previous commit provides a new cpu_util_cfs_boost_io interface for > > >> schedutil which uses the io boosted utilization of the per-task > > >> tracking strategy. Schedutil iowait boosting is therefore no longer > > >> necessary so remove it. > > > > > > I'm wondering about the cases when schedutil is used without EAS. > > > > > > Are they still going to be handled as before after this change? > > > > Well they should still get boosted (under the new conditions) and according > > to my tests that does work. > > OK > > > Anything in particular you're worried about? > > It is not particularly clear to me how exactly the boost is taken into > account without EAS. > > > So in terms of throughput I see similar results with EAS and CAS+sugov. > > I'm happy including numbers in the cover letter for future versions, too. > > So far my intuition was that nobody would care enough to include them > > (as long as it generally still works). > > Well, IMV clear understanding of the changes is more important. I think the major thing we need to be careful about is the behavior when the task is sleeping. I think the boosting will be removed when the task is dequeued and I can bet there will be systems out there where the BLOCK softirq being boosted when the task is sleeping will matter. FWIW I do have an implementation for per-task iowait boost where I went a step further and converted intel_pstate too and like Christian didn't notice a regression. But I am not sure (rather don't think) I triggered this use case. I can't tell when the systems truly have per-cpu cpufreq control or just appear so and they are actually shared but not visible at linux level.
On 25/03/2024 02:37, Qais Yousef wrote: > On 03/18/24 18:08, Rafael J. Wysocki wrote: >> On Mon, Mar 18, 2024 at 5:40 PM Christian Loehle >> <christian.loehle@arm.com> wrote: >>> >>> On 18/03/2024 14:07, Rafael J. Wysocki wrote: >>>> On Mon, Mar 4, 2024 at 9:17 PM Christian Loehle >>>> <christian.loehle@arm.com> wrote: >>>>> >>>>> The previous commit provides a new cpu_util_cfs_boost_io interface for >>>>> schedutil which uses the io boosted utilization of the per-task >>>>> tracking strategy. Schedutil iowait boosting is therefore no longer >>>>> necessary so remove it. >>>> >>>> I'm wondering about the cases when schedutil is used without EAS. >>>> >>>> Are they still going to be handled as before after this change? >>> >>> Well they should still get boosted (under the new conditions) and according >>> to my tests that does work. >> >> OK >> >>> Anything in particular you're worried about? >> >> It is not particularly clear to me how exactly the boost is taken into >> account without EAS. >> >>> So in terms of throughput I see similar results with EAS and CAS+sugov. >>> I'm happy including numbers in the cover letter for future versions, too. >>> So far my intuition was that nobody would care enough to include them >>> (as long as it generally still works). >> >> Well, IMV clear understanding of the changes is more important. > > I think the major thing we need to be careful about is the behavior when the > task is sleeping. I think the boosting will be removed when the task is > dequeued and I can bet there will be systems out there where the BLOCK softirq > being boosted when the task is sleeping will matter. Currently I see this mainly protected by the sugov rate_limit_us. With the enqueue's being the dominating cpufreq updates it's not really an issue, the boost is expected to survive the sleep duration, during which it wouldn't be active. I did experiment with some sort of 'stickiness' of the boost to the rq, but it is somewhat of a pain to deal with if we want to remove it once enqueued on a different rq. A sugov 1ms timer is much simpler of course. Currently it's not necessary IMO, but for the sake of being future-proof in terms of more frequent freq updates I might include it in v2. > > FWIW I do have an implementation for per-task iowait boost where I went a step > further and converted intel_pstate too and like Christian didn't notice > a regression. But I am not sure (rather don't think) I triggered this use case. > I can't tell when the systems truly have per-cpu cpufreq control or just appear > so and they are actually shared but not visible at linux level. Please do share your intel_pstate proposal! Kind Regards, Christian
On 04/19/24 14:42, Christian Loehle wrote: > > I think the major thing we need to be careful about is the behavior when the > > task is sleeping. I think the boosting will be removed when the task is > > dequeued and I can bet there will be systems out there where the BLOCK softirq > > being boosted when the task is sleeping will matter. > > Currently I see this mainly protected by the sugov rate_limit_us. > With the enqueue's being the dominating cpufreq updates it's not really an > issue, the boost is expected to survive the sleep duration, during which it > wouldn't be active. > I did experiment with some sort of 'stickiness' of the boost to the rq, but > it is somewhat of a pain to deal with if we want to remove it once enqueued > on a different rq. A sugov 1ms timer is much simpler of course. > Currently it's not necessary IMO, but for the sake of being future-proof in > terms of more frequent freq updates I might include it in v2. Making sure things work with purpose would be really great. This implicit dependency is not great IMHO and make both testing and reasoning about why things are good or bad harder when analysing real workloads. Especially by non kernel developers. > > > > > FWIW I do have an implementation for per-task iowait boost where I went a step > > further and converted intel_pstate too and like Christian didn't notice > > a regression. But I am not sure (rather don't think) I triggered this use case. > > I can't tell when the systems truly have per-cpu cpufreq control or just appear > > so and they are actually shared but not visible at linux level. > > Please do share your intel_pstate proposal! This is what I had. I haven't been working on this for the past few months, but I remember tried several tests on different machines then without a problem. I tried to re-order patches at some point though and I hope I didn't break something accidentally and forgot the state. https://github.com/torvalds/linux/compare/master...qais-yousef:linux:uclamp-max-aggregation
On 29/04/2024 12:18, Qais Yousef wrote: > On 04/19/24 14:42, Christian Loehle wrote: > >>> I think the major thing we need to be careful about is the behavior when the >>> task is sleeping. I think the boosting will be removed when the task is >>> dequeued and I can bet there will be systems out there where the BLOCK softirq >>> being boosted when the task is sleeping will matter. >> >> Currently I see this mainly protected by the sugov rate_limit_us. >> With the enqueue's being the dominating cpufreq updates it's not really an >> issue, the boost is expected to survive the sleep duration, during which it >> wouldn't be active. >> I did experiment with some sort of 'stickiness' of the boost to the rq, but >> it is somewhat of a pain to deal with if we want to remove it once enqueued >> on a different rq. A sugov 1ms timer is much simpler of course. >> Currently it's not necessary IMO, but for the sake of being future-proof in >> terms of more frequent freq updates I might include it in v2. > > Making sure things work with purpose would be really great. This implicit > dependency is not great IMHO and make both testing and reasoning about why > things are good or bad harder when analysing real workloads. Especially by non > kernel developers. Agreed. Even without your proposed changes [1] relying on sugov rate_limit_us is unfortunate. There is a problem with an arbitrarily low rate_limit_us more generally, not just because we kind of rely on the CPU being boosted right before the task is actually enqueued (for the interrupt/softirq part of it), but also because of the latency from requested frequency improvement to actually running on that frequency. If the task is 90% done by the time it sees the improvement and the frequency will be updated (back to a lower one) before the next enqueue, then that's hardly worth the effort. Currently this is covered by rate_limit_us probabillistically and that seems to be good enough in practice, but it's not very pleasing (and also EAS can't take it into consideration). That's not just exclusive for iowait wakeup tasks of course, but in theory any that is off the rq frequently (and still requests a higher frequency than it can realistically build up through util_avg like through uclamp_min). >>> >>> FWIW I do have an implementation for per-task iowait boost where I went a step >>> further and converted intel_pstate too and like Christian didn't notice >>> a regression. But I am not sure (rather don't think) I triggered this use case. >>> I can't tell when the systems truly have per-cpu cpufreq control or just appear >>> so and they are actually shared but not visible at linux level. >> >> Please do share your intel_pstate proposal! > > This is what I had. I haven't been working on this for the past few months, but > I remember tried several tests on different machines then without a problem. > I tried to re-order patches at some point though and I hope I didn't break > something accidentally and forgot the state. > > https://github.com/torvalds/linux/compare/master...qais-yousef:linux:uclamp-max-aggregation > Thanks for sharing, that looks reasonable with consolidating it into uclamp_min. Couple of thoughts on yours, I'm sure you're aware, but consider it me thinking out loud: - iowait boost is taken into consideration for task placement, but with just the 4 steps that made it more aggressive on HMP. (Potentially 2-3 consecutive iowait wakeups to land on the big instead of running at max OPP of a LITTLE). - If the current iowait boost decay is sensible is questionable, but there should probably be some decay. Taken to the extreme this would mean something like blk_wait_io() demands 1024 utilization, if it waits for a very long time. Repeating myself here, but iowait wakeups itself is tricky to work with (and I try to work around that). - The intel_pstate solution will increase boost even if previous_wakeup->iowait_boost > current->iowait_boost right? But using current->iowait_boost is a clever idea. [1] https://lore.kernel.org/lkml/ZgKFT5b423hfQdl9@gmail.com/T/ Kind Regards, Christian
On 05/07/24 16:19, Christian Loehle wrote: > On 29/04/2024 12:18, Qais Yousef wrote: > > On 04/19/24 14:42, Christian Loehle wrote: > > > >>> I think the major thing we need to be careful about is the behavior when the > >>> task is sleeping. I think the boosting will be removed when the task is > >>> dequeued and I can bet there will be systems out there where the BLOCK softirq > >>> being boosted when the task is sleeping will matter. > >> > >> Currently I see this mainly protected by the sugov rate_limit_us. > >> With the enqueue's being the dominating cpufreq updates it's not really an > >> issue, the boost is expected to survive the sleep duration, during which it > >> wouldn't be active. > >> I did experiment with some sort of 'stickiness' of the boost to the rq, but > >> it is somewhat of a pain to deal with if we want to remove it once enqueued > >> on a different rq. A sugov 1ms timer is much simpler of course. > >> Currently it's not necessary IMO, but for the sake of being future-proof in > >> terms of more frequent freq updates I might include it in v2. > > > > Making sure things work with purpose would be really great. This implicit > > dependency is not great IMHO and make both testing and reasoning about why > > things are good or bad harder when analysing real workloads. Especially by non > > kernel developers. > > Agreed. > Even without your proposed changes [1] relying on sugov rate_limit_us is > unfortunate. > There is a problem with an arbitrarily low rate_limit_us more generally, not > just because we kind of rely on the CPU being boosted right before the task is > actually enqueued (for the interrupt/softirq part of it), but also because of > the latency from requested frequency improvement to actually running on that > frequency. If the task is 90% done by the time it sees the improvement and > the frequency will be updated (back to a lower one) before the next enqueue, > then that's hardly worth the effort. I think that's why iowait boost is done the way it is today. You need to sustain the boost as the thing that need it run for a very short amount of time.. Have you looked at how long the iowait boosted tasks run for in your tests? > Currently this is covered by rate_limit_us probabillistically and that seems Side note. While looking more at the history of rate_limit_us and why it is set soo much higher than reported transition_latency by the driver, I am slowly reaching the conclusion that what is happening here is similar to introduction of down_rate_limit_us in Android. Our ability to predict system requirement is not great and we end up with prematurely lowering frequencies, and this rate limit just happens to more likely keep it higher so on average you see better perf in various workloads. I hope we can improve on this so we don't rely on this magic and enable better usage of the hardware ability to transition between frequencies fast and be more exact. I am trying to work on this as part of my magic margins series. But I think the story is bigger than that.. > to be good enough in practice, but it's not very pleasing (and also EAS can't > take it into consideration). > That's not just exclusive for iowait wakeup tasks of course, but in theory any > that is off the rq frequently (and still requests a higher frequency than it can > realistically build up through util_avg like through uclamp_min). For uclamp_min, if the task RUNNING duration is shorter than the hardware ability to apply the boost, I think there's little we can do. The user can opt to boost the system in general. Note that this is likely a problem on systems with multi-ms transition_delay_us. If the task is running for few 100s us and it really wants to be boosted for this short time then a static system wide boost is all we can do to guarantee what it wants. The hardware is poor fit for the use case in this scenario. And personally I'd push back against introducing complexity to deal with such poor fit scenarios. We can already set min freq for the policy via sysfs and uclamp_min can still help with task placement for HMP systems. Now the problem we could have which is similar to iowait boost scenario is when there's a chaining effect that requires the boost for the duration of the chain. I think we can do something about it if we: 1. Guarantee the chain will run on the same CPU. 2. Introduce a 'sticky' flag for the boost to stay while the chain is running. 3. Introduce a start/finish indication for the chain. I think we can do something like that with sched-qos [1] to tag the chain via a cookie and request the boost to apply to them collectively. Generally, userspace would be better to collapse this chain into a single task that runs in one go. I don't know how often this scenario exists in practice and what limitations exist that make the simple collapse solution infeasible.. So I'd leave this out until more info is available. > > >>> > >>> FWIW I do have an implementation for per-task iowait boost where I went a step > >>> further and converted intel_pstate too and like Christian didn't notice > >>> a regression. But I am not sure (rather don't think) I triggered this use case. > >>> I can't tell when the systems truly have per-cpu cpufreq control or just appear > >>> so and they are actually shared but not visible at linux level. > >> > >> Please do share your intel_pstate proposal! > > > > This is what I had. I haven't been working on this for the past few months, but > > I remember tried several tests on different machines then without a problem. > > I tried to re-order patches at some point though and I hope I didn't break > > something accidentally and forgot the state. > > > > https://github.com/torvalds/linux/compare/master...qais-yousef:linux:uclamp-max-aggregation > > > > Thanks for sharing, that looks reasonable with consolidating it into uclamp_min. > Couple of thoughts on yours, I'm sure you're aware, but consider it me thinking out > loud: > - iowait boost is taken into consideration for task placement, but with just the > 4 steps that made it more aggressive on HMP. (Potentially 2-3 consecutive iowait > wakeups to land on the big instead of running at max OPP of a LITTLE). Yeah I opted to keep the logic the same. I think there are gains to be had even without being smarter about the algorithm. But we do need to improve it, yes. The current logic is too aggressive and the perf/power trade-off will be tricky in practice. > - If the current iowait boost decay is sensible is questionable, but there should > probably be some decay. Taken to the extreme this would mean something > like blk_wait_io() demands 1024 utilization, if it waits for a very long time. > Repeating myself here, but iowait wakeups itself is tricky to work with (and I > try to work around that). I didn't get you here. But generally the story can go few levels deep yes. My approach was to make incremental progress without breaking existing stuff, but help move things in the right direction over time. Fixing everything in one go will be hard and not productive. > - The intel_pstate solution will increase boost even if > previous_wakeup->iowait_boost > current->iowait_boost > right? But using current->iowait_boost is a clever idea. I forgot the details now. But I seem to remember intel_pstate had its own accounting when it sees iowait boost in intel_pstate_hwp_boost_up/down(). Note that the major worry I had is about the softirq not being boosted. Although in my testing this didn't seem to show up as things seemed fine. But I haven't dug down to see how accidental this was. I could easily see my patches making some use case out there not happy as the softirq might not get a chance to see the boost. I got distracted with other stuff and didn't get back to the topic since then. I am more than happy to support your efforts though :) [1] https://lore.kernel.org/lkml/20230916213316.p36nhgnibsidoggt@airbuntu/
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c index cd0ca3cbd212..ed9fc88a74fc 100644 --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -6,8 +6,6 @@ * Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com> */ -#define IOWAIT_BOOST_MIN (SCHED_CAPACITY_SCALE / 8) - struct sugov_tunables { struct gov_attr_set attr_set; unsigned int rate_limit_us; @@ -42,10 +40,6 @@ struct sugov_cpu { struct sugov_policy *sg_policy; unsigned int cpu; - bool iowait_boost_pending; - unsigned int iowait_boost; - u64 last_update; - unsigned long util; unsigned long bw_min; @@ -195,141 +189,17 @@ unsigned long sugov_effective_cpu_perf(int cpu, unsigned long actual, return max(min, max); } -static void sugov_get_util(struct sugov_cpu *sg_cpu, unsigned long boost) +static void sugov_get_util(struct sugov_cpu *sg_cpu) { unsigned long min, max, util = cpu_util_cfs_boost(sg_cpu->cpu); unsigned long io_boost = cpu_util_io_boost(sg_cpu->cpu); - /* - * XXX: This already includes io boost now, makes little sense with - * sugov iowait boost on top - */ util = max(util, io_boost); util = effective_cpu_util(sg_cpu->cpu, util, &min, &max); - util = max(util, boost); sg_cpu->bw_min = min; sg_cpu->util = sugov_effective_cpu_perf(sg_cpu->cpu, util, min, max); } -/** - * sugov_iowait_reset() - Reset the IO boost status of a CPU. - * @sg_cpu: the sugov data for the CPU to boost - * @time: the update time from the caller - * @set_iowait_boost: true if an IO boost has been requested - * - * The IO wait boost of a task is disabled after a tick since the last update - * of a CPU. If a new IO wait boost is requested after more then a tick, then - * we enable the boost starting from IOWAIT_BOOST_MIN, which improves energy - * efficiency by ignoring sporadic wakeups from IO. - */ -static bool sugov_iowait_reset(struct sugov_cpu *sg_cpu, u64 time, - bool set_iowait_boost) -{ - s64 delta_ns = time - sg_cpu->last_update; - - /* Reset boost only if a tick has elapsed since last request */ - if (delta_ns <= TICK_NSEC) - return false; - - sg_cpu->iowait_boost = set_iowait_boost ? IOWAIT_BOOST_MIN : 0; - sg_cpu->iowait_boost_pending = set_iowait_boost; - - return true; -} - -/** - * sugov_iowait_boost() - Updates the IO boost status of a CPU. - * @sg_cpu: the sugov data for the CPU to boost - * @time: the update time from the caller - * @flags: SCHED_CPUFREQ_IOWAIT if the task is waking up after an IO wait - * - * Each time a task wakes up after an IO operation, the CPU utilization can be - * boosted to a certain utilization which doubles at each "frequent and - * successive" wakeup from IO, ranging from IOWAIT_BOOST_MIN to the utilization - * of the maximum OPP. - * - * To keep doubling, an IO boost has to be requested at least once per tick, - * otherwise we restart from the utilization of the minimum OPP. - */ -static void sugov_iowait_boost(struct sugov_cpu *sg_cpu, u64 time, - unsigned int flags) -{ - bool set_iowait_boost = flags & SCHED_CPUFREQ_IOWAIT; - - /* Reset boost if the CPU appears to have been idle enough */ - if (sg_cpu->iowait_boost && - sugov_iowait_reset(sg_cpu, time, set_iowait_boost)) - return; - - /* Boost only tasks waking up after IO */ - if (!set_iowait_boost) - return; - - /* Ensure boost doubles only one time at each request */ - if (sg_cpu->iowait_boost_pending) - return; - sg_cpu->iowait_boost_pending = true; - - /* Double the boost at each request */ - if (sg_cpu->iowait_boost) { - sg_cpu->iowait_boost = - min_t(unsigned int, sg_cpu->iowait_boost << 1, SCHED_CAPACITY_SCALE); - return; - } - - /* First wakeup after IO: start with minimum boost */ - sg_cpu->iowait_boost = IOWAIT_BOOST_MIN; -} - -/** - * sugov_iowait_apply() - Apply the IO boost to a CPU. - * @sg_cpu: the sugov data for the cpu to boost - * @time: the update time from the caller - * @max_cap: the max CPU capacity - * - * A CPU running a task which woken up after an IO operation can have its - * utilization boosted to speed up the completion of those IO operations. - * The IO boost value is increased each time a task wakes up from IO, in - * sugov_iowait_apply(), and it's instead decreased by this function, - * each time an increase has not been requested (!iowait_boost_pending). - * - * A CPU which also appears to have been idle for at least one tick has also - * its IO boost utilization reset. - * - * This mechanism is designed to boost high frequently IO waiting tasks, while - * being more conservative on tasks which does sporadic IO operations. - */ -static unsigned long sugov_iowait_apply(struct sugov_cpu *sg_cpu, u64 time, - unsigned long max_cap) -{ - /* No boost currently required */ - if (!sg_cpu->iowait_boost) - return 0; - - /* Reset boost if the CPU appears to have been idle enough */ - if (sugov_iowait_reset(sg_cpu, time, false)) - return 0; - - if (!sg_cpu->iowait_boost_pending) { - /* - * No boost pending; reduce the boost value. - */ - sg_cpu->iowait_boost >>= 1; - if (sg_cpu->iowait_boost < IOWAIT_BOOST_MIN) { - sg_cpu->iowait_boost = 0; - return 0; - } - } - - sg_cpu->iowait_boost_pending = false; - - /* - * sg_cpu->util is already in capacity scale; convert iowait_boost - * into the same scale so we can compare. - */ - return (sg_cpu->iowait_boost * max_cap) >> SCHED_CAPACITY_SHIFT; -} - #ifdef CONFIG_NO_HZ_COMMON static bool sugov_cpu_is_busy(struct sugov_cpu *sg_cpu) { @@ -357,18 +227,12 @@ static inline bool sugov_update_single_common(struct sugov_cpu *sg_cpu, u64 time, unsigned long max_cap, unsigned int flags) { - unsigned long boost; - - sugov_iowait_boost(sg_cpu, time, flags); - sg_cpu->last_update = time; - ignore_dl_rate_limit(sg_cpu); if (!sugov_should_update_freq(sg_cpu->sg_policy, time)) return false; - boost = sugov_iowait_apply(sg_cpu, time, max_cap); - sugov_get_util(sg_cpu, boost); + sugov_get_util(sg_cpu); return true; } @@ -458,7 +322,7 @@ static void sugov_update_single_perf(struct update_util_data *hook, u64 time, sg_cpu->sg_policy->last_freq_update_time = time; } -static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time) +static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu) { struct sugov_policy *sg_policy = sg_cpu->sg_policy; struct cpufreq_policy *policy = sg_policy->policy; @@ -469,11 +333,8 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time) for_each_cpu(j, policy->cpus) { struct sugov_cpu *j_sg_cpu = &per_cpu(sugov_cpu, j); - unsigned long boost; - - boost = sugov_iowait_apply(j_sg_cpu, time, max_cap); - sugov_get_util(j_sg_cpu, boost); + sugov_get_util(j_sg_cpu); util = max(j_sg_cpu->util, util); } @@ -489,13 +350,10 @@ sugov_update_shared(struct update_util_data *hook, u64 time, unsigned int flags) raw_spin_lock(&sg_policy->update_lock); - sugov_iowait_boost(sg_cpu, time, flags); - sg_cpu->last_update = time; - ignore_dl_rate_limit(sg_cpu); if (sugov_should_update_freq(sg_policy, time)) { - next_f = sugov_next_freq_shared(sg_cpu, time); + next_f = sugov_next_freq_shared(sg_cpu); if (!sugov_update_next_freq(sg_policy, time, next_f)) goto unlock;
The previous commit provides a new cpu_util_cfs_boost_io interface for schedutil which uses the io boosted utilization of the per-task tracking strategy. Schedutil iowait boosting is therefore no longer necessary so remove it. Signed-off-by: Christian Loehle <christian.loehle@arm.com> --- kernel/sched/cpufreq_schedutil.c | 152 +------------------------------ 1 file changed, 5 insertions(+), 147 deletions(-)