Messages in this thread | | | Date | Sun, 11 Mar 2018 11:01:35 -0700 (PDT) | From | Thomas Gleixner <> | Subject | Re: [PATCH v4.16-rc4 1/1] x86/vdso: on Intel, VDSO should handle CLOCK_MONOTONIC_RAW |
| |
On Sun, 11 Mar 2018, Jason Vas Dias wrote:
This looks better now. Though running that patch through checkpatch.pl results in:
total: 28 errors, 20 warnings, 139 lines checked
....
> +notrace static u64 vread_tsc_raw(void)
Why do you need a separate function? I asked you to use vread_tsc(). So you might have reasons for doing that, but please then explain WHY and not just throw the stuff in my direction w/o any comment.
> +{ > + u64 tsc, last=gtod->raw_cycle_last; > + if( likely( gtod->has_rdtscp ) ) { > + u32 tsc_lo, tsc_hi, > + tsc_cpu __attribute__((unused)); > + asm volatile > + ( "rdtscp" > + /* ^- has built-in cancellation point / pipeline stall "barrier" */ > + : "=a" (tsc_lo) > + , "=d" (tsc_hi) > + , "=c" (tsc_cpu) > + ); // since all variables 32-bit, eax, edx, ecx used - NOT rax, rdx, rcx > + tsc = ((((u64)tsc_hi) & 0xffffffffUL) << 32) | (((u64)tsc_lo) & 0xffffffffUL);
This is not required to make the vdso accessor for monotonic raw work.
If at all then the rdtscp support wants to be in a separate patch with a proper explanation.
Aside of that the code for rdtscp wants to be in a proper inline helper in the relevant header file and written according to the coding style the kernel uses for asm inlines.
> + } else { > + tsc = rdtsc_ordered(); > + } > + if (likely(tsc >= last)) > + return tsc; > + asm volatile (""); > + return last; > +}
The rest looks ok.
Thanks,
tglx
| |