Re: [PATCH] arch_topology: Update user supplied capacity to possible cpus in cluster

From: Sudeep Holla <sudeep.holla@arm.com>
To: Quentin Perret <quentin.perret@arm.com>
Cc: Chandra Sekhar Lingutla <clingutla@codeaurora.org>,
	catalin.marinas@arm.com, will.deacon@arm.com,
	Jeremy Linton <jeremy.linton@arm.com>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	dietmar.eggemann@arm.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arch_topology: Update user supplied capacity to possible cpus in cluster
Date: Tue, 5 Mar 2019 11:13:21 +0000	[thread overview]
Message-ID: <20190305111321.GA5458@e107155-lin> (raw)
In-Reply-To: <20190305092322.q7odi3inofnvzhre@queper01-lin>

On Tue, Mar 05, 2019 at 09:23:25AM +0000, Quentin Perret wrote:
> On Monday 04 Mar 2019 at 18:21:38 (+0000), Sudeep Holla wrote:
> > On Sat, Mar 02, 2019 at 07:00:43PM +0530, Chandra Sekhar Lingutla wrote:
> > > So cpus in cpu_topology->core_sibling mask would not need to have same
> > > capacity_cpu ?
> >
> > Yes, it need not. DSU is simple example. Even normal heterogeneous
> > multi-cluster single socket systems will have all the cpus in the die
> > present in core_siblings.
> >
> > > Then i think, we should update the cpu_capacity for only requested cpu
> > > right?
> >
> > One possible solution and a simpler one. But I am open to any better
> > alternative if it exists/possible.
>
> How about we update the capacity for the related_cpus of the CPUFreq
> policy ? This is what we're interested in here, I think, and is
> orthogonal to the topology stuff. And that should map fairly well to the
> core_sibling_mask for legacy platforms.
>

While I like the idea, I am afraid that linking this to cpufreq policy
may not be good. How will we deal with it on systems without CPUfreq ?

> FWIW, we already mandate something similar for EAS for example
> (see [1]), and I'm not sure we want to support having different uarchs
> in the same freq domain here either, even though strictly speaking
> DynamIQ doesn't forbid it.
>

Yes that dependency is other way around and topology is not optional, so
it works out well. The reverse may not be that simple.

> [1] https://elixir.bootlin.com/linux/latest/source/kernel/power/energy_model.c#L170
>
> [...]
>
> > I was always under the impression that this was in debugfs and will be
> > removed. I did mention this in one of the thread couple of months back.
> > I was wrong and do understand the need for this on system where firmware
> > doesn't provide this capacity value.
> >
> > If possible I want to drop the write capability for the sysfs.
>
> But yes, that is even better, if at all possible.
>

I think if there are no valid users of this, we *must* remove it. As I
have pointed out in past, giving user such access will need platform
knowledge. Though it's debatable topic, firmware providing this
information is the only correct solution IMO.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel