On Wed, Jul 18, 2018 at 06:01:51PM +0200, David Woodhouse wrote:
> On Wed, 2018-07-18 at 08:36 -0700, Paul E. McKenney wrote:
> > And I finally did get some near misses from an earlier commit, so we
> > should consider your patch to be officially off the hook.
>
> Yay, I like it when it's not my fault. I'll redo it with the ifdef
> CONFIG_NO_HZ_FULL.
Hey, I didn't say it wasn't your fault, only that it -officially- wasn't
your fault. ;-)
> What should it do for the !CONFIG_NO_HZ_FULL case? The existing call in
> guest_enter_irqoff() clearly wasn't actually doing the right thing
> anyway, hence the need for the need_resched() patch in $SUBJECT... so
> should I just leave it doing nothing in guest_enter_irqoff()?
One starting point would be the combination of your patch and my
patch, with -rcu commit IDs and diff below. But yes, it needs to be
!CONFIG_NO_HZ_FULL. And no, I am not at all confident that I actually
found all the places needing change in the core code, so this needs some
serious review both by the KVM guys and the NO_HZ_FULL guys.
And some serious testing. But you knew that already. ;-)
Thanx, Paul
------------------------------------------------------------------------
57e3b96d012a kvm/x86: Inform RCU of quiescent state when entering guest mode
f437e330a720 kvm: Inform RCU of quiescent state when entering guest mode
------------------------------------------------------------------------
diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
index d05609ad329d..8d2a9d3073ad 100644
--- a/include/linux/context_tracking.h
+++ b/include/linux/context_tracking.h
@@ -118,12 +118,12 @@ static inline void guest_enter_irqoff(void)
* 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());
+ rcu_kvm_enter();
}
static inline void guest_exit_irqoff(void)
{
+ rcu_kvm_exit();
if (context_tracking_is_enabled())
__context_tracking_exit(CONTEXT_GUEST);
@@ -143,12 +143,13 @@ static inline void guest_enter_irqoff(void)
*/
vtime_account_system(current);
current->flags |= PF_VCPU;
- rcu_virt_note_context_switch(smp_processor_id());
+ rcu_kvm_enter();
}
static inline void guest_exit_irqoff(void)
{
/* Flush the guest cputime we spent on the guest */
+ rcu_kvm_exit();
vtime_account_system(current);
current->flags &= ~PF_VCPU;
}
diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index 7fa4fb9e899e..a7aa5b3cfb81 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -81,10 +81,11 @@ static inline int rcu_needs_cpu(u64 basemono, u64 *nextevt)
* Take advantage of the fact that there is only one CPU, which
* allows us to ignore virtualization-based context switches.
*/
-static inline void rcu_virt_note_context_switch(int cpu) { }
static inline void rcu_cpu_stall_reset(void) { }
static inline void rcu_idle_enter(void) { }
static inline void rcu_idle_exit(void) { }
+static inline void rcu_kvm_enter(void) { }
+static inline void rcu_kvm_exit(void) { }
static inline void rcu_irq_enter(void) { }
static inline void rcu_irq_exit_irqson(void) { }
static inline void rcu_irq_enter_irqson(void) { }
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index 7f83179177d1..62b61e579bb4 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -34,17 +34,6 @@ void rcu_softirq_qs(void);
void rcu_note_context_switch(bool preempt);
int rcu_needs_cpu(u64 basem, u64 *nextevt);
void rcu_cpu_stall_reset(void);
-
-/*
- * Note a virtualization-based context switch. This is simply a
- * wrapper around rcu_note_context_switch(), which allows TINY_RCU
- * to save a few bytes. The caller must have disabled interrupts.
- */
-static inline void rcu_virt_note_context_switch(int cpu)
-{
- rcu_note_context_switch(false);
-}
-
void synchronize_rcu_expedited(void);
void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
@@ -55,6 +44,8 @@ void cond_synchronize_rcu(unsigned long oldstate);
void rcu_idle_enter(void);
void rcu_idle_exit(void);
+void rcu_kvm_enter(void);
+void rcu_kvm_exit(void);
void rcu_irq_enter(void);
void rcu_irq_exit(void);
void rcu_irq_enter_irqson(void);
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 8674ef151d50..cb182b7b0d9a 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -583,6 +583,24 @@ void rcu_idle_enter(void)
rcu_eqs_enter(false);
}
+/**
+ * rcu_kvm_enter - inform RCU that current CPU is entering a guest OS
+ *
+ * Enter guest-OS mode, in other words, -leave- the mode in which RCU
+ * read-side critical sections can occur. (Though RCU read-side critical
+ * sections can occur in irq handlers from guest OSes, a possibility
+ * handled by irq_enter() and irq_exit().)
+ *
+ * If you add or remove a call to rcu_kvm_enter(), be sure to test with
+ * CONFIG_RCU_EQS_DEBUG=y.
+ */
+void rcu_kvm_enter(void)
+{
+ lockdep_assert_irqs_disabled();
+ rcu_eqs_enter(true);
+}
+EXPORT_SYMBOL_GPL(rcu_kvm_enter);
+
#ifdef CONFIG_NO_HZ_FULL
/**
* rcu_user_enter - inform RCU that we are resuming userspace.
@@ -747,6 +765,22 @@ void rcu_idle_exit(void)
local_irq_restore(flags);
}
+/**
+ * rcu_kvm_exit - inform RCU that current CPU is leaving a guest OS
+ *
+ * Exit guest-OS mode, in other words, -enter- the mode in which RCU
+ * read-side critical sections can occur.
+ *
+ * If you add or remove a call to rcu_kvm_exit(), be sure to test with
+ * CONFIG_RCU_EQS_DEBUG=y.
+ */
+void rcu_kvm_exit(void)
+{
+ lockdep_assert_irqs_disabled();
+ rcu_eqs_exit(true);
+}
+EXPORT_SYMBOL_GPL(rcu_kvm_exit);
+
#ifdef CONFIG_NO_HZ_FULL
/**
* rcu_user_exit - inform RCU that we are exiting userspace.
