* Paolo Bonzini <pbonz...@redhat.com> wrote:
>
>
> On 08/06/2016 14:16, Ingo Molnar wrote:
> > > The guest ones are not quite as consistent. I can fix that later,
> > > there's no reason also to have guest context tracking split between
> > > include/linux/context_tracking.h and include/linux/kvm_host.h.
> >
> > Could we please first do the cleanups before complicating the code and
> > applying
> > more substantial changes?
>
> The further cleanups wouldn't complicate the code. It's just that
> guest_enter/guest_exit require IRQs off but don't have __.
>
> I'm thinking of something like this (untested):
>
> diff --git a/include/linux/context_tracking.h
> b/include/linux/context_tracking.h
> index d259274238db..c2dc581ddb0e 100644
> --- a/include/linux/context_tracking.h
> +++ b/include/linux/context_tracking.h
> @@ -84,7 +84,7 @@ static inline void context_tracking_init(void) { }
>
>
> #ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN
> -static inline void guest_enter(void)
> +static inline void __guest_enter(void)
> {
> if (vtime_accounting_cpu_enabled())
> vtime_guest_enter(current);
> @@ -93,9 +93,19 @@ static inline void guest_enter(void)
>
> if (context_tracking_is_enabled())
> __context_tracking_enter(CONTEXT_GUEST);
> +
> + /* KVM does not hold any references to rcu protected data when it
> + * switches CPU into a guest mode. In fact switching to a guest mode
Nit, please use the customary (multi-line) comment style:
/*
* Comment .....
* ...... goes here.
*/
> + * is very similar to exiting to userspace from rcu point of view. In
s/RCU
> + * addition CPU may stay in a guest mode for quite a long time (up to
> + * one time slice). Lets treat guest mode as quiescent state, just like
> + * we do with user-mode execution.
> + */
> + if (!context_tracking_cpu_is_enabled())
> + rcu_virt_note_context_switch(smp_processor_id());
> }
>
> -static inline void guest_exit(void)
> +static inline void __guest_exit(void)
> +static inline void guest_enter(void)
> +{
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + __guest_enter();
> + local_irq_restore(flags);
So I believe it would be cleaner to name the irqs-off code paths explicitly:
__guest_enter_irqsoff(), and propagate that naming into other parts as well?
> /* must be called with irqs disabled */
> static inline void __kvm_guest_exit(void)
This way all these random comments about irqs-off requirements would become
unnecessary - the code becomes self-documenting.
Thanks,
Ingo
