Messages in this thread | | | Date | Tue, 1 Nov 2022 11:34:04 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH] sched: Clear ttwu_pending after enqueue_task |
| |
On Tue, Nov 01, 2022 at 03:36:30PM +0800, Tianchen Ding wrote: > We found a long tail latency in schbench whem m*t is close to nr_cpus. > (e.g., "schbench -m 2 -t 16" on a machine with 32 cpus.) > > This is because when the wakee cpu is idle, rq->ttwu_pending is cleared > too early, and idle_cpu() will return true until the wakee task enqueued. > This will mislead the waker when selecting idle cpu, and wake multiple > worker threads on the same wakee cpu. This situation is enlarged by > commit f3dd3f674555 ("sched: Remove the limitation of WF_ON_CPU on > wakelist if wakee cpu is idle") because it tends to use wakelist. > > Here is the result of "schbench -m 2 -t 16" on a VM with 32vcpu > (Intel(R) Xeon(R) Platinum 8369B). > > Latency percentiles (usec): > base base+revert_f3dd3f674555 base+this_patch > 50.0000th: 9 13 9 > 75.0000th: 12 19 12 > 90.0000th: 15 22 15 > 95.0000th: 18 24 17 > *99.0000th: 27 31 24 > 99.5000th: 3364 33 27 > 99.9000th: 12560 36 30
Nice; but have you also ran other benchmarks and confirmed it doesn't negatively affect those?
If so; mentioning that is very helpful. If not; best go do so :-)
> Signed-off-by: Tianchen Ding <dtcccc@linux.alibaba.com> > --- > kernel/sched/core.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 87c9cdf37a26..b07de1753be5 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -3739,13 +3739,6 @@ void sched_ttwu_pending(void *arg) > if (!llist) > return; > > - /* > - * rq::ttwu_pending racy indication of out-standing wakeups. > - * Races such that false-negatives are possible, since they > - * are shorter lived that false-positives would be. > - */ > - WRITE_ONCE(rq->ttwu_pending, 0); > - > rq_lock_irqsave(rq, &rf); > update_rq_clock(rq); >
Could you try the below instead? Also note the comment; since you did the work to figure out why -- best record that for posterity.
@@ -3737,6 +3730,13 @@ void sched_ttwu_pending(void *arg) set_task_cpu(p, cpu_of(rq));
ttwu_do_activate(rq, p, p->sched_remote_wakeup ? WF_MIGRATED : 0, &rf); + /* + * Must be after enqueueing at least once task such that + * idle_cpu() does not observe a false-negative -- if it does, + * it is possible for select_idle_siblings() to stack a number + * of tasks on this CPU during that window. + */ + WRITE_ONCE(rq->ttwu_pending, 0); } rq_unlock_irqrestore(rq, &rf);
| |