Messages in this thread Patch in this message |  | | From | Vincent Guittot <> | Date | Tue, 11 Oct 2022 19:10:21 +0200 | Subject | Re: [PATCH v5 7/7] sched/fair: Add latency list |
| |
Hi Youssef,
On Sat, 8 Oct 2022 at 03:05, Youssef Esmat <youssefesmat@chromium.org> wrote: > > Hi Vincent, > > > On Sun, Sep 25, 2022 at 9:39 AM Vincent Guittot > <vincent.guittot@linaro.org> wrote: > > > > Add a rb tree for latency sensitive entities so we can schedule the most > > sensitive one first even when it failed to preempt current at wakeup or > > when it got quickly preempted by another entity of higher priority. > > > > In order to keep fairness, the latency is used once at wakeup to get a > > minimum slice and not during the following scheduling slice to prevent > > long running entity to got more running time than allocated to his nice > > priority. > > > > The rb tree nebales to cover the last corner case where latency > > sensitive entity can't got schedule quickly after the wakeup. > > > > hackbench -l 10000 -g $group & > > cyclictest --policy other -D 5 -q -n > > latency 0 latency -20 > > group min avg max min avg max > > 0 17 19 29 17 18 30 > > 1 65 306 7149 64 83 208 > > 4 50 395 15731 56 80 271 > > 8 56 781 41548 54 80 301 > > 16 60 1392 87237 59 86 490 > > > > group = 0 means that hackbench is not running. > > > > Both avg and max are significantly improved with nice latency -20. If we > > add the histogram parameters to get details of latency, we have : > > > > hackbench -l 10000 -g 16 & > > cyclictest --policy other -D 5 -q -n -H 20000 --histfile data.txt > > latency 0 latency -20 > > Min Latencies: 60 61 > > Avg Latencies: 1077 86 > > Max Latencies: 87311 444 > > 50% latencies: 92 85 > > 75% latencies: 554 90 > > 85% latencies: 1019 93 > > 90% latencies: 1346 96 > > 95% latencies: 5400 100 > > 99% latencies: 19044 110 > > > > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> > > --- > > The ability to boost the latency sensitivity of a task seems very > interesting. I have been playing around with these changes and have > some observations. > > I tried 2 bursty tasks affinitized to the same CPU. The tasks sleep > for 1ms and run for 10ms in a loop. I first tried it without adjusting > the latency_nice value and took perf sched traces:
The CPU is overloaded almost all the time as it can't run the 2 tasks (2*10ms every 11ms)
> > latency_test:7040 | 2447.137 ms | 8 | avg: 6.546 ms | > max: 10.674 ms | max start: 353.809487 s | max end: 353.820161 s > latency_test:7028 | 2454.777 ms | 7 | avg: 4.494 ms | > max: 10.609 ms | max start: 354.804386 s | max end: 354.814995 s > > Everything looked as expected, for a 5s run they had similar runtime > and latency. > > I then adjusted one task to have a latency_nice of -20 (pid 8614 > below) and took another set of traces: > > latency_test:8618 | 1845.534 ms | 131 | avg: 9.764 ms | > max: 10.686 ms | max start: 1405.737905 s | max end: 1405.748592 s > latency_test:8614 | 3033.635 ms | 16 | avg: 3.559 ms | > max: 10.467 ms | max start: 1407.594751 s | max end: 1407.605218 s > > The task with -20 latency_nice had significantly more runtime. The > average latency was improved but the max roughly stayed the same. As > expected the one with latency_nice value of 0 experienced more > switches, but so did the one with latency_nice of -20.
Your results look unexpected because the vruntime of the tasks is not modified. So I can imagine that the thread with the low latency runs first up to the offset at the beg of the test but then they should switch regularly. I have tried a similar test with a modified rt-app and the result seems ok. I have a small difference but not the difference that you see.
Could you share more details about your setup ? I'm going to try to reproduce your sequence
> > Also tried running the same test but instead of using latency nice I > adjusted the nice value as a comparison. In that case one task had a > nice of -5 and the other was 0. > > nice_test:25219 | 1216.839 ms | 242 | avg: 10.295 ms | > max: 11.927 ms | max start: 5877.881279 s | max end: 5877.893206 s > nice_test:25235 | 3711.788 ms | 6 | avg: 1.026 ms | > max: 6.143 ms | max start: 5875.603741 s | max end: 5875.609883 s > > As expected the one with a nice value of -5 had more runtime but also > had better latency numbers than in the previous case of using > latency_nice. > > I also tried a similar test with 3 bursty tasks instead of two. In > this case all tasks had a latency_nice of 0: > > latency_test:11467 | 1641.131 ms | 161 | avg: 17.489 ms | > max: 21.011 ms | max start: 1542.656275 s | max end: 1542.677286 s > latency_test:11463 | 1644.809 ms | 161 | avg: 11.994 ms | > max: 25.012 ms | max start: 1545.657776 s | max end: 1545.682788 s > latency_test:11478 | 1643.211 ms | 160 | avg: 11.465 ms | > max: 21.012 ms | max start: 1546.159026 s | max end: 1546.180038 s > > Next I tried two tasks with a latency_nice of 0 and a third one had a > latency_nice of -20 (pid 11763 below): > > latency_test:11763 | 1645.482 ms | 159 | avg: 19.634 ms | > max: 31.016 ms | max start: 1623.834862 s | max end: 1623.865877 s > latency_test:11750 | 1644.276 ms | 259 | avg: 9.985 ms | > max: 21.012 ms | max start: 1623.953921 s | max end: 1623.974933 s > latency_test:11747 | 1642.745 ms | 262 | avg: 9.079 ms | > max: 25.013 ms | max start: 1620.980435 s | max end: 1621.005447 s > > In this case it seemed like the runtime was not affected by the > latency_nice value, but strangely the task with the latency nice of > -20 had a worse average and max latency than the other two. The > context switch times are also increased from the previous case. > > Have we considered an approach where the task that is marked as > latency sensitive gets a boosted nice value when it sleeps and is > either scaled down exponentially as it runs, or immediately reset to > its default when it runs for one tick? > > Thanks, > Youssef > > > > include/linux/sched.h | 2 + > > kernel/sched/fair.c | 96 +++++++++++++++++++++++++++++++++++++++++-- > > kernel/sched/sched.h | 1 + > > 3 files changed, 96 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/sched.h b/include/linux/sched.h > > index a74cad08e91e..0b92674e3664 100644 > > --- a/include/linux/sched.h > > +++ b/include/linux/sched.h > > @@ -547,6 +547,8 @@ struct sched_entity { > > /* For load-balancing: */ > > struct load_weight load; > > struct rb_node run_node; > > + struct rb_node latency_node; > > + unsigned int on_latency; > > struct list_head group_node; > > unsigned int on_rq; > > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > > index e524e892d118..1a72f34136d8 100644 > > --- a/kernel/sched/fair.c > > +++ b/kernel/sched/fair.c > > @@ -664,7 +664,77 @@ struct sched_entity *__pick_last_entity(struct cfs_rq *cfs_rq) > > > > return __node_2_se(last); > > } > > +#endif > > > > +/************************************************************** > > + * Scheduling class tree data structure manipulation methods: > > + * for latency > > + */ > > + > > +static inline bool latency_before(struct sched_entity *a, > > + struct sched_entity *b) > > +{ > > + return (s64)(a->vruntime + a->latency_offset - b->vruntime - b->latency_offset) < 0; > > +} > > + > > +#define __latency_node_2_se(node) \ > > + rb_entry((node), struct sched_entity, latency_node) > > + > > +static inline bool __latency_less(struct rb_node *a, const struct rb_node *b) > > +{ > > + return latency_before(__latency_node_2_se(a), __latency_node_2_se(b)); > > +} > > + > > +/* > > + * Enqueue an entity into the latency rb-tree: > > + */ > > +static void __enqueue_latency(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags) > > +{ > > + > > + /* Only latency sensitive entity can be added to the list */ > > + if (se->latency_offset >= 0) > > + return; > > + > > + if (se->on_latency) > > + return; > > + > > + /* > > + * An execution time less than sysctl_sched_min_granularity means that > > + * the entity has been preempted by a higher sched class or an entity > > + * with higher latency constraint. > > + * Put it back in the list so it gets a chance to run 1st during the > > + * next slice. > > + */ > > + if (!(flags & ENQUEUE_WAKEUP)) { > > + u64 delta_exec = se->sum_exec_runtime - se->prev_sum_exec_runtime; > > + > > + if (delta_exec >= sysctl_sched_min_granularity) > > + return; > > + } > > + > > + rb_add_cached(&se->latency_node, &cfs_rq->latency_timeline, __latency_less); > > + se->on_latency = 1; > > +} > > + > > +static void __dequeue_latency(struct cfs_rq *cfs_rq, struct sched_entity *se) > > +{ > > + if (se->on_latency) { > > + rb_erase_cached(&se->latency_node, &cfs_rq->latency_timeline); > > + se->on_latency = 0; > > + } > > +} > > + > > +static struct sched_entity *__pick_first_latency(struct cfs_rq *cfs_rq) > > +{ > > + struct rb_node *left = rb_first_cached(&cfs_rq->latency_timeline); > > + > > + if (!left) > > + return NULL; > > + > > + return __latency_node_2_se(left); > > +} > > + > > +#ifdef CONFIG_SCHED_DEBUG > > /************************************************************** > > * Scheduling class statistics methods: > > */ > > @@ -4455,8 +4525,10 @@ enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags) > > check_schedstat_required(); > > update_stats_enqueue_fair(cfs_rq, se, flags); > > check_spread(cfs_rq, se); > > - if (!curr) > > + if (!curr) { > > __enqueue_entity(cfs_rq, se); > > + __enqueue_latency(cfs_rq, se, flags); > > + } > > se->on_rq = 1; > > > > if (cfs_rq->nr_running == 1) { > > @@ -4542,8 +4614,10 @@ dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags) > > > > clear_buddies(cfs_rq, se); > > > > - if (se != cfs_rq->curr) > > + if (se != cfs_rq->curr) { > > __dequeue_entity(cfs_rq, se); > > + __dequeue_latency(cfs_rq, se); > > + } > > se->on_rq = 0; > > account_entity_dequeue(cfs_rq, se); > > > > @@ -4631,6 +4705,7 @@ set_next_entity(struct cfs_rq *cfs_rq, struct sched_entity *se) > > */ > > update_stats_wait_end_fair(cfs_rq, se); > > __dequeue_entity(cfs_rq, se); > > + __dequeue_latency(cfs_rq, se); > > update_load_avg(cfs_rq, se, UPDATE_TG); > > } > > > > @@ -4669,7 +4744,7 @@ static struct sched_entity * > > pick_next_entity(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > { > > struct sched_entity *left = __pick_first_entity(cfs_rq); > > - struct sched_entity *se; > > + struct sched_entity *latency, *se; > > > > /* > > * If curr is set we have to see if its left of the leftmost entity > > @@ -4711,6 +4786,12 @@ pick_next_entity(struct cfs_rq *cfs_rq, struct sched_entity *curr) > > se = cfs_rq->last; > > } > > > > + /* Check for latency sensitive entity waiting for running */ > > + latency = __pick_first_latency(cfs_rq); > > + if (latency && (latency != se) && > > + wakeup_preempt_entity(latency, se) < 1) > > + se = latency; > > + > > return se; > > } > > > > @@ -4734,6 +4815,7 @@ static void put_prev_entity(struct cfs_rq *cfs_rq, struct sched_entity *prev) > > update_stats_wait_start_fair(cfs_rq, prev); > > /* Put 'current' back into the tree. */ > > __enqueue_entity(cfs_rq, prev); > > + __enqueue_latency(cfs_rq, prev, 0); > > /* in !on_rq case, update occurred at dequeue */ > > update_load_avg(cfs_rq, prev, 0); > > } > > @@ -11717,6 +11799,7 @@ static void set_next_task_fair(struct rq *rq, struct task_struct *p, bool first) > > void init_cfs_rq(struct cfs_rq *cfs_rq) > > { > > cfs_rq->tasks_timeline = RB_ROOT_CACHED; > > + cfs_rq->latency_timeline = RB_ROOT_CACHED; > > u64_u32_store(cfs_rq->min_vruntime, (u64)(-(1LL << 20))); > > #ifdef CONFIG_SMP > > raw_spin_lock_init(&cfs_rq->removed.lock); > > @@ -12025,8 +12108,15 @@ int sched_group_set_latency(struct task_group *tg, s64 latency) > > > > for_each_possible_cpu(i) { > > struct sched_entity *se = tg->se[i]; > > + struct rq *rq = cpu_rq(i); > > + struct rq_flags rf; > > + > > + rq_lock_irqsave(rq, &rf); > > > > + __dequeue_latency(se->cfs_rq, se); > > WRITE_ONCE(se->latency_offset, latency); > > + > > + rq_unlock_irqrestore(rq, &rf); > > } > > > > mutex_unlock(&shares_mutex); > > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h > > index a15fb955092c..76bca172585c 100644 > > --- a/kernel/sched/sched.h > > +++ b/kernel/sched/sched.h > > @@ -599,6 +599,7 @@ struct cfs_rq { > > #endif > > > > struct rb_root_cached tasks_timeline; > > + struct rb_root_cached latency_timeline; > > > > /* > > * 'curr' points to currently running entity on this cfs_rq. > > -- > > 2.17.1 > > > > > >
|  |