linux-kernel - Re: [RFC PATCH] sched/eevdf: Return leftmost entity in pick

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAB8ipk_B7zsQt2wa5pda4ehN6f0yHi_mV=unVThnOYTKQz=5aw@mail.gmail.com>
Date: Fri, 19 Apr 2024 17:17:17 +0800
From: Xuewen Yan <xuewen.yan94@...il.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Chen Yu <yu.c.chen@...el.com>, Abel Wu <wuyun.abel@...edance.com>, 
	Ingo Molnar <mingo@...hat.com>, Vincent Guittot <vincent.guittot@...aro.org>, 
	Juri Lelli <juri.lelli@...hat.com>, Tim Chen <tim.c.chen@...el.com>, 
	Tiwei Bie <tiwei.btw@...group.com>, Honglei Wang <wanghonglei@...ichuxing.com>, 
	Aaron Lu <aaron.lu@...el.com>, Chen Yu <yu.chen.surf@...il.com>, 
	Yujie Liu <yujie.liu@...el.com>, linux-kernel@...r.kernel.org, 
	kernel test robot <oliver.sang@...el.com>, Tianchen Ding <dtcccc@...ux.alibaba.com>
Subject: Re: [RFC PATCH] sched/eevdf: Return leftmost entity in pick_eevdf()
 if no eligible entity is found

On Fri, Apr 19, 2024 at 4:24 PM Peter Zijlstra <peterz@...radead.org> wrote:
>
> On Thu, Apr 18, 2024 at 09:03:36PM +0800, Chen Yu wrote:
>
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 31bca05c3612..9f203012e8f5 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -696,15 +696,23 @@ u64 avg_vruntime(struct cfs_rq *cfs_rq)
> >   *
> >   * XXX could add max_slice to the augmented data to track this.
> >   */
> > +
> > +static s64 limit_entity_lag(struct sched_entity *se, s64 lag)
> > +{
> > +     s64 limit;
> > +
> > +     limit = calc_delta_fair(max_t(u64, 2*se->slice, TICK_NSEC), se);
> > +     return clamp(lag, -limit, limit);
> > +}
>
> Right, helper makes sense.
>
> > @@ -3721,6 +3729,7 @@ static void reweight_eevdf(struct cfs_rq *cfs_rq, struct sched_entity *se,
> >       if (avruntime != se->vruntime) {
> >               vlag = (s64)(avruntime - se->vruntime);
> >               vlag = div_s64(vlag * old_weight, weight);
> > +             vlag = limit_entity_lag(se, vlag);
> >               se->vruntime = avruntime - vlag;
>
> So the !on_rq case has clamping in update_entity_lag() which is before
> scaling. And that makes more sense to me, because putting a limit on
> vlag before the multiplication *should* ensure the multiplication itself
> doesn't overflow.
>
> But now you allow it to compute garbage and then clip the garbage.
>
> >       }
> >
> > @@ -3768,6 +3777,9 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
> >
> >       update_load_set(&se->load, weight);
> >
> > +     if (!se->on_rq)
> > +             se->vlag = limit_entity_lag(se, se->vlag);
> > +
>
> Except you now add clamping after scaling too, but in a really weird
> place. Should this not go right after the div_s64() that scales?

Yes, it's better to put it  after the div_s64().

>
> Unlike the reweight_eevdf() case, there might be an argument for doing
> it after scaling in this case. Namely, you can have multiple reweights
> stacking their scale ops.
>
>
> Also, could you put a few words in on how often these clips are hit? I
> suspect it's fairly rare (but crucial when it does).

I think this probability may be not small, because the user space
often modifies the task's nice value while the task is sleeping.
I also would add logs to check it.

BR