Re: [PATCH] KVM guest: prevent tracing recursion with kvmclock

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 2011-11-15 at 15:01 +0200, Avi Kivity wrote:
> Stop tracing when we read the clock, since tracing will also
> want to read the clock, and recurse indefinitely.

I would rephrase the above. You don't actually stop tracing, you just
don't trace the preempt disable. I would reword that to something like:

Prevent tracing of preempt_disable() in get_cpu_var() in
kvm_clock_read(). When CONFIG_DEBUG_PREEMPT is enabled,
preempt_disable/enable() are traced and this causes the function_graph
tracer to go into an infinite recursion. By open coding the
preempt_disable() around the get_cpu_var(), we can use the notrace
version which prevents preempt_disable/enable() from being traced and
prevents the recursion.

Something like the above.

> 
> Based on a similar patch for Xen from Jeremy Fitzhardinge.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>

This was exactly my first thought, but I was thinking it may be better
to have a get_cpu_var_notrace() than to have to open code this stuff.
Maybe there's not that many users that open code is not an issue. I'll
still want to add that recursion protection with the warn on though.

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

> ---
>  arch/x86/kernel/kvmclock.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index c1a0188..44842d7 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -74,9 +74,10 @@ static cycle_t kvm_clock_read(void)
>  	struct pvclock_vcpu_time_info *src;
>  	cycle_t ret;
>  
> -	src = &get_cpu_var(hv_clock);
> +	preempt_disable_notrace();
> +	src = &__get_cpu_var(hv_clock);
>  	ret = pvclock_clocksource_read(src);
> -	put_cpu_var(hv_clock);
> +	preempt_enable_notrace();
>  	return ret;
>  }
>