linux-kernel - Re: [PATCH v2] kvm/x86: Inform RCU of quiescent state when entering guest mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1f63cd8e-1afa-b508-ba66-751c9091fde6@de.ibm.com>
Date:   Thu, 12 Jul 2018 14:04:41 +0200
From:   Christian Borntraeger <borntraeger@...ibm.com>
To:     David Woodhouse <dwmw2@...radead.org>, paulmck@...ux.vnet.ibm.com
Cc:     peterz@...radead.org, mhillenb@...zon.de,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v2] kvm/x86: Inform RCU of quiescent state when entering
 guest mode



On 07/12/2018 01:58 PM, Christian Borntraeger wrote:
> 
> 
> On 07/12/2018 01:10 PM, David Woodhouse wrote:
>> On Thu, 2018-07-12 at 13:00 +0200, Christian Borntraeger wrote:
>>>
>>> On s390 this seems to add about 10ns (~3%) for a guest exit/rentry
>>> microbenchmark mostly due to  rcu_eqs_enter and rcu_eqs_exit now being
>>> visible in perf samples. The older interface was cheaper.
>>
>> Well, the older interface wasn't actually working, which made it
>> moderately suboptimal :)
>>
>> But that is fixed by the first patch of the two, so perhaps the second
>> ought to be conditional on CONFIG_NO_HZ_FULL since it's only really
>> fixing a real bug there?
>>
>> Unless we can contrive some way to do the rcu_eqs_enter/exit only on
>> the paths to/from userspace, and *not* when we loop in the kernel to
>> handle interrupts (etc.) and immediately re-enter the guest without
>> returning? That's somewhat non-trivial though...
> 
> Assuming that patch1 really fixes the issue (and all KVM hot loops are
> supposed to have something like
>         if (need_resched())
>                 schedule();
> 
> this could work out given that patch1 does what it is supposed to do.
> 
> So you are thinking of something like
> 
> diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
> index d8ee9f949ce3..cac5de8a887c 100644
> --- a/include/linux/rcutree.h
> +++ b/include/linux/rcutree.h
> @@ -72,8 +72,17 @@ void cond_synchronize_sched(unsigned long oldstate);
>  
>  void rcu_idle_enter(void);
>  void rcu_idle_exit(void);
> +#ifdef CONFIG_NO_HZ_FULL
>  void rcu_kvm_enter(void);
>  void rcu_kvm_exit(void);
> +#else
> +static inline void rcu_kvm_enter(void)
> +{
> +}
> +static inline void rcu_kvm_exit(void)
> +{
> +}
> +#endif
>  void rcu_irq_enter(void);
>  void rcu_irq_exit(void);
>  void rcu_irq_enter_irqson(void);
> 
> ?
> 

Of course we also need to shift the ifdefs in rcu.c
and the rcu_virt_note_context_switch() on enter as you
said.