Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 31 May 2023 19:22:43 +0100
From:   Qais Yousef <qyousef@layalina.io>
To:     Lukasz Luba <lukasz.luba@arm.com>
Cc:     Vincent Guittot <vincent.guittot@linaro.org>,
        Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        linux-kernel@vger.kernel.org, Wei Wang <wvw@google.com>,
        Xuewen Yan <xuewen.yan94@gmail.com>,
        Hank <han.lin@mediatek.com>,
        Jonathan JMChen <Jonathan.JMChen@mediatek.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>
Subject: Re: [PATCH v2 1/3] sched/uclamp: Set max_spare_cap_cpu even if
 max_spare_cap is 0
Message-ID: <20230531182243.r4d5teuijiadwp2c@airbuntu>
References: <20230205224318.2035646-1-qyousef@layalina.io>
 <20230205224318.2035646-2-qyousef@layalina.io>
 <CAKfTPtA9yOncmGuLfo-XaXDZ5F7+9iL-hdyGeaOQi1qrDa_RqQ@mail.gmail.com>
 <9e935645-9baf-af9f-73bd-3eaeaec044a8@arm.com>
 <20230211175052.b7a4hddhkjk4j6qf@airbuntu>
 <48db3f08-a066-c078-bfc9-bf20f66e067a@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <48db3f08-a066-c078-bfc9-bf20f66e067a@arm.com>
Precedence: bulk

Hi Lukasz!

Sorry for late response..

On 05/22/23 09:30, Lukasz Luba wrote:
> Hi Qais,
> 
> I have a question regarding the 'soft cpu affinity'.

[...]

> > IIUC I'm not seeing this being a problem. The goal of capping with uclamp_max
> > is two folds:
> > 
> > 	1. Prevent tasks from consuming energy.
> > 	2. Keep them away from expensive CPUs.
> > 
> > 2 is actually very important for 2 reasons:
> > 
> > 	a. Because of max aggregation - any uncapped tasks that wakes up will
> > 	   cause a frequency spike on this 'expensive' cpu. We don't have
> > 	   a mechanism to downmigrate it - which is another thing I'm working
> > 	   on.
> > 	b. It is desired to keep these bigger cpu idle ready for more important
> > 	   work.
> > 
> > For 2, generally we don't want these tasks to steal bandwidth from these CPUs
> > that we'd like to preserve for other type of work.
> 
> I'm a bit afraid about such 'strong force'. That means the task would
> not go via EAS if we set uclamp_max e.g. 90, while the little capacity
> is 125. Or am I missing something?

We should go via EAS, actually that's the whole point.

Why do you think we won't go via EAS? The logic should be is we give a hint to
prefer the little core, but we still can pick something else if it's more
energy efficient.

What uclamp_max enables us is to still consider that little core even if it's
utilization says it doesn't fit there. We need to merge these patches first
though as it's broken at the moment. if little capacity is 125 and utilization
of the task is 125, then even if uclamp_max is 0, EAS will skip the whole
little cluster as apotential candidate because there's no spare_capacity there.
Even if the whole little cluster is idle.

> 
> This might effectively use more energy for those tasks which can run on
> any CPU and EAS would figure a good energy placement. I'm worried
> about this, since we have L3+littles in one DVFS domain and the L3
> would be only bigger in future.

It's a bias that will enable the search algorithm in EAS to still consider the
little core for big tasks. This bias will depend on the uclamp_max value chosen
by userspace (so they have some control on how hard to cap the task), and what
else is happening in the system at the time it wakes up.

> 
> IMO to keep the big cpus more in idle, we should give them big energy
> wake up cost. That's my 3rd feature to the EM presented in OSPM2023.

Considering the wake up cost in EAS would be a great addition to have :)

> 
> > 
> > Of course userspace has control by selecting the right uclamp_max value. They
> > can increase it to allow a spill to next pd - or keep it low to steer them more
> > strongly on a specific pd.
> 
> This would we be interesting to see in practice. I think we need such
> experiment, for such changes.

I'm not sure what you mean by 'such changes'. I hope you don't mean these
patches as they are not the key. They fix an obvious bug where task placement
hint won't work at all. They don't modify any behavior that shouldn't have
already been there. Nor introduce new limitation. I have to say I am
disappointed that these patches aren't considered an important fix for an
obvious breakage.


Thanks

--
Qais Yousef