Re: [RFC] Make need_resched() return true when rcu_urgent_qs requested

[Paul E. McKenney]

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.