RE: [PATCH v2 08/10] clocksource/drivers/hyper-v: Handle sched_clock differences inline
From: Michael Kelley <hidden>
Date: 2021-03-02 07:47:27
Also in:
linux-arch, linux-hyperv
From: Daniel Lezcano <redacted> Sent: Monday, March 1, 2021 6:25 AM
On 01/03/2021 02:15, Michael Kelley wrote:quoted
While the Hyper-V Reference TSC code is architecture neutral, the pv_ops.time.sched_clock() function is implemented for x86/x64, but not for ARM64. Current code calls a utility function under arch/x86 (and coming, under arch/arm64) to handle the difference. Change this approach to handle the difference inline based on whether GENERIC_SCHED_CLOCK is present. The new approach removes code under arch/* since the difference is tied more to the specifics of the Linux implementation than to the architecture. No functional change. Signed-off-by: Michael Kelley <redacted> Reviewed-by: Boqun Feng <redacted> ---[ ... ]quoted
+/* + * Reference to pv_ops must be inline so objtool + * detection of noinstr violations can work correctly. + */ +static __always_inline void hv_setup_sched_clock(void *sched_clock) +{ +#ifdef CONFIG_GENERIC_SCHED_CLOCK + /* + * We're on an architecture with generic sched clock (not x86/x64). + * The Hyper-V sched clock read function returns nanoseconds, not + * the normal 100ns units of the Hyper-V synthetic clock. + */ + sched_clock_register(sched_clock, 64, NSEC_PER_SEC); +#else +#ifdef CONFIG_PARAVIRT + /* We're on x86/x64 *and* using PV ops */ + pv_ops.time.sched_clock = sched_clock; +#endif +#endif +}Please refer to: Documentation/process/coding-style.rst Section 21) [ ... ] Prefer to compile out entire functions, rather than portions of functions or portions of expressions. [ ... ]
OK. I'll rework the #ifdef in v3 of the patch set. Is the following
the preferred approach?
#ifdef CONFIG_GENERIC_SCHED_CLOCK
static __always_inline void hv_setup_sched_clock(void *sched_clock)
{
sched_clock_register(sched_clock, 64 NSEC_PER_SEC);
}
#else
#ifdef CONFIG_PARAVIRT
static __always_inline void hv_setup_sched_clock(void *sched_clock)
{
pv_ops.time.sched_clock = sched_clock:
}
#else
static __always_inline void hv_setup_sched_clock(void *sched_clock) {}
#endif
#endif
Michael