Re: [PATCH v2] sched/eevdf: Fix wakeup-preempt by checking cfs_rq->nr_running

[Chen Yu]

On 2024-09-30 at 10:04:13 +0800, Honglei Wang wrote:
> Hi Oliver,
> 
> On 2024/9/29 21:51, Oliver Sang wrote:
> > hi, Chenyu and Honglei,
> > 
> > On Sat, Sep 28, 2024 at 02:28:30PM +0800, Chen Yu wrote:
> >> Hi Honglei,
> >>
> >> On 2024-09-27 at 21:38:53 +0800, Honglei Wang wrote:
> >>> Hi Yu,
> >>>
> >>> Yep, I understand the preemption should happen in the same cfs level. Just
> >>> not sure the purpose of the 'nr_running check' stuff. Perhaps its role is
> >>> just to judge whether it’s necessary to do the preemption check. If there is
> >>> at least one more ready (cfs) task in the rq and current is not eligible, we
> >>> take care of the waiting tasks. Thoughts?
> >>
> >> I got your point and it makes sense. Whether the preemption check should be triggered
> >> seems to be a heuristic trade-off to me. I'm ok with using more aggressive preemption
> >> strategy as it depends on whether that workload prefers latency or throughput, and as
> >> long as it did not introduce regression :-)
> >>
> >> Oliver, may I know if you happen to have time for a test if the following change
> >> suggested by Honglei would make the regression go away? Thanks.
> > 
> > I applied the patch directly upon 85e511df3cec, the test found it can reduce the
> > regression but not totally reovered
> > 
> > =========================================================================================
> > compiler/cpufreq_governor/ipc/iterations/kconfig/mode/nr_threads/rootfs/tbox_group/testcase:
> >   gcc-12/performance/socket/4/x86_64-rhel-8.3/process/50%/debian-12-x86_64-20240206.cgz/lkp-spr-r02/hackbench
> > 
> > commit:
> >   82e9d0456e06 ("sched/fair: Avoid re-setting virtual deadline on 'migrations'")
> >   85e511df3cec ("sched/eevdf: Allow shorter slices to wakeup-preempt")
> >   8079496d311b  <--- patch from Honglei
> > 
> > 82e9d0456e06cebe 85e511df3cec46021024176672a 8079496d311b6b0d4dae973f4df
> > ---------------- --------------------------- ---------------------------
> >          %stddev     %change         %stddev     %change         %stddev
> >              \          |                \          |                \
> >     623219           -13.1%     541887            -3.2%     603080        hackbench.throughput
> > 
> 
> Thanks a lot for running the test. The result is as expectation, as the
> strategy of short slices tends to favor more frequent scheduling to help
> delay-sensitive tasks acquire CPU as early as possible.
> 
> I suspect that the current test environment does not have any special
> configurations for slices. In this case, a 3.2% regression is still
> somewhat significant. As Yu mentioned, this is a heuristic adjustment.
> In this particular case, it seems that Yu's patch is more effective in
> solving the problem. Let's delegate the preemption opportunity to the
> higher-level update_curr() function.
>

Thanks Oliver and Hongwei.

Hi Hongwei, do you mind if I added your Reviewed-by tag on this?

thanks,
Chenyu