Re: [PATCH v6 07/18] arm64: cpufeature: Move cpu capability helpers inside C file

From: Catalin Marinas <catalin.marinas@arm.com>
To: Amit Kachhap <amit.kachhap@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Kees Cook <keescook@chromium.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Kristina Martsenko <kristina.martsenko@arm.com>,
	Dave Martin <Dave.Martin@arm.com>,
	Mark Brown <broonie@kernel.org>,
	James Morse <james.morse@arm.com>,
	Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>,
	Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Will Deacon <will@kernel.org>, Ard Biesheuvel <ardb@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 07/18] arm64: cpufeature: Move cpu capability helpers inside C file
Date: Wed, 11 Mar 2020 10:50:20 +0000	[thread overview]
Message-ID: <20200311105020.GA3216816@arrakis.emea.arm.com> (raw)
In-Reply-To: <e3749f3f-e314-c3b8-cf71-b1ea9d699f4e@arm.com>

On Tue, Mar 10, 2020 at 06:23:15PM +0530, Amit Kachhap wrote:
> On 3/10/20 5:50 PM, Vincenzo Frascino wrote:
> > On 3/6/20 6:35 AM, Amit Daniel Kachhap wrote:
> > 
> > [...]
> > 
> > > -static inline bool
> > > -cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap)
> > > -{
> > > -	return !!(cap->type & ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU);
> > > -}
> > > -
> > > -static inline bool
> > > -cpucap_late_cpu_permitted(const struct arm64_cpu_capabilities *cap)
> > > -{
> > > -	return !!(cap->type & ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU);
> > > -}
> > > -
> > >   /*
> > >    * Generic helper for handling capabilties with multiple (match,enable) pairs
> > >    * of call backs, sharing the same capability bit.
> > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > index b12e386..865dce6 100644
> > > --- a/arch/arm64/kernel/cpufeature.c
> > > +++ b/arch/arm64/kernel/cpufeature.c
> > > @@ -1363,6 +1363,19 @@ static bool can_use_gic_priorities(const struct arm64_cpu_capabilities *entry,
> > >   }
> > >   #endif
> > > +/* Internal helper functions to match cpu capability type */
> > > +static bool
> > > +cpucap_late_cpu_optional(const struct arm64_cpu_capabilities *cap)
> > > +{
> > > +	return !!(cap->type & ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU);
> > > +}
> > > +
> > > +static bool
> > > +cpucap_late_cpu_permitted(const struct arm64_cpu_capabilities *cap)
> > > +{
> > > +	return !!(cap->type & ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU);
> > > +}
> > > +
> > >   static const struct arm64_cpu_capabilities arm64_features[] = {
> > >   	{
> > >   		.desc = "GIC system register CPU interface",
> > > 
> > 
> > Seems that the signature of the functions above is changed during the migration.
> > In particular you dropped "inline". Is there any specific reason?
> 
> Earlier Catalin pointed me here [1]. I guess its not a hot-path function.
> 
> [1]: https://www.spinics.net/lists/arm-kernel/msg789696.html

Indeed, it had to be static inline in a header but not anymore in a .c
file. Also if it's really essential to be inlined and the compiler
doesn't do this automatically, use __always_inline. But my preference is
not to bother unless you back it up by numbers.

-- 
Catalin

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