Re: [PATCH] arm64: KVM: debug: Remove spurious inline attributes

["Alex Bennée" <alex.bennee@linaro.org>]

Marc Zyngier <marc.zyngier@arm.com> writes:

> On 17/12/15 16:28, Alex Bennée wrote:
>>
>> Marc Zyngier <marc.zyngier@arm.com> writes:
>>
>>> The debug trapping code is pretty heavy on the "inline" attribute,
>>> but most functions are actually referenced in the sysreg tables,
>>> making the inlining imposible.
>>>
>>> Removing the useless inline qualifier seems the right thing to do,
>>> having verified that the output code is similar.
>>>
>>> Cc: Alex Bennée <alex.bennee@linaro.org>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>>  arch/arm64/kvm/sys_regs.c | 58 +++++++++++++++++++++++------------------------
>>>  1 file changed, 29 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>>> index 88adebf..eec3598 100644
>>> --- a/arch/arm64/kvm/sys_regs.c
>>> +++ b/arch/arm64/kvm/sys_regs.c
>>> @@ -220,9 +220,9 @@ static bool trap_debug_regs(struct kvm_vcpu *vcpu,
>>>   * All writes will set the KVM_ARM64_DEBUG_DIRTY flag to ensure the
>>>   * hyp.S code switches between host and guest values in future.
>>>   */
>>> -static inline void reg_to_dbg(struct kvm_vcpu *vcpu,
>>> -			      struct sys_reg_params *p,
>>> -			      u64 *dbg_reg)
>>> +static void reg_to_dbg(struct kvm_vcpu *vcpu,
>>> +		       struct sys_reg_params *p,
>>> +		       u64 *dbg_reg)
>>>  {
>>>  	u64 val = p->regval;
>>>
>>> @@ -235,18 +235,18 @@ static inline void reg_to_dbg(struct kvm_vcpu *vcpu,
>>>  	vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
>>>  }
>>>
>>> -static inline void dbg_to_reg(struct kvm_vcpu *vcpu,
>>> -			      struct sys_reg_params *p,
>>> -			      u64 *dbg_reg)
>>> +static void dbg_to_reg(struct kvm_vcpu *vcpu,
>>> +		       struct sys_reg_params *p,
>>> +		       u64 *dbg_reg)
>>>  {
>>>  	p->regval = *dbg_reg;
>>>  	if (p->is_32bit)
>>>  		p->regval &= 0xffffffffUL;
>>>  }
>>
>> Christoffer's "register keyword" comments not-withstanding I'd prefer to
>> keep the reg_to_dbg/dbg_to_reg functions as inline because they really
>> are just boilerplate helpers I didn't want to repeat in the actual
>> access functions - although if you've looked at the code I assume that
>> means GCC has been smart about it.
>
> Indeed, GCC is smart enough to directly inline it. In general, GCC is
> doing a pretty good job at inlining static functions that are small
> enough not to be worth jumping to. These days, "static inline" only
> really makes sense in an include file.

Fair enough.

>
>> The rest all make sense. I wonder why I was being so inline happy?
>>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
> Thanks,
>
> 	M.


--
Alex Bennée