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

[Honglei Wang]

On 2024/9/30 10:22, Chen Yu wrote:
> 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?
> 

OK. Feel free to include:

Reviewed-by: Honglei Wang <jameshongleiwang@xxxxxxx>

Thanks,
Honglei

> thanks,
> Chenyu