[PATCHv2 1/2] rcu/tree: handle VM stoppage in stall detection

[PATCHv2 1/2] rcu/tree: handle VM stoppage in stall detection

[Sergey Senozhatsky]

Soft watchdog timer function checks if a virtual machine
was suspended and hence what looks like a lockup in fact
is a false positive.

This is what kvm_check_and_clear_guest_paused() does: it
tests guest PVCLOCK_GUEST_STOPPED (which is set by the host)
and if it's set then we need to touch all watchdogs and bail
out.

Watchdog timer function runs from IRQ, so PVCLOCK_GUEST_STOPPED
check works fine.

There is, however, one more watchdog that runs from IRQ, so
watchdog timer fn races with it, and that watchdog is not aware
of PVCLOCK_GUEST_STOPPED - RCU stall detector.

apic_timer_interrupt()
 smp_apic_timer_interrupt()
  hrtimer_interrupt()
   __hrtimer_run_queues()
    tick_sched_timer()
     tick_sched_handle()
      update_process_times()
       rcu_sched_clock_irq()

This triggers RCU stalls on our devices during VM resume.

If tick_sched_handle()->rcu_sched_clock_irq() runs on a VCPU
before watchdog_timer_fn()->kvm_check_and_clear_guest_paused()
then there is nothing on this VCPU that touches watchdogs and
RCU reads stale gp stall timestamp and new jiffies value, which
makes it think that RCU has stalled.

Make RCU stall watchdog aware of PVCLOCK_GUEST_STOPPED and
don't report RCU stalls when we resume the VM.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---

v2: fixed powerpc build breakage

 kernel/rcu/tree_stall.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
index d574e3bbd929..bc689911a81d 100644
--- a/kernel/rcu/tree_stall.h
+++ b/kernel/rcu/tree_stall.h
@@ -7,6 +7,8 @@
  * Author: Paul E. McKenney <paulmck@linux.ibm.com>
  */
 
+#include <linux/kvm_para.h>
+
 //////////////////////////////////////////////////////////////////////////////
 //
 // Controlling CPU stall warnings, including delay calculation.
@@ -698,6 +700,14 @@ static void check_cpu_stall(struct rcu_data *rdp)
 	    (READ_ONCE(rnp->qsmask) & rdp->grpmask) &&
 	    cmpxchg(&rcu_state.jiffies_stall, js, jn) == js) {
 
+		/*
+		 * If a virtual machine is stopped by the host it can look to
+		 * the watchdog like an RCU stall. Check to see if the host
+		 * stopped the vm.
+		 */
+		if (kvm_check_and_clear_guest_paused())
+			return;
+
 		/* We haven't checked in, so go dump stack. */
 		print_cpu_stall(gps);
 		if (READ_ONCE(rcu_cpu_stall_ftrace_dump))
@@ -707,6 +717,14 @@ static void check_cpu_stall(struct rcu_data *rdp)
 		   ULONG_CMP_GE(j, js + RCU_STALL_RAT_DELAY) &&
 		   cmpxchg(&rcu_state.jiffies_stall, js, jn) == js) {
 
+		/*
+		 * If a virtual machine is stopped by the host it can look to
+		 * the watchdog like an RCU stall. Check to see if the host
+		 * stopped the vm.
+		 */
+		if (kvm_check_and_clear_guest_paused())
+			return;
+
 		/* They had a few time units to dump stack, so complain. */
 		print_other_cpu_stall(gs2, gps);
 		if (READ_ONCE(rcu_cpu_stall_ftrace_dump))
-- 
2.31.1.818.g46aad6cb9e-goog

[PATCH 2/2] rcu: do not disable gp stall detection in rcu_cpu_stall_reset()

[Sergey Senozhatsky]

rcu_cpu_stall_reset() is one of the functions virtual CPUs
execute during VM resume in order to handle jiffies skew
that can trigger false positive stall warnings. Paul has
pointed out that this approach is problematic because
rcu_cpu_stall_reset() disables RCU grace period stall-detection
virtually forever, while in fact it can just restart the
stall-detection timeout.

Suggested-by: "Paul E. McKenney" <paulmck@kernel.org>
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 kernel/rcu/tree_stall.h | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
index bc689911a81d..b9b52f91e5b8 100644
--- a/kernel/rcu/tree_stall.h
+++ b/kernel/rcu/tree_stall.h
@@ -119,17 +119,14 @@ static void panic_on_rcu_stall(void)
 }
 
 /**
- * rcu_cpu_stall_reset - prevent further stall warnings in current grace period
- *
- * Set the stall-warning timeout way off into the future, thus preventing
- * any RCU CPU stall-warning messages from appearing in the current set of
- * RCU grace periods.
+ * rcu_cpu_stall_reset - restart stall-warning timeout for current grace period
  *
  * The caller must disable hard irqs.
  */
 void rcu_cpu_stall_reset(void)
 {
-	WRITE_ONCE(rcu_state.jiffies_stall, jiffies + ULONG_MAX / 2);
+	WRITE_ONCE(rcu_state.jiffies_stall,
+		   jiffies + rcu_jiffies_till_stall_check());
 }
 
 //////////////////////////////////////////////////////////////////////////////
