Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <20220409135104.3733193-1-wuyun.abel@bytedance.com>
 <20220409135104.3733193-2-wuyun.abel@bytedance.com> <CABk29NtBL2WovUVcxXW8cF7Nk+UM_AeEJaX_JbQ4Wue-qMvz_w@mail.gmail.com>
 <56ded56d-f7ad-a49f-c19c-8aa40fb33a29@bytedance.com> <CABk29Nu+6vh825SqyNmSJoWBQmEBgCsk6mcEtWLmXCq_T0xUjg@mail.gmail.com>
 <45fdb9f1-ac3e-2b92-9bd9-0f5e5886e2c9@bytedance.com>
In-Reply-To: <45fdb9f1-ac3e-2b92-9bd9-0f5e5886e2c9@bytedance.com>
From:   Josh Don <joshdon@google.com>
Date:   Fri, 15 Apr 2022 16:21:30 -0700
Message-ID: <CABk29NtifQV7+kgg_8yLqGHraMAS48BJjmU9WfkJihu-C4c8hw@mail.gmail.com>
Subject: Re: [RFC v2 1/2] sched/fair: filter out overloaded cpus in SIS
To:     Abel Wu <wuyun.abel@bytedance.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Mel Gorman <mgorman@suse.de>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

> > Does this check help that much? Given that you added the filter below
> > to cut out searching overloaded cpus, I would think that the below is
> > sufficient.
>
> I see a ~10% performance drop in the higher load part of the hackbench
> and tbench without this check, in which cases system is quite overloaded
> and idle cpus can hardly exist.
>
> >
> > Another use case that would break with the above:
> >
> > A few cpus are reserved for a job, so that it always has a couple cpus
> > dedicated to it. It can run across the entire machine though (no
> > affinity restriction). If the rest of the machine is very busy, we'd
> > still want to be able to search for and find the idle reserved cpus
> > for the job.
>
> Yes, this could be true if very few cpus are reserved for the job. Along
> with the previous affinity case, I think the following might help both:
>
> static inline bool
> sched_domain_overloaded(struct sched_domain *sd, int nr_overloaded)
> {
>         return nr_overloaded == sd->span_weight;
> }
>
> Besides, I think sched_idle_balance() will work well on this case.

The change to sched_domain_overloaded SGTM. But note that an async
load balancing operation such as sched_idle_balance() can't be relied
on for keeping wakeup latency low if we fail to find an idle cpu to
wake on (and one exists).