Re: [4/4] hwmon: (dell-smm) Measure time duration of SMM call around inlined asm

[Guenter Roeck <linux@roeck-us.net>]

On Sat, Jan 27, 2018 at 05:23:51PM +0100, Pali Rohár wrote:
> Measure only inlined asm code, not other functions to have as precise as
> possible measured time.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/hwmon/dell-smm-hwmon.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index bf3bb7e1adab..e001afd53f46 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -147,14 +147,16 @@ static int i8k_smm_func(void *par)
>  	int ebx = regs->ebx;
>  	unsigned long duration;
>  	ktime_t calltime, delta, rettime;
> -
> -	calltime = ktime_get();
>  #endif
>  
>  	/* SMM requires CPU 0 */
>  	if (smp_processor_id() != 0)
>  		return -EBUSY;
>  
> +#ifdef DEBUG
> +	calltime = ktime_get();
> +#endif
> +
>  #if defined(CONFIG_X86_64)
>  	asm volatile("pushq %%rax\n\t"
>  		"movl 0(%%rax),%%edx\n\t"
> @@ -208,13 +210,17 @@ static int i8k_smm_func(void *par)
>  	    :    "a"(regs)
>  	    :    "%ebx", "%ecx", "%edx", "%esi", "%edi", "memory");
>  #endif
> -	if (rc != 0 || (regs->eax & 0xffff) == 0xffff || regs->eax == eax)
> -		rc = -EINVAL;
>  
>  #ifdef DEBUG
>  	rettime = ktime_get();
>  	delta = ktime_sub(rettime, calltime);
>  	duration = ktime_to_ns(delta) >> 10;
> +#endif
> +
> +	if (rc != 0 || (regs->eax & 0xffff) == 0xffff || regs->eax == eax)
> +		rc = -EINVAL;
> +
> +#ifdef DEBUG

FWIW, the error introduced by dividing nS by 1,024 instead of 1,000 is
much worse than the improvements from moving the calls around. Using
specific numbers, the current code reports 500 mS as 488,281 uS.
So why bother ?

I would have suggested to use ktime_us_delta(ktime_get(), calltime)
instead to make the results more accurate. Sure, you get the offset from
the additional divide operation, but at least that would be a constant
and not a systematic error.

I'll hold this patch off for a bit. Please confirm that you want it
applied as-is. I applied the other patches from the series.

Thanks,
Guenter