-- 
2.31.1.818.g46aad6cb9e-goog

Re: [PATCHv2 1/2] rcu/tree: handle VM stoppage in stall detection

[Paul E. McKenney]

On Sat, May 22, 2021 at 12:56:23AM +0900, Sergey Senozhatsky wrote:
> Soft watchdog timer function checks if a virtual machine
> was suspended and hence what looks like a lockup in fact
> is a false positive.
> 
> This is what kvm_check_and_clear_guest_paused() does: it
> tests guest PVCLOCK_GUEST_STOPPED (which is set by the host)
> and if it's set then we need to touch all watchdogs and bail
> out.
> 
> Watchdog timer function runs from IRQ, so PVCLOCK_GUEST_STOPPED
> check works fine.
> 
> There is, however, one more watchdog that runs from IRQ, so
> watchdog timer fn races with it, and that watchdog is not aware
> of PVCLOCK_GUEST_STOPPED - RCU stall detector.
> 
> apic_timer_interrupt()
>  smp_apic_timer_interrupt()
>   hrtimer_interrupt()
>    __hrtimer_run_queues()
>     tick_sched_timer()
>      tick_sched_handle()
>       update_process_times()
>        rcu_sched_clock_irq()
> 
> This triggers RCU stalls on our devices during VM resume.
> 
> If tick_sched_handle()->rcu_sched_clock_irq() runs on a VCPU
> before watchdog_timer_fn()->kvm_check_and_clear_guest_paused()
> then there is nothing on this VCPU that touches watchdogs and
> RCU reads stale gp stall timestamp and new jiffies value, which
> makes it think that RCU has stalled.
> 
> Make RCU stall watchdog aware of PVCLOCK_GUEST_STOPPED and
> don't report RCU stalls when we resume the VM.
> 
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>

I have queued both for testing and further review, thank you!

							Thanx, Paul

> ---
> 
> v2: fixed powerpc build breakage
> 
>  kernel/rcu/tree_stall.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
> index d574e3bbd929..bc689911a81d 100644
> --- a/kernel/rcu/tree_stall.h
> +++ b/kernel/rcu/tree_stall.h
> @@ -7,6 +7,8 @@
>   * Author: Paul E. McKenney <paulmck@linux.ibm.com>
>   */
>  
> +#include <linux/kvm_para.h>
> +
>  //////////////////////////////////////////////////////////////////////////////
>  //
>  // Controlling CPU stall warnings, including delay calculation.
> @@ -698,6 +700,14 @@ static void check_cpu_stall(struct rcu_data *rdp)
>  	    (READ_ONCE(rnp->qsmask) & rdp->grpmask) &&
>  	    cmpxchg(&rcu_state.jiffies_stall, js, jn) == js) {
>  
> +		/*
> +		 * If a virtual machine is stopped by the host it can look to
> +		 * the watchdog like an RCU stall. Check to see if the host
> +		 * stopped the vm.
> +		 */
> +		if (kvm_check_and_clear_guest_paused())
> +			return;
> +
>  		/* We haven't checked in, so go dump stack. */
>  		print_cpu_stall(gps);
>  		if (READ_ONCE(rcu_cpu_stall_ftrace_dump))
> @@ -707,6 +717,14 @@ static void check_cpu_stall(struct rcu_data *rdp)
>  		   ULONG_CMP_GE(j, js + RCU_STALL_RAT_DELAY) &&
>  		   cmpxchg(&rcu_state.jiffies_stall, js, jn) == js) {
>  
> +		/*
> +		 * If a virtual machine is stopped by the host it can look to
> +		 * the watchdog like an RCU stall. Check to see if the host
> +		 * stopped the vm.
> +		 */
> +		if (kvm_check_and_clear_guest_paused())
> +			return;
> +
>  		/* They had a few time units to dump stack, so complain. */
>  		print_other_cpu_stall(gs2, gps);
>  		if (READ_ONCE(rcu_cpu_stall_ftrace_dump))
> -- 
> 2.31.1.818.g46aad6cb9e-goog
> 

Re: [PATCHv2 1/2] rcu/tree: handle VM stoppage in stall detection

[Paul E. McKenney]

On Fri, May 21, 2021 at 11:01:27AM -0700, Paul E. McKenney wrote:
> On Sat, May 22, 2021 at 12:56:23AM +0900, Sergey Senozhatsky wrote:
> > Soft watchdog timer function checks if a virtual machine
> > was suspended and hence what looks like a lockup in fact
> > is a false positive.
> > 
> > This is what kvm_check_and_clear_guest_paused() does: it
> > tests guest PVCLOCK_GUEST_STOPPED (which is set by the host)
> > and if it's set then we need to touch all watchdogs and bail
> > out.
> > 
> > Watchdog timer function runs from IRQ, so PVCLOCK_GUEST_STOPPED
> > check works fine.
> > 
> > There is, however, one more watchdog that runs from IRQ, so
> > watchdog timer fn races with it, and that watchdog is not aware
> > of PVCLOCK_GUEST_STOPPED - RCU stall detector.
> > 
> > apic_timer_interrupt()
> >  smp_apic_timer_interrupt()
> >   hrtimer_interrupt()
> >    __hrtimer_run_queues()
> >     tick_sched_timer()
> >      tick_sched_handle()
> >       update_process_times()
> >        rcu_sched_clock_irq()
> > 
> > This triggers RCU stalls on our devices during VM resume.
> > 
> > If tick_sched_handle()->rcu_sched_clock_irq() runs on a VCPU
> > before watchdog_timer_fn()->kvm_check_and_clear_guest_paused()
> > then there is nothing on this VCPU that touches watchdogs and
> > RCU reads stale gp stall timestamp and new jiffies value, which
> > makes it think that RCU has stalled.
> > 
> > Make RCU stall watchdog aware of PVCLOCK_GUEST_STOPPED and
> > don't report RCU stalls when we resume the VM.
> > 
> > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> 
> I have queued both for testing and further review, thank you!

And on that otherwise inexplicable refetch of the jiffies counter within
check_cpu_stall(), the commit below makes it more effective.

If check_cpu_stall() is delayed before or while printing the stall
warning, we really want to wait the full time duration between the
end of that stall warning and the start of the next one.  Of course,
if there is some way to learn whether printk() is overloaded, even more
effective approaches could be taken.

Thoughts?

							Thanx, Paul

------------------------------------------------------------------------

commit b9c5dc2856c1538ccf2d09246df2b58bede72cca
Author: Paul E. McKenney <paulmck@kernel.org>
Date:   Fri May 21 14:23:03 2021 -0700

    rcu: Start timing stall repetitions after warning complete
    
    Systems with low-bandwidth consoles can have very large printk()
    latencies, and on such systems it makes no sense to have the next RCU CPU
    stall warning message start output before the prior message completed.
    This commit therefore sets the time of the next stall only after the
    prints have completed.  While printing, the time of the next stall
    message is set to ULONG_MAX/2 jiffies into the future.
    
    Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
index 05012a8081a1..ff239189a627 100644
--- a/kernel/rcu/tree_stall.h
+++ b/kernel/rcu/tree_stall.h
@@ -648,6 +648,7 @@ static void print_cpu_stall(unsigned long gps)
 
 static void check_cpu_stall(struct rcu_data *rdp)
 {
+	bool didstall = false;
 	unsigned long gs1;
 	unsigned long gs2;
 	unsigned long gps;
@@ -693,7 +694,7 @@ static void check_cpu_stall(struct rcu_data *rdp)
 	    ULONG_CMP_GE(gps, js))
 		return; /* No stall or GP completed since entering function. */
 	rnp = rdp->mynode;
-	jn = jiffies + 3 * rcu_jiffies_till_stall_check() + 3;
+	jn = jiffies + ULONG_MAX / 2;
 	if (rcu_gp_in_progress() &&
 	    (READ_ONCE(rnp->qsmask) & rdp->grpmask) &&
 	    cmpxchg(&rcu_state.jiffies_stall, js, jn) == js) {
@@ -710,6 +711,7 @@ static void check_cpu_stall(struct rcu_data *rdp)
 		print_cpu_stall(gps);
 		if (READ_ONCE(rcu_cpu_stall_ftrace_dump))
 			rcu_ftrace_dump(DUMP_ALL);
+		didstall = true;
 
 	} else if (rcu_gp_in_progress() &&
 		   ULONG_CMP_GE(j, js + RCU_STALL_RAT_DELAY) &&
@@ -727,6 +729,11 @@ static void check_cpu_stall(struct rcu_data *rdp)
 		print_other_cpu_stall(gs2, gps);
 		if (READ_ONCE(rcu_cpu_stall_ftrace_dump))
 			rcu_ftrace_dump(DUMP_ALL);
+		didstall = true;
+	}
+	if (didstall && READ_ONCE(rcu_state.jiffies_stall) == jn) {
+		jn = jiffies + 3 * rcu_jiffies_till_stall_check() + 3;
+		WRITE_ONCE(rcu_state.jiffies_stall, jn);
 	}
 }
 

Re: [PATCHv2 1/2] rcu/tree: handle VM stoppage in stall detection

[Sergey Senozhatsky]

On (21/05/21 14:38), Paul E. McKenney wrote:
> 
> And on that otherwise inexplicable refetch of the jiffies counter within
> check_cpu_stall(), the commit below makes it more effective.
> 
> If check_cpu_stall() is delayed before or while printing the stall
> warning, we really want to wait the full time duration between the
> end of that stall warning and the start of the next one.
>

Nice improvement!

> Of course, if there is some way to learn whether printk() is overloaded,
> even more effective approaches could be taken.

There is no better to do this.

> commit b9c5dc2856c1538ccf2d09246df2b58bede72cca
> Author: Paul E. McKenney <paulmck@kernel.org>
> Date:   Fri May 21 14:23:03 2021 -0700
> 
>     rcu: Start timing stall repetitions after warning complete
>     
>     Systems with low-bandwidth consoles can have very large printk()
>     latencies, and on such systems it makes no sense to have the next RCU CPU
>     stall warning message start output before the prior message completed.
>     This commit therefore sets the time of the next stall only after the
>     prints have completed.  While printing, the time of the next stall
>     message is set to ULONG_MAX/2 jiffies into the future.
>     
>     Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

FWIW,

Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>

> diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
> index 05012a8081a1..ff239189a627 100644
> --- a/kernel/rcu/tree_stall.h
> +++ b/kernel/rcu/tree_stall.h
> @@ -648,6 +648,7 @@ static void print_cpu_stall(unsigned long gps)
>  
>  static void check_cpu_stall(struct rcu_data *rdp)
>  {
> +	bool didstall = false;
>  	unsigned long gs1;
>  	unsigned long gs2;
>  	unsigned long gps;
> @@ -693,7 +694,7 @@ static void check_cpu_stall(struct rcu_data *rdp)
>  	    ULONG_CMP_GE(gps, js))
>  		return; /* No stall or GP completed since entering function. */
>  	rnp = rdp->mynode;
> -	jn = jiffies + 3 * rcu_jiffies_till_stall_check() + 3;
> +	jn = jiffies + ULONG_MAX / 2;
>  	if (rcu_gp_in_progress() &&
>  	    (READ_ONCE(rnp->qsmask) & rdp->grpmask) &&
>  	    cmpxchg(&rcu_state.jiffies_stall, js, jn) == js) {
> @@ -710,6 +711,7 @@ static void check_cpu_stall(struct rcu_data *rdp)
>  		print_cpu_stall(gps);
>  		if (READ_ONCE(rcu_cpu_stall_ftrace_dump))
>  			rcu_ftrace_dump(DUMP_ALL);
> +		didstall = true;
>  
>  	} else if (rcu_gp_in_progress() &&
>  		   ULONG_CMP_GE(j, js + RCU_STALL_RAT_DELAY) &&
> @@ -727,6 +729,11 @@ static void check_cpu_stall(struct rcu_data *rdp)
>  		print_other_cpu_stall(gs2, gps);
>  		if (READ_ONCE(rcu_cpu_stall_ftrace_dump))
>  			rcu_ftrace_dump(DUMP_ALL);
> +		didstall = true;
> +	}
> +	if (didstall && READ_ONCE(rcu_state.jiffies_stall) == jn) {

Can `rcu_state.jiffies_stall` change here?

> +		jn = jiffies + 3 * rcu_jiffies_till_stall_check() + 3;
> +		WRITE_ONCE(rcu_state.jiffies_stall, jn);
>  	}
>  }

Re: [PATCHv2 1/2] rcu/tree: handle VM stoppage in stall detection

[Paul E. McKenney]

On Mon, May 24, 2021 at 10:56:44AM +0900, Sergey Senozhatsky wrote:
> On (21/05/21 14:38), Paul E. McKenney wrote:
> > 
> > And on that otherwise inexplicable refetch of the jiffies counter within
> > check_cpu_stall(), the commit below makes it more effective.
> > 
> > If check_cpu_stall() is delayed before or while printing the stall
> > warning, we really want to wait the full time duration between the
> > end of that stall warning and the start of the next one.
> >
> 
> Nice improvement!

Thank you, glad you like it!

> > Of course, if there is some way to learn whether printk() is overloaded,
> > even more effective approaches could be taken.
> 
> There is no better to do this.

I was afraid of that.  ;-)

> > commit b9c5dc2856c1538ccf2d09246df2b58bede72cca
> > Author: Paul E. McKenney <paulmck@kernel.org>
> > Date:   Fri May 21 14:23:03 2021 -0700
> > 
> >     rcu: Start timing stall repetitions after warning complete
> >     
> >     Systems with low-bandwidth consoles can have very large printk()
> >     latencies, and on such systems it makes no sense to have the next RCU CPU
> >     stall warning message start output before the prior message completed.
> >     This commit therefore sets the time of the next stall only after the
> >     prints have completed.  While printing, the time of the next stall
> >     message is set to ULONG_MAX/2 jiffies into the future.
> >     
> >     Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> 
> FWIW,
> 
> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>

Thank you again, I will apply this on my next rebase.

> > diff --git a/kernel/rcu/tree_stall.h b/kernel/rcu/tree_stall.h
> > index 05012a8081a1..ff239189a627 100644
> > --- a/kernel/rcu/tree_stall.h
> > +++ b/kernel/rcu/tree_stall.h
> > @@ -648,6 +648,7 @@ static void print_cpu_stall(unsigned long gps)
> >  
> >  static void check_cpu_stall(struct rcu_data *rdp)
> >  {
> > +	bool didstall = false;
> >  	unsigned long gs1;
> >  	unsigned long gs2;
> >  	unsigned long gps;
> > @@ -693,7 +694,7 @@ static void check_cpu_stall(struct rcu_data *rdp)
> >  	    ULONG_CMP_GE(gps, js))
> >  		return; /* No stall or GP completed since entering function. */
> >  	rnp = rdp->mynode;
> > -	jn = jiffies + 3 * rcu_jiffies_till_stall_check() + 3;
> > +	jn = jiffies + ULONG_MAX / 2;
> >  	if (rcu_gp_in_progress() &&
> >  	    (READ_ONCE(rnp->qsmask) & rdp->grpmask) &&
> >  	    cmpxchg(&rcu_state.jiffies_stall, js, jn) == js) {
> > @@ -710,6 +711,7 @@ static void check_cpu_stall(struct rcu_data *rdp)
> >  		print_cpu_stall(gps);
> >  		if (READ_ONCE(rcu_cpu_stall_ftrace_dump))
> >  			rcu_ftrace_dump(DUMP_ALL);
> > +		didstall = true;
> >  
> >  	} else if (rcu_gp_in_progress() &&
> >  		   ULONG_CMP_GE(j, js + RCU_STALL_RAT_DELAY) &&
> > @@ -727,6 +729,11 @@ static void check_cpu_stall(struct rcu_data *rdp)
> >  		print_other_cpu_stall(gs2, gps);
> >  		if (READ_ONCE(rcu_cpu_stall_ftrace_dump))
> >  			rcu_ftrace_dump(DUMP_ALL);
> > +		didstall = true;
> > +	}
> > +	if (didstall && READ_ONCE(rcu_state.jiffies_stall) == jn) {
> 
> Can `rcu_state.jiffies_stall` change here?

In theory, yes, sort of, anyway.  In practice, highly unlikely.
The most plausible way for this to happen is for this code path to be
delayed for a long time on a 32-bit system, so that jiffies+ULONG_MAX/2
actually arrives.  But in that case, all sorts of other complaints
would happen first.

But I could make this a cmpxchg(), if that is what you are getting at.

							Thanx, Paul

> > +		jn = jiffies + 3 * rcu_jiffies_till_stall_check() + 3;
> > +		WRITE_ONCE(rcu_state.jiffies_stall, jn);
> >  	}
> >  }

Re: [PATCHv2 1/2] rcu/tree: handle VM stoppage in stall detection

[Sergey Senozhatsky]

On (21/05/23 20:46), Paul E. McKenney wrote:
> 
> In theory, yes, sort of, anyway.  In practice, highly unlikely.
> The most plausible way for this to happen is for this code path to be
> delayed for a long time on a 32-bit system, so that jiffies+ULONG_MAX/2
> actually arrives.  But in that case, all sorts of other complaints
> would happen first.

I see.

> But I could make this a cmpxchg(), if that is what you are getting at.

No, it's good. I was just curious what scenario I was missing.

Re: [PATCHv2 1/2] rcu/tree: handle VM stoppage in stall detection

[Sergey Senozhatsky]

On (21/05/22 00:56), Sergey Senozhatsky wrote:
> Soft watchdog timer function checks if a virtual machine
> was suspended and hence what looks like a lockup in fact
> is a false positive.
> 
> This is what kvm_check_and_clear_guest_paused() does: it
> tests guest PVCLOCK_GUEST_STOPPED (which is set by the host)
> and if it's set then we need to touch all watchdogs and bail
> out.
> 
> Watchdog timer function runs from IRQ, so PVCLOCK_GUEST_STOPPED
> check works fine.
> 
> There is, however, one more watchdog that runs from IRQ, so
> watchdog timer fn races with it, and that watchdog is not aware
> of PVCLOCK_GUEST_STOPPED - RCU stall detector.
> 
> apic_timer_interrupt()
>  smp_apic_timer_interrupt()
>   hrtimer_interrupt()
>    __hrtimer_run_queues()
>     tick_sched_timer()
>      tick_sched_handle()
>       update_process_times()
>        rcu_sched_clock_irq()
> 
> This triggers RCU stalls on our devices during VM resume.
> 
> If tick_sched_handle()->rcu_sched_clock_irq() runs on a VCPU
> before watchdog_timer_fn()->kvm_check_and_clear_guest_paused()
> then there is nothing on this VCPU that touches watchdogs and
> RCU reads stale gp stall timestamp and new jiffies value, which
> makes it think that RCU has stalled.
> 
> Make RCU stall watchdog aware of PVCLOCK_GUEST_STOPPED and
> don't report RCU stalls when we resume the VM.

Hello Paul,

I've noticed that this patch set didn't make it to Linus's tree.
Was it intentional?

Re: [PATCHv2 1/2] rcu/tree: handle VM stoppage in stall detection

[Paul E. McKenney]

On Thu, Jul 15, 2021 at 06:09:45PM +0900, Sergey Senozhatsky wrote:
> On (21/05/22 00:56), Sergey Senozhatsky wrote:
> > Soft watchdog timer function checks if a virtual machine
> > was suspended and hence what looks like a lockup in fact
> > is a false positive.
> > 
> > This is what kvm_check_and_clear_guest_paused() does: it
> > tests guest PVCLOCK_GUEST_STOPPED (which is set by the host)
> > and if it's set then we need to touch all watchdogs and bail
> > out.
> > 
> > Watchdog timer function runs from IRQ, so PVCLOCK_GUEST_STOPPED
> > check works fine.
> > 
> > There is, however, one more watchdog that runs from IRQ, so
> > watchdog timer fn races with it, and that watchdog is not aware
> > of PVCLOCK_GUEST_STOPPED - RCU stall detector.
> > 
> > apic_timer_interrupt()
> >  smp_apic_timer_interrupt()
> >   hrtimer_interrupt()
> >    __hrtimer_run_queues()
> >     tick_sched_timer()
> >      tick_sched_handle()
> >       update_process_times()
> >        rcu_sched_clock_irq()
> > 
> > This triggers RCU stalls on our devices during VM resume.
> > 
> > If tick_sched_handle()->rcu_sched_clock_irq() runs on a VCPU
> > before watchdog_timer_fn()->kvm_check_and_clear_guest_paused()
> > then there is nothing on this VCPU that touches watchdogs and
> > RCU reads stale gp stall timestamp and new jiffies value, which
> > makes it think that RCU has stalled.
> > 
> > Make RCU stall watchdog aware of PVCLOCK_GUEST_STOPPED and
> > don't report RCU stalls when we resume the VM.
> 
> Hello Paul,
> 
> I've noticed that this patch set didn't make it to Linus's tree.
> Was it intentional?

This patch (and the 18 preceding it) didn't make the cutoff for the
just-past merge window.  If this patch is urgent, please let me know
and I can push it, with luck by the end of next week.

If that one is urgent, are these two also?

817690fd18af ("rcu: Do not disable GP stall detection in rcu_cpu_stall_reset()")
9ed9bf0d17cd ("rcu: Start timing stall repetitions after warning complete")

If so, it is better to handle them as a group than separately.

The cutoff for a given merge window is normally shortly after the close
of the previous merge window.  This time, I am a bit slow creating
branches, but the cutoff for the v5.15 merge window should be by the
end of the week.  This is a bit more lag than most subsystems, but
this is after all RCU.

As always, if a given commit is urgent, please let me know and I
will see what I can do to fast-track it.

For reference:

https://mirrors.edge.kernel.org/pub/linux/kernel/people/paulmck/rcutodo.html

Again, if this one needs to hit mainline before the v5.15 merge
window, please let me know.

							Thanx, Paul

Re: [PATCHv2 1/2] rcu/tree: handle VM stoppage in stall detection

[Sergey Senozhatsky]

On (21/07/15 06:32), Paul E. McKenney wrote:
> > 
> > Hello Paul,
> > 
> > I've noticed that this patch set didn't make it to Linus's tree.
> > Was it intentional?
> 
> This patch (and the 18 preceding it) didn't make the cutoff for the
> just-past merge window.

Oh, I see.

> If this patch is urgent, please let me know
> and I can push it, with luck by the end of next week.
> 
> If that one is urgent, are these two also?

No, Paul, nothing is urgent.

Many thanks for the detailed clarifications.