Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1530846 > unrolled thread

Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and `mem_cgroup_shrink_node`

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Donald Buczek <buczek@molgen.mpg.de> - 2016-11-27 10:20 +0100
    Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Michal Hocko <mhocko@kernel.org> - 2016-11-28 12:10 +0100
      Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Paul Menzel <pmenzel@molgen.mpg.de> - 2016-11-28 13:30 +0100
        Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Donald Buczek <buczek@molgen.mpg.de> - 2016-11-30 11:30 +0100
          Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Michal Hocko <mhocko@kernel.org> - 2016-11-30 12:20 +0100
            Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Donald Buczek <buczek@molgen.mpg.de> - 2016-11-30 12:50 +0100
              Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Donald Buczek <buczek@molgen.mpg.de> - 2016-12-02 10:20 +0100
                Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Donald Buczek <buczek@molgen.mpg.de> - 2016-12-06 09:40 +0100
            Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-11-30 13:00 +0100
              Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Paul Menzel <pmenzel@molgen.mpg.de> - 2016-11-30 13:40 +0100
                Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-11-30 15:40 +0100
            Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-11-30 13:00 +0100
              Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Michal Hocko <mhocko@kernel.org> - 2016-11-30 14:20 +0100
                Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-11-30 15:40 +0100
                  Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Peter Zijlstra <peterz@infradead.org> - 2016-11-30 17:40 +0100
                    Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Michal Hocko <mhocko@kernel.org> - 2016-11-30 18:10 +0100
                      Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-11-30 18:30 +0100
                        Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Michal Hocko <mhocko@kernel.org> - 2016-11-30 18:40 +0100
                      Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Peter Zijlstra <peterz@infradead.org> - 2016-11-30 19:00 +0100
                        Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-11-30 22:40 +0100
                          Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Peter Zijlstra <peterz@infradead.org> - 2016-12-01 06:40 +0100
                            Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-12-01 13:50 +0100
                              Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Peter Zijlstra <peterz@infradead.org> - 2016-12-01 17:40 +0100
                                Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-12-01 18:00 +0100
                                  Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Peter Zijlstra <peterz@infradead.org> - 2016-12-01 19:20 +0100
                                    Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-12-01 19:50 +0100
                                      Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` Peter Zijlstra <peterz@infradead.org> - 2016-12-01 20:00 +0100
                    Re: INFO: rcu_sched detected stalls on CPUs/tasks with `kswapd` and  `mem_cgroup_shrink_node` "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-11-30 18:10 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1533817

On Wed, Nov 30, 2016 at 11:40:19AM -0800, Paul E. McKenney wrote:

> > See commit:
> > 
> >   4a81e8328d37 ("rcu: Reduce overhead of cond_resched() checks for RCU")
> > 
> > Someone actually wrote down what the problem was.
> 
> Don't worry, it won't happen again.  ;-)
> 
> OK, so the regressions were in the "open1" test of Anton Blanchard's
> "will it scale" suite, and were due to faster (and thus more) grace
> periods rather than path length.
> 
> I could likely counter the grace-period speedup by regulating the rate
> at which the grace-period machinery pays attention to the rcu_qs_ctr
> per-CPU variable.  Actually, this looks pretty straightforward (famous
> last words).  But see patch below, which is untested and probably
> completely bogus.

Possible I suppose. Didn't look too hard at it.

> > > > Also, I seem to have missed, why are we going through this again?
> > > 
> > > Well, the point I've brought that up is because having basically two
> > > APIs for cond_resched is more than confusing. Basically all longer in
> > > kernel loops do cond_resched() but it seems that this will not help the
> > > silence RCU lockup detector in rare cases where nothing really wants to
> > > schedule. I am really not sure whether we want to sprinkle
> > > cond_resched_rcu_qs at random places just to silence RCU detector...
> > 
> > Right.. now, this is obviously all PREEMPT=n code, which therefore also
> > implies this is rcu-sched.
> > 
> > Paul, now doesn't rcu-sched, when the grace-period has been long in
> > coming, try and force it? And doesn't that forcing include prodding CPUs
> > with resched_cpu() ?
> 
> It does in the v4.8.4 kernel that Boris is running.  It still does in my
> -rcu tree, but only after an RCU CPU stall (something about people not
> liking IPIs).  I may need to do a resched_cpu() halfway to stall-warning
> time or some such.

Sure, we all dislike IPIs, but I'm thinking this half-way point is
sensible, no point in issuing user visible annoyance if indeed we can
prod things back to life, no?

Only if we utterly fail to make it respond should we bug the user with
our failure..

> > I'm thinking not, because if it did, that would make cond_resched()
> > actually schedule, which would then call into rcu_note_context_switch()
> > which would then make RCU progress, no?
> 
> Sounds plausible, but from what I can see some of the loops pointed
> out by Boris's stall-warning messages don't have cond_resched().
> There was another workload that apparently worked better when moved from
> cond_resched() to cond_resched_rcu_qs(), but I don't know what kernel
> version was running.

Egads.. cursed if you do, cursed if you dont eh..

[toc] | [prev] | [next] | [standalone]

#1534064

On Thu, Dec 01, 2016 at 06:30:35AM +0100, Peter Zijlstra wrote:
> On Wed, Nov 30, 2016 at 11:40:19AM -0800, Paul E. McKenney wrote:
> 
> > > See commit:
> > > 
> > >   4a81e8328d37 ("rcu: Reduce overhead of cond_resched() checks for RCU")
> > > 
> > > Someone actually wrote down what the problem was.
> > 
> > Don't worry, it won't happen again.  ;-)
> > 
> > OK, so the regressions were in the "open1" test of Anton Blanchard's
> > "will it scale" suite, and were due to faster (and thus more) grace
> > periods rather than path length.
> > 
> > I could likely counter the grace-period speedup by regulating the rate
> > at which the grace-period machinery pays attention to the rcu_qs_ctr
> > per-CPU variable.  Actually, this looks pretty straightforward (famous
> > last words).  But see patch below, which is untested and probably
> > completely bogus.
> 
> Possible I suppose. Didn't look too hard at it.
> 
> > > > > Also, I seem to have missed, why are we going through this again?
> > > > 
> > > > Well, the point I've brought that up is because having basically two
> > > > APIs for cond_resched is more than confusing. Basically all longer in
> > > > kernel loops do cond_resched() but it seems that this will not help the
> > > > silence RCU lockup detector in rare cases where nothing really wants to
> > > > schedule. I am really not sure whether we want to sprinkle
> > > > cond_resched_rcu_qs at random places just to silence RCU detector...
> > > 
> > > Right.. now, this is obviously all PREEMPT=n code, which therefore also
> > > implies this is rcu-sched.
> > > 
> > > Paul, now doesn't rcu-sched, when the grace-period has been long in
> > > coming, try and force it? And doesn't that forcing include prodding CPUs
> > > with resched_cpu() ?
> > 
> > It does in the v4.8.4 kernel that Boris is running.  It still does in my
> > -rcu tree, but only after an RCU CPU stall (something about people not
> > liking IPIs).  I may need to do a resched_cpu() halfway to stall-warning
> > time or some such.
> 
> Sure, we all dislike IPIs, but I'm thinking this half-way point is
> sensible, no point in issuing user visible annoyance if indeed we can
> prod things back to life, no?
> 
> Only if we utterly fail to make it respond should we bug the user with
> our failure..

Sold!  ;-)

I will put together a patch later today.

My intent is to hold off on the "upgrade cond_resched()" patch, one
step at a time.  Longer term, I do very much like the idea of having
cond_resched() do both scheduling and RCU quiescent states, assuming
that this avoids performance pitfalls.

> > > I'm thinking not, because if it did, that would make cond_resched()
> > > actually schedule, which would then call into rcu_note_context_switch()
> > > which would then make RCU progress, no?
> > 
> > Sounds plausible, but from what I can see some of the loops pointed
> > out by Boris's stall-warning messages don't have cond_resched().
> > There was another workload that apparently worked better when moved from
> > cond_resched() to cond_resched_rcu_qs(), but I don't know what kernel
> > version was running.
> 
> Egads.. cursed if you do, cursed if you dont eh..

Almost like this was real life!  ;-)

							Thanx, Paul

[toc] | [prev] | [next] | [standalone]

#1534276

On Thu, Dec 01, 2016 at 04:40:24AM -0800, Paul E. McKenney wrote:
> On Thu, Dec 01, 2016 at 06:30:35AM +0100, Peter Zijlstra wrote:

> > Sure, we all dislike IPIs, but I'm thinking this half-way point is
> > sensible, no point in issuing user visible annoyance if indeed we can
> > prod things back to life, no?
> > 
> > Only if we utterly fail to make it respond should we bug the user with
> > our failure..
> 
> Sold!  ;-)
> 
> I will put together a patch later today.
> 
> My intent is to hold off on the "upgrade cond_resched()" patch, one
> step at a time.  Longer term, I do very much like the idea of having
> cond_resched() do both scheduling and RCU quiescent states, assuming
> that this avoids performance pitfalls.

Well, with the above change cond_resched() is already sufficient, no?

In fact, by doing the IPI thing we get the entire cond_resched*()
family, and we could add the should_resched() guard to
cond_resched_rcu().

[toc] | [prev] | [next] | [standalone]

#1534286

On Thu, Dec 01, 2016 at 05:36:14PM +0100, Peter Zijlstra wrote:
> On Thu, Dec 01, 2016 at 04:40:24AM -0800, Paul E. McKenney wrote:
> > On Thu, Dec 01, 2016 at 06:30:35AM +0100, Peter Zijlstra wrote:
> 
> > > Sure, we all dislike IPIs, but I'm thinking this half-way point is
> > > sensible, no point in issuing user visible annoyance if indeed we can
> > > prod things back to life, no?
> > > 
> > > Only if we utterly fail to make it respond should we bug the user with
> > > our failure..
> > 
> > Sold!  ;-)
> > 
> > I will put together a patch later today.
> > 
> > My intent is to hold off on the "upgrade cond_resched()" patch, one
> > step at a time.  Longer term, I do very much like the idea of having
> > cond_resched() do both scheduling and RCU quiescent states, assuming
> > that this avoids performance pitfalls.
> 
> Well, with the above change cond_resched() is already sufficient, no?

Maybe.  Right now, cond_resched_rcu_qs() gets a quiescent state to
the RCU core in less than one jiffy, with my other change, this becomes
a handful of jiffies depending on HZ and NR_CPUS.  I expect this
increase to a handful of jiffies to be a non-event.

After my upcoming patch, cond_resched() will get a quiescent state to
the RCU core in about ten seconds.  While I am am not all that nervous
about the increase from less than a jiffy to a handful of jiffies,
increasing to ten seconds via cond_resched() does make me quite nervous.
Past experience indicates that someone's kernel will likely be fatally
inconvenienced by this magnitude of change.

Or am I misunderstanding what you are proposing?

> In fact, by doing the IPI thing we get the entire cond_resched*()
> family, and we could add the should_resched() guard to
> cond_resched_rcu().

So that cond_resched_rcu_qs() looks something like this, in order
to avoid the function call in the case where the scheduler has nothing
to do?

#define cond_resched_rcu_qs() \
do { \
	if (!should_resched(current) || !cond_resched()) \
		rcu_note_voluntary_context_switch(current); \
} while (0)

							Thanx, Paul

[toc] | [prev] | [next] | [standalone]

#1534371

On Thu, Dec 01, 2016 at 08:59:18AM -0800, Paul E. McKenney wrote:
> On Thu, Dec 01, 2016 at 05:36:14PM +0100, Peter Zijlstra wrote:
> > Well, with the above change cond_resched() is already sufficient, no?
> 
> Maybe.  Right now, cond_resched_rcu_qs() gets a quiescent state to
> the RCU core in less than one jiffy, with my other change, this becomes
> a handful of jiffies depending on HZ and NR_CPUS.  I expect this
> increase to a handful of jiffies to be a non-event.
> 
> After my upcoming patch, cond_resched() will get a quiescent state to
> the RCU core in about ten seconds.  While I am am not all that nervous
> about the increase from less than a jiffy to a handful of jiffies,
> increasing to ten seconds via cond_resched() does make me quite nervous.
> Past experience indicates that someone's kernel will likely be fatally
> inconvenienced by this magnitude of change.
> 
> Or am I misunderstanding what you are proposing?

No, that is indeed what I was proposing. Hurm.. OK let me ponder that a
bit. There might be a few games we can play with !PREEMPT to avoid IPIs.

Thing is, I'm slightly uncomfortable with de-coupling rcu-sched from
actual schedule() calls.

> > In fact, by doing the IPI thing we get the entire cond_resched*()
> > family, and we could add the should_resched() guard to
> > cond_resched_rcu().
> 
> So that cond_resched_rcu_qs() looks something like this, in order
> to avoid the function call in the case where the scheduler has nothing
> to do?

I was actually thinking of this:

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 2d0c82e1d348..2dc7d8056b2a 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -3374,9 +3374,11 @@ static inline int signal_pending_state(long state, struct task_struct *p)
 static inline void cond_resched_rcu(void)
 {
 #if defined(CONFIG_DEBUG_ATOMIC_SLEEP) || !defined(CONFIG_PREEMPT_RCU)
-	rcu_read_unlock();
-	cond_resched();
-	rcu_read_lock();
+	if (should_resched(1)) {
+		rcu_read_unlock();
+		cond_resched();
+		rcu_read_lock();
+	}
 #endif
 }
 

[toc] | [prev] | [next] | [standalone]

#1534381

On Thu, Dec 01, 2016 at 07:09:53PM +0100, Peter Zijlstra wrote:
> On Thu, Dec 01, 2016 at 08:59:18AM -0800, Paul E. McKenney wrote:
> > On Thu, Dec 01, 2016 at 05:36:14PM +0100, Peter Zijlstra wrote:
> > > Well, with the above change cond_resched() is already sufficient, no?
> > 
> > Maybe.  Right now, cond_resched_rcu_qs() gets a quiescent state to
> > the RCU core in less than one jiffy, with my other change, this becomes
> > a handful of jiffies depending on HZ and NR_CPUS.  I expect this
> > increase to a handful of jiffies to be a non-event.
> > 
> > After my upcoming patch, cond_resched() will get a quiescent state to
> > the RCU core in about ten seconds.  While I am am not all that nervous
> > about the increase from less than a jiffy to a handful of jiffies,
> > increasing to ten seconds via cond_resched() does make me quite nervous.
> > Past experience indicates that someone's kernel will likely be fatally
> > inconvenienced by this magnitude of change.
> > 
> > Or am I misunderstanding what you are proposing?
> 
> No, that is indeed what I was proposing. Hurm.. OK let me ponder that a
> bit. There might be a few games we can play with !PREEMPT to avoid IPIs.
> 
> Thing is, I'm slightly uncomfortable with de-coupling rcu-sched from
> actual schedule() calls.

OK, what is the source of your discomfort?

There are several intermediate levels of evasive action:

0.	If there is another runnable task and certain other conditions
	are met, cond_resched() will invoke schedule(), which will
	provide an RCU quiescent state.

1.	All cond_resched_rcu_qs() invocations increment the CPU's
	rcu_qs_ctr per-CPU variable, which is treated by later
	invocations of RCU core as a quiescent state.  (I have
	a patch queued that causes RCU to ignore changes to this
	counter until the grace period is a few jiffies old.)

	In this case, the rcu_node locks plus smp_mb__after_unlock_lock()
	provide the needed ordering.

2.	If any cond_resched_rcu_qs() sees that an expedited grace
	period is waiting on the current CPU, it invokes rcu_sched_qs()
	to force RCU to see the quiescent state.  (To your point,
	rcu_sched_qs() is normally called from schedule(), but also
	from the scheduling-clock interrupt when it interrupts
	usermode or idle.)

	Again, the rcu_node locks plus smp_mb__after_unlock_lock()
	provide the needed ordering.

3.	If the grace period extends for more than 50 milliseconds
	(by default, tunable), all subsequent cond_resched_rcu_qs()
	invocations on that CPU turn into momentary periods of
	idleness from RCU's viewpoint.  (Atomically add 2 to the
	dyntick-idle counter.)

	Here, the atomic increment is surrounded by smp_mb__*_atomic()
	to provide the needed ordering, which should be a good substitute
	for actually passing through schedule().

4.	If the grace period extends for more than 21 seconds (by default),
	we emit an RCU CPU stall warning and then do a resched_cpu().
	I am proposing also doing a resched_cpu() halfway to RCU CPU
	stall-warning time.

5.	An RCU-sched expedited grace period does a local resched_cpu()
	from its IPI handler to force the CPU through a quiescent
	state.  (Yes, I could just invoke resched_cpu() from the
	task orchestrating the expedited grace period, but this approach
	allows more common code between RCU-preempt and RCU-sched
	expedited grace periods.)

> > > In fact, by doing the IPI thing we get the entire cond_resched*()
> > > family, and we could add the should_resched() guard to
> > > cond_resched_rcu().
> > 
> > So that cond_resched_rcu_qs() looks something like this, in order
> > to avoid the function call in the case where the scheduler has nothing
> > to do?
> 
> I was actually thinking of this:

Oh!  I had forgotten about cond_resched_rcu(), and thought you did a typo.

Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 2d0c82e1d348..2dc7d8056b2a 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -3374,9 +3374,11 @@ static inline int signal_pending_state(long state, struct task_struct *p)
>  static inline void cond_resched_rcu(void)
>  {
>  #if defined(CONFIG_DEBUG_ATOMIC_SLEEP) || !defined(CONFIG_PREEMPT_RCU)
> -	rcu_read_unlock();
> -	cond_resched();
> -	rcu_read_lock();
> +	if (should_resched(1)) {
> +		rcu_read_unlock();
> +		cond_resched();
> +		rcu_read_lock();
> +	}
>  #endif
>  }
> 
> 

[toc] | [prev] | [next] | [standalone]

#1534383

On Thu, Dec 01, 2016 at 10:42:52AM -0800, Paul E. McKenney wrote:
> On Thu, Dec 01, 2016 at 07:09:53PM +0100, Peter Zijlstra wrote:
> > Thing is, I'm slightly uncomfortable with de-coupling rcu-sched from
> > actual schedule() calls.
> 
> OK, what is the source of your discomfort?

Good question; after a little thought its not much different from other
cases. So let me ponder this a bit more..

[toc] | [prev] | [next] | [standalone]

#1533449

On Wed, Nov 30, 2016 at 05:38:20PM +0100, Peter Zijlstra wrote:
> On Wed, Nov 30, 2016 at 06:29:55AM -0800, Paul E. McKenney wrote:
> > We can, and you are correct that cond_resched() does not unconditionally
> > supply RCU quiescent states, and never has.  Last time I tried to add
> > cond_resched_rcu_qs() semantics to cond_resched(), I got told "no",
> > but perhaps it is time to try again.
> 
> Well, you got told: "ARRGH my benchmark goes all regress", or something
> along those lines. Didn't we recently dig out those commits for some
> reason or other?

Were "those commits" the benchmark or putting cond_resched_rcu_qs()
functionality into cond_resched()?  Either way, no idea.

> Finding out what benchmark that was and running it against this patch
> would make sense.

Agreed, especially given that I believe cond_resched_rcu_qs() is lighter
weight than it used to be.  No idea what benchmarks they were, though.

> Also, I seem to have missed, why are we going through this again?

People are running workloads that force long-running loops in the kernel,
which get them RCU CPU stall warning messages.  My reaction has been
to insert cond_resched_rcu_qs() as needed, and Michal wondered why
cond_resched() couldn't just handle both scheduling latency and RCU
quiescent states.  I remembered trying it, but not what the issue was.

So I posted the patch assuming that I would eventually either find out
what the issue was or that the issue no longer applied.  ;-)

							Thanx, Paul

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web