Re: [PATCH] arm64: Clarify when cpu_enable() is called

From: Mark Brown <broonie@kernel.org>
To: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: catalin.marinas@arm.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64: Clarify when cpu_enable() is called
Date: Thu, 8 Aug 2019 14:46:43 +0100	[thread overview]
Message-ID: <20190808134643.GH3795@sirena.co.uk> (raw)
In-Reply-To: <4975be8c-09ca-9acc-06ea-87e831f7fd79@arm.com>

[-- Attachment #1.1: Type: text/plain, Size: 2003 bytes --]

On Thu, Aug 08, 2019 at 02:21:42PM +0100, Suzuki K Poulose wrote:
> On 08/08/2019 13:19, Mark Brown wrote:

> > > > 	If this is called for any CPU in the system then it will be
> > > > 	called for all of them.

> > > > might cover it?

> > > 	* current CPU. If this capability is detected by the kernel, this will
> > > 	* called on all the CPUs in the system, including the hotplugged
> > > 	* CPUs.
> > > 	*/

> > 	 If this capability is detected by the kernel
> > 	 this will called on all the CPUs in the system, including
> > 	 the hotplugged CPUs, regardless of if the capability was
> > 	 detected on that specific CPU.

> I think the only issue with this, as also with the original statement, is that
> you are overloading "detected" for the "specific CPU" case.  In the first
> use, the "detect" is dependent on the SCOPE of the capability and in the
> latter one

That's not quite what I'm trying to get over here - what I'm trying to
get over is that the enable does not have the same scope as the
detection, I think it's fairly natural to assume that that is the case.
That is to say that the behaviour for the system scope detection case is
expected but for anything that's CPU local it's a surprise.

> is strictly "LOCAL" scope. If you replace the second "detected" with say, "not
> available" or even "not matched", it makes it less confusing.

>  	If the capability is detected by the kernel this will be called on all
> 	the CPUs in the system, including the hotplugged CPUs, regardless of if
> 	the capability was *available* on that specific CPU. This is useful for
> 	some capabilities (e.g, working around CPU errata), where all the CPUs
> 	must take some action (e.g, changing system control/configuration).
> 	Thus, if an action is required only if the CPU has the capability, then
> 	the routine must check it before taking any action.

That's a bit verbose but I think it's sufficiently unambiguous. I'm
still confused about how this differs from what I originally proposed :/

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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