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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZvoLWQICLdTonRE/@chenyu5-mobl2>
Date: Mon, 30 Sep 2024 10:22:17 +0800
From: Chen Yu <yu.c.chen@...el.com>
To: Honglei Wang <jameshongleiwang@....com>
CC: Oliver Sang <oliver.sang@...el.com>, Ingo Molnar <mingo@...hat.com>,
	"Peter Zijlstra" <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>,
	"Vincent Guittot" <vincent.guittot@...aro.org>, Dietmar Eggemann
	<dietmar.eggemann@....com>, Valentin Schneider <vschneid@...hat.com>,
	"Chunxin Zang" <zangchunxin@...iang.com>, <linux-kernel@...r.kernel.org>,
	Chen Yu <yu.chen.surf@...il.com>, K Prateek Nayak <kprateek.nayak@....com>
Subject: Re: [PATCH v2] sched/eevdf: Fix wakeup-preempt by checking
 cfs_rq->nr_running

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