On Saturday 27 January 2018 09:51:45 Guenter Roeck wrote:
> 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@xxxxxxxxx>
> > ---
> > 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 ?
Ah, I have not noticed this yet :-(
> 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.
Ok, I will fix this patch and resend just new version of this one.
Anyway, if 2/4 is targeting stable, then 3/4 should too. But now I see
that I added Cc only to patch 2/4.
--
Pali Rohár
pali.rohar@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
