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

[Marc Zyngier <marc.zyngier@arm.com>]

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.

> 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.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm