* Re: [PATCH v4 5/8] sched/fair: Take into account latency priority at wakeup [not found] <20220916080305.29574-6-vincent.guittot@linaro.org> @ 2022-09-16 12:02 ` Hillf Danton 2022-09-16 13:36 ` Vincent Guittot 0 siblings, 1 reply; 6+ messages in thread From: Hillf Danton @ 2022-09-16 12:02 UTC (permalink / raw) To: Vincent Guittot Cc: peterz, mgorman, valentin.schneider, parth, linux-mm, linux-kernel Hello Vincent On 16 Sep 2022 10:03:02 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote: > > @@ -4606,6 +4608,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > se = __pick_first_entity(cfs_rq); > delta = curr->vruntime - se->vruntime; > + delta -= wakeup_latency_gran(curr, se); > > if (delta < 0) > return; What is derived from the latency nice you added is the runtime granulaity which has a role in preempting the current task. Given the same defination of latency nice as the nice, the runtime granularity can be computed without introducing the latency nice. Only for thoughts now. Hillf +++ b/kernel/sched/fair.c @@ -4569,7 +4569,7 @@ dequeue_entity(struct cfs_rq *cfs_rq, st static void check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) { - unsigned long ideal_runtime, delta_exec; + unsigned long ideal_runtime, delta_exec, granu; struct sched_entity *se; s64 delta; @@ -4594,6 +4594,14 @@ check_preempt_tick(struct cfs_rq *cfs_rq return; se = __pick_first_entity(cfs_rq); + + granu = sysctl_sched_min_granularity + + (ideal_runtime - sysctl_sched_min_granularity) * + (se->latency_nice + 20) / LATENCY_NICE_WIDTH; + + if (delta_exec < granu) + return; + delta = curr->vruntime - se->vruntime; if (delta < 0) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 5/8] sched/fair: Take into account latency priority at wakeup 2022-09-16 12:02 ` [PATCH v4 5/8] sched/fair: Take into account latency priority at wakeup Hillf Danton @ 2022-09-16 13:36 ` Vincent Guittot 2022-09-17 22:58 ` Hillf Danton 0 siblings, 1 reply; 6+ messages in thread From: Vincent Guittot @ 2022-09-16 13:36 UTC (permalink / raw) To: Hillf Danton Cc: peterz, mgorman, valentin.schneider, parth, linux-mm, linux-kernel Hi Hillf, On Fri, 16 Sept 2022 at 14:03, Hillf Danton <hdanton@sina.com> wrote: > > Hello Vincent > > On 16 Sep 2022 10:03:02 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote: > > > > @@ -4606,6 +4608,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > > > se = __pick_first_entity(cfs_rq); > > delta = curr->vruntime - se->vruntime; > > + delta -= wakeup_latency_gran(curr, se); > > > > if (delta < 0) > > return; > > What is derived from the latency nice you added is the runtime granulaity > which has a role in preempting the current task. > > Given the same defination of latency nice as the nice, the runtime granularity > can be computed without introducing the latency nice. > > Only for thoughts now. > > Hillf > > +++ b/kernel/sched/fair.c > @@ -4569,7 +4569,7 @@ dequeue_entity(struct cfs_rq *cfs_rq, st > static void > check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) > { > - unsigned long ideal_runtime, delta_exec; > + unsigned long ideal_runtime, delta_exec, granu; > struct sched_entity *se; > s64 delta; > > @@ -4594,6 +4594,14 @@ check_preempt_tick(struct cfs_rq *cfs_rq > return; > > se = __pick_first_entity(cfs_rq); > + > + granu = sysctl_sched_min_granularity + > + (ideal_runtime - sysctl_sched_min_granularity) * > + (se->latency_nice + 20) / LATENCY_NICE_WIDTH; There is no latency_nice field in se but a latency_offset instead Also at this step, we are sure that curr has run at least sysctl_sched_min_granularity and we want now to compare curr vruntime with first se's one. We take the latency offset into account to make sure we will not preempt curr too early > + > + if (delta_exec < granu) > + return; > + > delta = curr->vruntime - se->vruntime; > > if (delta < 0) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 5/8] sched/fair: Take into account latency priority at wakeup 2022-09-16 13:36 ` Vincent Guittot @ 2022-09-17 22:58 ` Hillf Danton 2022-09-18 10:46 ` Vincent Guittot 0 siblings, 1 reply; 6+ messages in thread From: Hillf Danton @ 2022-09-17 22:58 UTC (permalink / raw) To: Vincent Guittot Cc: peterz, mgorman, valentin.schneider, parth, linux-mm, linux-kernel On 16 Sep 2022 15:36:53 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote: > > Hi Hillf, > > On Fri, 16 Sept 2022 at 14:03, Hillf Danton <hdanton@sina.com> wrote: > > > > Hello Vincent > > > > On 16 Sep 2022 10:03:02 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote: > > > > > > @@ -4606,6 +4608,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > > > > > se = __pick_first_entity(cfs_rq); > > > delta = curr->vruntime - se->vruntime; > > > + delta -= wakeup_latency_gran(curr, se); > > > > > > if (delta < 0) > > > return; > > > > What is derived from the latency nice you added is the runtime granulaity > > which has a role in preempting the current task. > > > > Given the same defination of latency nice as the nice, the runtime granularity > > can be computed without introducing the latency nice. > > > > Only for thoughts now. > > > > Hillf > > > > +++ b/kernel/sched/fair.c > > @@ -4569,7 +4569,7 @@ dequeue_entity(struct cfs_rq *cfs_rq, st > > static void > > check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > { > > - unsigned long ideal_runtime, delta_exec; > > + unsigned long ideal_runtime, delta_exec, granu; > > struct sched_entity *se; > > s64 delta; > > > > @@ -4594,6 +4594,14 @@ check_preempt_tick(struct cfs_rq *cfs_rq > > return; > > > > se = __pick_first_entity(cfs_rq); > > + > > + granu = sysctl_sched_min_granularity + > > + (ideal_runtime - sysctl_sched_min_granularity) * > > + (se->latency_nice + 20) / LATENCY_NICE_WIDTH; > > There is no latency_nice field in se but a latency_offset instead > > Also at this step, we are sure that curr has run at least > sysctl_sched_min_granularity and we want now to compare curr vruntime > with first se's one. We take the latency offset into account to make > sure we will not preempt curr too early > > > + > > + if (delta_exec < granu) > > + return; > > + > > delta = curr->vruntime - se->vruntime; > > > > if (delta < 0) return; if (delta > ideal_runtime) resched_curr(rq_of(cfs_rq)); After another look, curr is not preempted without the gap in vruntime between curr and the first entity growing more than ideal runtime, while with latency_offset, since the gap becomes larger, preempt happens later than ideal runtime thoughts IMO. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 5/8] sched/fair: Take into account latency priority at wakeup 2022-09-17 22:58 ` Hillf Danton @ 2022-09-18 10:46 ` Vincent Guittot 2022-09-20 11:32 ` Hillf Danton 0 siblings, 1 reply; 6+ messages in thread From: Vincent Guittot @ 2022-09-18 10:46 UTC (permalink / raw) To: Hillf Danton Cc: peterz, mgorman, valentin.schneider, parth, linux-mm, linux-kernel On Sun, 18 Sept 2022 at 00:58, Hillf Danton <hdanton@sina.com> wrote: > > On 16 Sep 2022 15:36:53 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote: > > > > Hi Hillf, > > > > On Fri, 16 Sept 2022 at 14:03, Hillf Danton <hdanton@sina.com> wrote: > > > > > > Hello Vincent > > > > > > On 16 Sep 2022 10:03:02 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote: > > > > > > > > @@ -4606,6 +4608,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > > > > > > > se = __pick_first_entity(cfs_rq); > > > > delta = curr->vruntime - se->vruntime; > > > > + delta -= wakeup_latency_gran(curr, se); > > > > > > > > if (delta < 0) > > > > return; > > > > > > What is derived from the latency nice you added is the runtime granulaity > > > which has a role in preempting the current task. > > > > > > Given the same defination of latency nice as the nice, the runtime granularity > > > can be computed without introducing the latency nice. > > > > > > Only for thoughts now. > > > > > > Hillf > > > > > > +++ b/kernel/sched/fair.c > > > @@ -4569,7 +4569,7 @@ dequeue_entity(struct cfs_rq *cfs_rq, st > > > static void > > > check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > > { > > > - unsigned long ideal_runtime, delta_exec; > > > + unsigned long ideal_runtime, delta_exec, granu; > > > struct sched_entity *se; > > > s64 delta; > > > > > > @@ -4594,6 +4594,14 @@ check_preempt_tick(struct cfs_rq *cfs_rq > > > return; > > > > > > se = __pick_first_entity(cfs_rq); > > > + > > > + granu = sysctl_sched_min_granularity + > > > + (ideal_runtime - sysctl_sched_min_granularity) * > > > + (se->latency_nice + 20) / LATENCY_NICE_WIDTH; > > > > There is no latency_nice field in se but a latency_offset instead > > > > Also at this step, we are sure that curr has run at least > > sysctl_sched_min_granularity and we want now to compare curr vruntime > > with first se's one. We take the latency offset into account to make > > sure we will not preempt curr too early > > > > > + > > > + if (delta_exec < granu) > > > + return; > > > + > > > delta = curr->vruntime - se->vruntime; > > > > > > if (delta < 0) > return; > > if (delta > ideal_runtime) > resched_curr(rq_of(cfs_rq)); > > After another look, curr is not preempted without the gap in vruntime > between curr and the first entity growing more than ideal runtime, while Curr can be preempted as it has run more than the ideal time (1st test). This one is to make sure that the diff does not become too large. Here we reuse the same comparison as wakeup to make sure that a newly curr will get a chance to run its ideal time after having preempted at wakeup the previous current > with latency_offset, since the gap becomes larger, preempt happens later > than ideal runtime thoughts IMO. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 5/8] sched/fair: Take into account latency priority at wakeup 2022-09-18 10:46 ` Vincent Guittot @ 2022-09-20 11:32 ` Hillf Danton 2022-09-20 15:17 ` Vincent Guittot 0 siblings, 1 reply; 6+ messages in thread From: Hillf Danton @ 2022-09-20 11:32 UTC (permalink / raw) To: Vincent Guittot Cc: peterz, mgorman, valentin.schneider, parth, linux-mm, linux-kernel On 18 Sep 2022 12:46:00 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote: > On Sun, 18 Sept 2022 at 00:58, Hillf Danton <hdanton@sina.com> wrote: > > > > On 16 Sep 2022 15:36:53 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote: > > > > > > Hi Hillf, > > > > > > On Fri, 16 Sept 2022 at 14:03, Hillf Danton <hdanton@sina.com> wrote: > > > > > > > > Hello Vincent > > > > > > > > On 16 Sep 2022 10:03:02 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote: > > > > > > > > > > @@ -4606,6 +4608,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > > > > > > > > > se = __pick_first_entity(cfs_rq); > > > > > delta = curr->vruntime - se->vruntime; > > > > > + delta -= wakeup_latency_gran(curr, se); > > > > > > > > > > if (delta < 0) > > > > > return; > > > > > > > > What is derived from the latency nice you added is the runtime granulaity > > > > which has a role in preempting the current task. > > > > > > > > Given the same defination of latency nice as the nice, the runtime granularity > > > > can be computed without introducing the latency nice. > > > > > > > > Only for thoughts now. > > > > > > > > Hillf > > > > > > > > +++ b/kernel/sched/fair.c > > > > @@ -4569,7 +4569,7 @@ dequeue_entity(struct cfs_rq *cfs_rq, st > > > > static void > > > > check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > > > { > > > > - unsigned long ideal_runtime, delta_exec; > > > > + unsigned long ideal_runtime, delta_exec, granu; > > > > struct sched_entity *se; > > > > s64 delta; > > > > > > > > @@ -4594,6 +4594,14 @@ check_preempt_tick(struct cfs_rq *cfs_rq > > > > return; > > > > > > > > se = __pick_first_entity(cfs_rq); > > > > + > > > > + granu = sysctl_sched_min_granularity + > > > > + (ideal_runtime - sysctl_sched_min_granularity) * > > > > + (se->latency_nice + 20) / LATENCY_NICE_WIDTH; > > > > > > There is no latency_nice field in se but a latency_offset instead > > > > > > Also at this step, we are sure that curr has run at least > > > sysctl_sched_min_granularity and we want now to compare curr vruntime > > > with first se's one. We take the latency offset into account to make > > > sure we will not preempt curr too early > > > > > > > + > > > > + if (delta_exec < granu) > > > > + return; > > > > + > > > > delta = curr->vruntime - se->vruntime; > > > > > > > > if (delta < 0) > > return; > > > > if (delta > ideal_runtime) > > resched_curr(rq_of(cfs_rq)); > > > > After another look, curr is not preempted without the gap in vruntime > > between curr and the first entity growing more than ideal runtime, while > > Curr can be preempted as it has run more than the ideal time (1st > test). This one is to make sure that the diff does not become too > large. Here we reuse the same comparison as wakeup to make sure that a > newly curr will get a chance to run its ideal time after having > preempted at wakeup the previous current IIUC it would take two checks to preempt correctly - diff in vruntime is checked first to avoid preempting too early, then it is checked again with latency_offset taken into account to avoid preempting too late. +++ b/kernel/sched/fair.c @@ -4571,7 +4571,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq { unsigned long ideal_runtime, delta_exec; struct sched_entity *se; - s64 delta; + s64 delta, d2; ideal_runtime = sched_slice(cfs_rq, curr); delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime; @@ -4595,11 +4595,9 @@ check_preempt_tick(struct cfs_rq *cfs_rq se = __pick_first_entity(cfs_rq); delta = curr->vruntime - se->vruntime; + d2 = delta - wakeup_latency_gran(curr, se); - if (delta < 0) - return; - - if (delta > ideal_runtime) + if (delta > ideal_runtime || d2 > ideal_runtime) resched_curr(rq_of(cfs_rq)); } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 5/8] sched/fair: Take into account latency priority at wakeup 2022-09-20 11:32 ` Hillf Danton @ 2022-09-20 15:17 ` Vincent Guittot 0 siblings, 0 replies; 6+ messages in thread From: Vincent Guittot @ 2022-09-20 15:17 UTC (permalink / raw) To: Hillf Danton Cc: peterz, mgorman, valentin.schneider, parth, linux-mm, linux-kernel On Tue, 20 Sept 2022 at 13:32, Hillf Danton <hdanton@sina.com> wrote: > > On 18 Sep 2022 12:46:00 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote: > > On Sun, 18 Sept 2022 at 00:58, Hillf Danton <hdanton@sina.com> wrote: > > > > > > On 16 Sep 2022 15:36:53 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote: > > > > > > > > Hi Hillf, > > > > > > > > On Fri, 16 Sept 2022 at 14:03, Hillf Danton <hdanton@sina.com> wrote: > > > > > > > > > > Hello Vincent > > > > > > > > > > On 16 Sep 2022 10:03:02 +0200 Vincent Guittot <vincent.guittot@linaro.org> wrote: > > > > > > > > > > > > @@ -4606,6 +4608,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > > > > > > > > > > > se = __pick_first_entity(cfs_rq); > > > > > > delta = curr->vruntime - se->vruntime; > > > > > > + delta -= wakeup_latency_gran(curr, se); > > > > > > > > > > > > if (delta < 0) > > > > > > return; > > > > > > > > > > What is derived from the latency nice you added is the runtime granulaity > > > > > which has a role in preempting the current task. > > > > > > > > > > Given the same defination of latency nice as the nice, the runtime granularity > > > > > can be computed without introducing the latency nice. > > > > > > > > > > Only for thoughts now. > > > > > > > > > > Hillf > > > > > > > > > > +++ b/kernel/sched/fair.c > > > > > @@ -4569,7 +4569,7 @@ dequeue_entity(struct cfs_rq *cfs_rq, st > > > > > static void > > > > > check_preempt_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > > > > { > > > > > - unsigned long ideal_runtime, delta_exec; > > > > > + unsigned long ideal_runtime, delta_exec, granu; > > > > > struct sched_entity *se; > > > > > s64 delta; > > > > > > > > > > @@ -4594,6 +4594,14 @@ check_preempt_tick(struct cfs_rq *cfs_rq > > > > > return; > > > > > > > > > > se = __pick_first_entity(cfs_rq); > > > > > + > > > > > + granu = sysctl_sched_min_granularity + > > > > > + (ideal_runtime - sysctl_sched_min_granularity) * > > > > > + (se->latency_nice + 20) / LATENCY_NICE_WIDTH; > > > > > > > > There is no latency_nice field in se but a latency_offset instead > > > > > > > > Also at this step, we are sure that curr has run at least > > > > sysctl_sched_min_granularity and we want now to compare curr vruntime > > > > with first se's one. We take the latency offset into account to make > > > > sure we will not preempt curr too early > > > > > > > > > + > > > > > + if (delta_exec < granu) > > > > > + return; > > > > > + > > > > > delta = curr->vruntime - se->vruntime; > > > > > > > > > > if (delta < 0) > > > return; > > > > > > if (delta > ideal_runtime) > > > resched_curr(rq_of(cfs_rq)); > > > > > > After another look, curr is not preempted without the gap in vruntime > > > between curr and the first entity growing more than ideal runtime, while > > > > Curr can be preempted as it has run more than the ideal time (1st > > test). This one is to make sure that the diff does not become too > > large. Here we reuse the same comparison as wakeup to make sure that a > > newly curr will get a chance to run its ideal time after having > > preempted at wakeup the previous current > > IIUC it would take two checks to preempt correctly - diff in vruntime > is checked first to avoid preempting too early, then it is checked again > with latency_offset taken into account to avoid preempting too late. The 1st test in check_preempt_tick() : if (delta_exec > ideal_runtime) ensures that a resched happens after the current run is slice The 2nd test : if (delta_exec < sysctl_sched_min_granularity) ensures that current will run at least 3ms The 3rd one : if (delta > ideal_runtime) is there to make sure that there is not too much diff between the vruntime. But we are comparing virtual runtime with execution time and as Peter mentioned in another thread that's weird. [1] will fix it in addition to ensure max runtime. Then, current might have preempted first_entity few ms before thanks to its latency_offset. If the tick happens quickly after the preemption, delta might be above ideal_runtime whereas current has run its ideal time yet [1] https://lore.kernel.org/all/20220916131538.24706-1-vincent.guittot@linaro.org/ > > +++ b/kernel/sched/fair.c > @@ -4571,7 +4571,7 @@ check_preempt_tick(struct cfs_rq *cfs_rq > { > unsigned long ideal_runtime, delta_exec; > struct sched_entity *se; > - s64 delta; > + s64 delta, d2; > > ideal_runtime = sched_slice(cfs_rq, curr); > delta_exec = curr->sum_exec_runtime - curr->prev_sum_exec_runtime; > @@ -4595,11 +4595,9 @@ check_preempt_tick(struct cfs_rq *cfs_rq > > se = __pick_first_entity(cfs_rq); > delta = curr->vruntime - se->vruntime; > + d2 = delta - wakeup_latency_gran(curr, se); > > - if (delta < 0) > - return; > - > - if (delta > ideal_runtime) > + if (delta > ideal_runtime || d2 > ideal_runtime) > resched_curr(rq_of(cfs_rq)); > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-20 15:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20220916080305.29574-6-vincent.guittot@linaro.org>
2022-09-16 12:02 ` [PATCH v4 5/8] sched/fair: Take into account latency priority at wakeup Hillf Danton
2022-09-16 13:36 ` Vincent Guittot
2022-09-17 22:58 ` Hillf Danton
2022-09-18 10:46 ` Vincent Guittot
2022-09-20 11:32 ` Hillf Danton
2022-09-20 15:17 ` Vincent Guittot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).