Re: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt quiescent states when disabled
On Mon, Nov 26, 2018 at 01:55:37PM +, Ran Rozenstein wrote: > > > > Hearing no objections, here is the updated patch. > > > > Thanx, Paul > > > > > > > > commit 970cab5d3d206029ed27274a98ea1c3d7e780e53 > > Author: Paul E. McKenney > > Date: Mon Oct 29 07:36:50 2018 -0700 > > > > rcu: Avoid signed integer overflow in rcu_preempt_deferred_qs() > > > > Subtracting INT_MIN can be interpreted as unconditional signed integer > > overflow, which according to the C standard is undefined behavior. > > Therefore, kernel build arguments notwithstanding, it would be good to > > future-proof the code. This commit therefore substitutes INT_MAX for > > INT_MIN in order to avoid undefined behavior. > > > > While in the neighborhood, this commit also creates some meaningful > > names > > for INT_MAX and friends in order to improve readability, as suggested > > by Joel Fernandes. > > > > Reported-by: Ran Rozenstein > > Signed-off-by: Paul E. McKenney > > > > squash! rcu: Avoid signed integer overflow in rcu_preempt_deferred_qs() > > > > While in the neighborhood, use macros to give meaningful names. > > > > Signed-off-by: Paul E. McKenney > > Hi, > > What is the acceptance status of this patch? It is queued in -rcu. If no problems arise beforehand, I intend to submit it as part of a pull request into -tip, which (again if no problems arise) be pulled into mainline during the next merge window. Oddly enough, a couple of weeks ago the C++ Standards Committee voted in a proposal for C++20 removing undefined behavior for signed integer overflow. This is C++ rather than C, and C must support additional hardware that wouldn't much like forcing twos complement for signed integer overflow. But still... ;-) Thanx, Paul
RE: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt quiescent states when disabled
> > Hearing no objections, here is the updated patch. > > Thanx, Paul > > > > commit 970cab5d3d206029ed27274a98ea1c3d7e780e53 > Author: Paul E. McKenney > Date: Mon Oct 29 07:36:50 2018 -0700 > > rcu: Avoid signed integer overflow in rcu_preempt_deferred_qs() > > Subtracting INT_MIN can be interpreted as unconditional signed integer > overflow, which according to the C standard is undefined behavior. > Therefore, kernel build arguments notwithstanding, it would be good to > future-proof the code. This commit therefore substitutes INT_MAX for > INT_MIN in order to avoid undefined behavior. > > While in the neighborhood, this commit also creates some meaningful > names > for INT_MAX and friends in order to improve readability, as suggested > by Joel Fernandes. > > Reported-by: Ran Rozenstein > Signed-off-by: Paul E. McKenney > > squash! rcu: Avoid signed integer overflow in rcu_preempt_deferred_qs() > > While in the neighborhood, use macros to give meaningful names. > > Signed-off-by: Paul E. McKenney > Hi, What is the acceptance status of this patch? Thanks, Ran
Re: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt quiescent states when disabled
On Wed, Oct 31, 2018 at 11:22:59AM -0700, Paul E. McKenney wrote:
> On Tue, Oct 30, 2018 at 03:21:23PM -0700, Joel Fernandes wrote:
> > On Tue, Oct 30, 2018 at 05:58:00AM -0700, Paul E. McKenney wrote:
> > > On Mon, Oct 29, 2018 at 08:44:52PM -0700, Joel Fernandes wrote:
> > > > On Mon, Oct 29, 2018 at 07:27:35AM -0700, Paul E. McKenney wrote:
[ . . . ]
> > > > The INT_MAX naming could be very confusing for nesting levels, could we
> > > > not
> > > > instead just define something like:
> > > > #define RCU_NESTING_MIN (INT_MIN - 1)
> > > > #define RCU_NESTING_MAX (INT_MAX)
> > > >
> > > > and just use that? also one more comment below:
> > >
> > > Hmmm... There is currently no use for RCU_NESTING_MAX, but if the check
> > > at the end of __rcu_read_unlock() were to be extended to check for
> > > too-deep positive nesting, it would need to check for something like
> > > INT_MAX/2. You could of course argue that the current check against
> > > INT_MIN/2 should instead be against -INT_MAX/2, but there really isn't
> > > much difference between the two.
> > >
> > > Another approach would be to convert to unsigned in order to avoid the
> > > overflow problem completely.
> > >
> > > For the moment, anyway, I am inclined to leave it as is.
> >
> > Both the unsigned and INT_MIN/2 options sound good to me, but if you want
> > leave it as is, that would be fine as well. thanks,
>
> One approach would be something like this:
>
> #define RCU_READ_LOCK_BIAS (INT_MAX)
> #define RCU_READ_LOCK_NMAX (-INT_MAX)
> #define RCU_READ_LOCK_PMAX INT_MAX
>
> Then _rcu_read_unlock() would set ->rcu_read_lock_nesting to
> -RCU_READ_LOCK_BIAS, and compare against RCU_READ_LOCK_NMAX.
> The comparison against RCU_READ_LOCK_PMAX would preferably take
> place just after the increment in __rcu_read_lock(), again only under
> CONFIG_PROVE_RCU.
>
> rcu_preempt_deferred_qs() would then subtract then add RCU_READ_LOCK_BIAS.
>
> Thoughts?
Hearing no objections, here is the updated patch.
Thanx, Paul
commit 970cab5d3d206029ed27274a98ea1c3d7e780e53
Author: Paul E. McKenney
Date: Mon Oct 29 07:36:50 2018 -0700
rcu: Avoid signed integer overflow in rcu_preempt_deferred_qs()
Subtracting INT_MIN can be interpreted as unconditional signed integer
overflow, which according to the C standard is undefined behavior.
Therefore, kernel build arguments notwithstanding, it would be good to
future-proof the code. This commit therefore substitutes INT_MAX for
INT_MIN in order to avoid undefined behavior.
While in the neighborhood, this commit also creates some meaningful names
for INT_MAX and friends in order to improve readability, as suggested
by Joel Fernandes.
Reported-by: Ran Rozenstein
Signed-off-by: Paul E. McKenney
squash! rcu: Avoid signed integer overflow in rcu_preempt_deferred_qs()
While in the neighborhood, use macros to give meaningful names.
Signed-off-by: Paul E. McKenney
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index bd8186d0f4a7..e60f820ffb83 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -397,6 +397,11 @@ static int rcu_preempt_blocked_readers_cgp(struct rcu_node
*rnp)
return rnp->gp_tasks != NULL;
}
+/* Bias and limit values for ->rcu_read_lock_nesting. */
+#define RCU_NEST_BIAS INT_MAX
+#define RCU_NEST_NMAX (-INT_MAX / 2)
+#define RCU_NEST_PMAX (INT_MAX / 2)
+
/*
* Preemptible RCU implementation for rcu_read_lock().
* Just increment ->rcu_read_lock_nesting, shared state will be updated
@@ -405,6 +410,8 @@ static int rcu_preempt_blocked_readers_cgp(struct rcu_node
*rnp)
void __rcu_read_lock(void)
{
current->rcu_read_lock_nesting++;
+ if (IS_ENABLED(CONFIG_PROVE_LOCKING))
+ WARN_ON_ONCE(current->rcu_read_lock_nesting > RCU_NEST_PMAX);
barrier(); /* critical section after entry code. */
}
EXPORT_SYMBOL_GPL(__rcu_read_lock);
@@ -424,20 +431,18 @@ void __rcu_read_unlock(void)
--t->rcu_read_lock_nesting;
} else {
barrier(); /* critical section before exit code. */
- t->rcu_read_lock_nesting = INT_MIN;
+ t->rcu_read_lock_nesting = -RCU_NEST_BIAS;
barrier(); /* assign before ->rcu_read_unlock_special load */
if (unlikely(READ_ONCE(t->rcu_read_unlock_special.s)))
rcu_read_unlock_special(t);
barrier(); /* ->rcu_read_unlock_special load before assign */
t->rcu_read_lock_nesting = 0;
}
-#ifdef CONFIG_PROVE_LOCKING
- {
- int rrln = READ_ONCE(t->rcu_read_lock_nesting);
+ if (IS_ENABLED(CONFIG_PROVE_LOCKING)) {
+ int rrln = t->rcu_read_lock_nesting;
- WARN_ON_ONCE(rrln < 0 && rrln >
Re: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt quiescent states when disabled
On Tue, Oct 30, 2018 at 03:21:23PM -0700, Joel Fernandes wrote: > On Tue, Oct 30, 2018 at 05:58:00AM -0700, Paul E. McKenney wrote: > > On Mon, Oct 29, 2018 at 08:44:52PM -0700, Joel Fernandes wrote: > > > On Mon, Oct 29, 2018 at 07:27:35AM -0700, Paul E. McKenney wrote: > > > > On Mon, Oct 29, 2018 at 11:24:42AM +, Ran Rozenstein wrote: > > > > > Hi Paul and all, > > > > > > > > > > > -Original Message- > > > > > > From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel- > > > > > > ow...@vger.kernel.org] On Behalf Of Paul E. McKenney > > > > > > Sent: Thursday, August 30, 2018 01:21 > > > > > > To: linux-kernel@vger.kernel.org > > > > > > Cc: mi...@kernel.org; jiangshan...@gmail.com; dipan...@in.ibm.com; > > > > > > a...@linux-foundation.org; mathieu.desnoy...@efficios.com; > > > > > > j...@joshtriplett.org; t...@linutronix.de; pet...@infradead.org; > > > > > > rost...@goodmis.org; dhowe...@redhat.com; eduma...@google.com; > > > > > > fweis...@gmail.com; o...@redhat.com; j...@joelfernandes.org; Paul E. > > > > > > McKenney > > > > > > Subject: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt > > > > > > quiescent states when disabled > > > > > > > > > > > > This commit defers reporting of RCU-preempt quiescent states at > > > > > > rcu_read_unlock_special() time when any of interrupts, softirq, or > > > > > > preemption are disabled. These deferred quiescent states are > > > > > > reported at a > > > > > > later RCU_SOFTIRQ, context switch, idle entry, or CPU-hotplug > > > > > > offline > > > > > > operation. Of course, if another RCU read-side critical section > > > > > > has started in > > > > > > the meantime, the reporting of the quiescent state will be further > > > > > > deferred. > > > > > > > > > > > > This also means that disabling preemption, interrupts, and/or > > > > > > softirqs will act > > > > > > as an RCU-preempt read-side critical section. > > > > > > This is enforced by checking preempt_count() as needed. > > > > > > > > > > > > Some special cases must be handled on an ad-hoc basis, for example, > > > > > > context switch is a quiescent state even though both the scheduler > > > > > > and > > > > > > do_exit() disable preemption. In these cases, additional calls to > > > > > > rcu_preempt_deferred_qs() override the preemption disabling. > > > > > > Similar logic > > > > > > overrides disabled interrupts in rcu_preempt_check_callbacks() > > > > > > because in > > > > > > this case the quiescent state happened just before the corresponding > > > > > > scheduling-clock interrupt. > > > > > > > > > > > > In theory, this change lifts a long-standing restriction that > > > > > > required that if > > > > > > interrupts were disabled across a call to rcu_read_unlock() that > > > > > > the matching > > > > > > rcu_read_lock() also be contained within that interrupts-disabled > > > > > > region of > > > > > > code. Because the reporting of the corresponding RCU-preempt > > > > > > quiescent > > > > > > state is now deferred until after interrupts have been enabled, it > > > > > > is no longer > > > > > > possible for this situation to result in deadlocks involving the > > > > > > scheduler's > > > > > > runqueue and priority-inheritance locks. This may allow some code > > > > > > simplification that might reduce interrupt latency a bit. > > > > > > Unfortunately, in > > > > > > practice this would also defer deboosting a low-priority task that > > > > > > had been > > > > > > subjected to RCU priority boosting, so real-time-response > > > > > > considerations > > > > > > might well force this restriction to remain in place. > > > > > > > > > > > > Because RCU-preempt grace periods are now blocked not only by RCU > > > > > > read- > > > > > > side critical sections, but also by disabling of interrupts, > > > > > > preemption, and > > > > > > softirqs, it will be possible to eliminate RCU-bh and RCU-sched in > > > > > > favor of > > > > > > RCU-preempt in CONFIG_PREEMPT=y kernels. This may require some > > > > > > additional plumbing to provide the network denial-of-service > > > > > > guarantees > > > > > > that have been traditionally provided by RCU-bh. Once these are in > > > > > > place, > > > > > > CONFIG_PREEMPT=n kernels will be able to fold RCU-bh into RCU-sched. > > > > > > This would mean that all kernels would have but one flavor of RCU, > > > > > > which > > > > > > would open the door to significant code cleanup. > > > > > > > > > > > > Moving to a single flavor of RCU would also have the beneficial > > > > > > effect of > > > > > > reducing the NOCB kthreads by at least a factor of two. > > > > > > > > > > > > Signed-off-by: Paul E. McKenney [ > > > > > > paulmck: > > > > > > Apply rcu_read_unlock_special() preempt_count() feedback > > > > > > from Joel Fernandes. ] > > > > > > [ paulmck: Adjust rcu_eqs_enter() call to rcu_preempt_deferred_qs() > > > > > > in > > > > > > response to bug reports from kbuild
Re: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt quiescent states when disabled
On Tue, Oct 30, 2018 at 05:58:00AM -0700, Paul E. McKenney wrote: > On Mon, Oct 29, 2018 at 08:44:52PM -0700, Joel Fernandes wrote: > > On Mon, Oct 29, 2018 at 07:27:35AM -0700, Paul E. McKenney wrote: > > > On Mon, Oct 29, 2018 at 11:24:42AM +, Ran Rozenstein wrote: > > > > Hi Paul and all, > > > > > > > > > -Original Message- > > > > > From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel- > > > > > ow...@vger.kernel.org] On Behalf Of Paul E. McKenney > > > > > Sent: Thursday, August 30, 2018 01:21 > > > > > To: linux-kernel@vger.kernel.org > > > > > Cc: mi...@kernel.org; jiangshan...@gmail.com; dipan...@in.ibm.com; > > > > > a...@linux-foundation.org; mathieu.desnoy...@efficios.com; > > > > > j...@joshtriplett.org; t...@linutronix.de; pet...@infradead.org; > > > > > rost...@goodmis.org; dhowe...@redhat.com; eduma...@google.com; > > > > > fweis...@gmail.com; o...@redhat.com; j...@joelfernandes.org; Paul E. > > > > > McKenney > > > > > Subject: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt > > > > > quiescent states when disabled > > > > > > > > > > This commit defers reporting of RCU-preempt quiescent states at > > > > > rcu_read_unlock_special() time when any of interrupts, softirq, or > > > > > preemption are disabled. These deferred quiescent states are > > > > > reported at a > > > > > later RCU_SOFTIRQ, context switch, idle entry, or CPU-hotplug offline > > > > > operation. Of course, if another RCU read-side critical section has > > > > > started in > > > > > the meantime, the reporting of the quiescent state will be further > > > > > deferred. > > > > > > > > > > This also means that disabling preemption, interrupts, and/or > > > > > softirqs will act > > > > > as an RCU-preempt read-side critical section. > > > > > This is enforced by checking preempt_count() as needed. > > > > > > > > > > Some special cases must be handled on an ad-hoc basis, for example, > > > > > context switch is a quiescent state even though both the scheduler and > > > > > do_exit() disable preemption. In these cases, additional calls to > > > > > rcu_preempt_deferred_qs() override the preemption disabling. Similar > > > > > logic > > > > > overrides disabled interrupts in rcu_preempt_check_callbacks() > > > > > because in > > > > > this case the quiescent state happened just before the corresponding > > > > > scheduling-clock interrupt. > > > > > > > > > > In theory, this change lifts a long-standing restriction that > > > > > required that if > > > > > interrupts were disabled across a call to rcu_read_unlock() that the > > > > > matching > > > > > rcu_read_lock() also be contained within that interrupts-disabled > > > > > region of > > > > > code. Because the reporting of the corresponding RCU-preempt > > > > > quiescent > > > > > state is now deferred until after interrupts have been enabled, it is > > > > > no longer > > > > > possible for this situation to result in deadlocks involving the > > > > > scheduler's > > > > > runqueue and priority-inheritance locks. This may allow some code > > > > > simplification that might reduce interrupt latency a bit. > > > > > Unfortunately, in > > > > > practice this would also defer deboosting a low-priority task that > > > > > had been > > > > > subjected to RCU priority boosting, so real-time-response > > > > > considerations > > > > > might well force this restriction to remain in place. > > > > > > > > > > Because RCU-preempt grace periods are now blocked not only by RCU > > > > > read- > > > > > side critical sections, but also by disabling of interrupts, > > > > > preemption, and > > > > > softirqs, it will be possible to eliminate RCU-bh and RCU-sched in > > > > > favor of > > > > > RCU-preempt in CONFIG_PREEMPT=y kernels. This may require some > > > > > additional plumbing to provide the network denial-of-service > > > > > guarantees > > > > > that have been traditionally provided by RCU-bh. Once these are in > > > > > place, > > > > > CONFIG_PREEMPT=n kernels will be able to fold RCU-bh into RCU-sched. > > > > > This would mean that all kernels would have but one flavor of RCU, > > > > > which > > > > > would open the door to significant code cleanup. > > > > > > > > > > Moving to a single flavor of RCU would also have the beneficial > > > > > effect of > > > > > reducing the NOCB kthreads by at least a factor of two. > > > > > > > > > > Signed-off-by: Paul E. McKenney [ > > > > > paulmck: > > > > > Apply rcu_read_unlock_special() preempt_count() feedback > > > > > from Joel Fernandes. ] > > > > > [ paulmck: Adjust rcu_eqs_enter() call to rcu_preempt_deferred_qs() in > > > > > response to bug reports from kbuild test robot. ] [ paulmck: Fix > > > > > bug located > > > > > by kbuild test robot involving recursion > > > > > via rcu_preempt_deferred_qs(). ] > > > > > --- > > > > > .../RCU/Design/Requirements/Requirements.html | 50 +++--- > > > > > include/linux/rcutiny.h
Re: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt quiescent states when disabled
On Mon, Oct 29, 2018 at 08:44:52PM -0700, Joel Fernandes wrote: > On Mon, Oct 29, 2018 at 07:27:35AM -0700, Paul E. McKenney wrote: > > On Mon, Oct 29, 2018 at 11:24:42AM +, Ran Rozenstein wrote: > > > Hi Paul and all, > > > > > > > -Original Message- > > > > From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel- > > > > ow...@vger.kernel.org] On Behalf Of Paul E. McKenney > > > > Sent: Thursday, August 30, 2018 01:21 > > > > To: linux-kernel@vger.kernel.org > > > > Cc: mi...@kernel.org; jiangshan...@gmail.com; dipan...@in.ibm.com; > > > > a...@linux-foundation.org; mathieu.desnoy...@efficios.com; > > > > j...@joshtriplett.org; t...@linutronix.de; pet...@infradead.org; > > > > rost...@goodmis.org; dhowe...@redhat.com; eduma...@google.com; > > > > fweis...@gmail.com; o...@redhat.com; j...@joelfernandes.org; Paul E. > > > > McKenney > > > > Subject: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt > > > > quiescent states when disabled > > > > > > > > This commit defers reporting of RCU-preempt quiescent states at > > > > rcu_read_unlock_special() time when any of interrupts, softirq, or > > > > preemption are disabled. These deferred quiescent states are reported > > > > at a > > > > later RCU_SOFTIRQ, context switch, idle entry, or CPU-hotplug offline > > > > operation. Of course, if another RCU read-side critical section has > > > > started in > > > > the meantime, the reporting of the quiescent state will be further > > > > deferred. > > > > > > > > This also means that disabling preemption, interrupts, and/or softirqs > > > > will act > > > > as an RCU-preempt read-side critical section. > > > > This is enforced by checking preempt_count() as needed. > > > > > > > > Some special cases must be handled on an ad-hoc basis, for example, > > > > context switch is a quiescent state even though both the scheduler and > > > > do_exit() disable preemption. In these cases, additional calls to > > > > rcu_preempt_deferred_qs() override the preemption disabling. Similar > > > > logic > > > > overrides disabled interrupts in rcu_preempt_check_callbacks() because > > > > in > > > > this case the quiescent state happened just before the corresponding > > > > scheduling-clock interrupt. > > > > > > > > In theory, this change lifts a long-standing restriction that required > > > > that if > > > > interrupts were disabled across a call to rcu_read_unlock() that the > > > > matching > > > > rcu_read_lock() also be contained within that interrupts-disabled > > > > region of > > > > code. Because the reporting of the corresponding RCU-preempt quiescent > > > > state is now deferred until after interrupts have been enabled, it is > > > > no longer > > > > possible for this situation to result in deadlocks involving the > > > > scheduler's > > > > runqueue and priority-inheritance locks. This may allow some code > > > > simplification that might reduce interrupt latency a bit. > > > > Unfortunately, in > > > > practice this would also defer deboosting a low-priority task that had > > > > been > > > > subjected to RCU priority boosting, so real-time-response considerations > > > > might well force this restriction to remain in place. > > > > > > > > Because RCU-preempt grace periods are now blocked not only by RCU read- > > > > side critical sections, but also by disabling of interrupts, > > > > preemption, and > > > > softirqs, it will be possible to eliminate RCU-bh and RCU-sched in > > > > favor of > > > > RCU-preempt in CONFIG_PREEMPT=y kernels. This may require some > > > > additional plumbing to provide the network denial-of-service guarantees > > > > that have been traditionally provided by RCU-bh. Once these are in > > > > place, > > > > CONFIG_PREEMPT=n kernels will be able to fold RCU-bh into RCU-sched. > > > > This would mean that all kernels would have but one flavor of RCU, which > > > > would open the door to significant code cleanup. > > > > > > > > Moving to a single flavor of RCU would also have the beneficial effect > > > > of > > > > reducing the NOCB kthreads by at least a factor of two. > > > > > > > > Signed-off-by: Paul E. McKenney [ paulmck: > > > > Apply rcu_read_unlock_special() preempt_count() feedback > > > > from Joel Fernandes. ] > > > > [ paulmck: Adjust rcu_eqs_enter() call to rcu_preempt_deferred_qs() in > > > > response to bug reports from kbuild test robot. ] [ paulmck: Fix bug > > > > located > > > > by kbuild test robot involving recursion > > > > via rcu_preempt_deferred_qs(). ] > > > > --- > > > > .../RCU/Design/Requirements/Requirements.html | 50 +++--- > > > > include/linux/rcutiny.h | 5 + > > > > kernel/rcu/tree.c | 9 ++ > > > > kernel/rcu/tree.h | 3 + > > > > kernel/rcu/tree_exp.h | 71 +++-- > > > > kernel/rcu/tree_plugin.h | 144 +- > > > > 6 files changed,
Re: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt quiescent states when disabled
On Mon, Oct 29, 2018 at 07:27:35AM -0700, Paul E. McKenney wrote: > On Mon, Oct 29, 2018 at 11:24:42AM +, Ran Rozenstein wrote: > > Hi Paul and all, > > > > > -Original Message- > > > From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel- > > > ow...@vger.kernel.org] On Behalf Of Paul E. McKenney > > > Sent: Thursday, August 30, 2018 01:21 > > > To: linux-kernel@vger.kernel.org > > > Cc: mi...@kernel.org; jiangshan...@gmail.com; dipan...@in.ibm.com; > > > a...@linux-foundation.org; mathieu.desnoy...@efficios.com; > > > j...@joshtriplett.org; t...@linutronix.de; pet...@infradead.org; > > > rost...@goodmis.org; dhowe...@redhat.com; eduma...@google.com; > > > fweis...@gmail.com; o...@redhat.com; j...@joelfernandes.org; Paul E. > > > McKenney > > > Subject: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt > > > quiescent states when disabled > > > > > > This commit defers reporting of RCU-preempt quiescent states at > > > rcu_read_unlock_special() time when any of interrupts, softirq, or > > > preemption are disabled. These deferred quiescent states are reported at > > > a > > > later RCU_SOFTIRQ, context switch, idle entry, or CPU-hotplug offline > > > operation. Of course, if another RCU read-side critical section has > > > started in > > > the meantime, the reporting of the quiescent state will be further > > > deferred. > > > > > > This also means that disabling preemption, interrupts, and/or softirqs > > > will act > > > as an RCU-preempt read-side critical section. > > > This is enforced by checking preempt_count() as needed. > > > > > > Some special cases must be handled on an ad-hoc basis, for example, > > > context switch is a quiescent state even though both the scheduler and > > > do_exit() disable preemption. In these cases, additional calls to > > > rcu_preempt_deferred_qs() override the preemption disabling. Similar > > > logic > > > overrides disabled interrupts in rcu_preempt_check_callbacks() because in > > > this case the quiescent state happened just before the corresponding > > > scheduling-clock interrupt. > > > > > > In theory, this change lifts a long-standing restriction that required > > > that if > > > interrupts were disabled across a call to rcu_read_unlock() that the > > > matching > > > rcu_read_lock() also be contained within that interrupts-disabled region > > > of > > > code. Because the reporting of the corresponding RCU-preempt quiescent > > > state is now deferred until after interrupts have been enabled, it is no > > > longer > > > possible for this situation to result in deadlocks involving the > > > scheduler's > > > runqueue and priority-inheritance locks. This may allow some code > > > simplification that might reduce interrupt latency a bit. Unfortunately, > > > in > > > practice this would also defer deboosting a low-priority task that had > > > been > > > subjected to RCU priority boosting, so real-time-response considerations > > > might well force this restriction to remain in place. > > > > > > Because RCU-preempt grace periods are now blocked not only by RCU read- > > > side critical sections, but also by disabling of interrupts, preemption, > > > and > > > softirqs, it will be possible to eliminate RCU-bh and RCU-sched in favor > > > of > > > RCU-preempt in CONFIG_PREEMPT=y kernels. This may require some > > > additional plumbing to provide the network denial-of-service guarantees > > > that have been traditionally provided by RCU-bh. Once these are in place, > > > CONFIG_PREEMPT=n kernels will be able to fold RCU-bh into RCU-sched. > > > This would mean that all kernels would have but one flavor of RCU, which > > > would open the door to significant code cleanup. > > > > > > Moving to a single flavor of RCU would also have the beneficial effect of > > > reducing the NOCB kthreads by at least a factor of two. > > > > > > Signed-off-by: Paul E. McKenney [ paulmck: > > > Apply rcu_read_unlock_special() preempt_count() feedback > > > from Joel Fernandes. ] > > > [ paulmck: Adjust rcu_eqs_enter() call to rcu_preempt_deferred_qs() in > > > response to bug reports from kbuild test robot. ] [ paulmck: Fix bug > > > located > > > by kbuild test robot involving recursion > > > via rcu_preempt_deferred_qs(). ] > > > --- > > > .../RCU/Design/Requirements/Requirements.html | 50 +++--- > > > include/linux/rcutiny.h | 5 + > > > kernel/rcu/tree.c | 9 ++ > > > kernel/rcu/tree.h | 3 + > > > kernel/rcu/tree_exp.h | 71 +++-- > > > kernel/rcu/tree_plugin.h | 144 +- > > > 6 files changed, 205 insertions(+), 77 deletions(-) > > > > > > > We started seeing the trace below in our regression system, after I > > bisected I found this is the offending commit. > > This appears immediately on boot. > > Please let me know if you need any additional details. > >
Re: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt quiescent states when disabled
On Mon, Oct 29, 2018 at 11:24:42AM +, Ran Rozenstein wrote:
> Hi Paul and all,
>
> > -Original Message-
> > From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-
> > ow...@vger.kernel.org] On Behalf Of Paul E. McKenney
> > Sent: Thursday, August 30, 2018 01:21
> > To: linux-kernel@vger.kernel.org
> > Cc: mi...@kernel.org; jiangshan...@gmail.com; dipan...@in.ibm.com;
> > a...@linux-foundation.org; mathieu.desnoy...@efficios.com;
> > j...@joshtriplett.org; t...@linutronix.de; pet...@infradead.org;
> > rost...@goodmis.org; dhowe...@redhat.com; eduma...@google.com;
> > fweis...@gmail.com; o...@redhat.com; j...@joelfernandes.org; Paul E.
> > McKenney
> > Subject: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt
> > quiescent states when disabled
> >
> > This commit defers reporting of RCU-preempt quiescent states at
> > rcu_read_unlock_special() time when any of interrupts, softirq, or
> > preemption are disabled. These deferred quiescent states are reported at a
> > later RCU_SOFTIRQ, context switch, idle entry, or CPU-hotplug offline
> > operation. Of course, if another RCU read-side critical section has
> > started in
> > the meantime, the reporting of the quiescent state will be further deferred.
> >
> > This also means that disabling preemption, interrupts, and/or softirqs will
> > act
> > as an RCU-preempt read-side critical section.
> > This is enforced by checking preempt_count() as needed.
> >
> > Some special cases must be handled on an ad-hoc basis, for example,
> > context switch is a quiescent state even though both the scheduler and
> > do_exit() disable preemption. In these cases, additional calls to
> > rcu_preempt_deferred_qs() override the preemption disabling. Similar logic
> > overrides disabled interrupts in rcu_preempt_check_callbacks() because in
> > this case the quiescent state happened just before the corresponding
> > scheduling-clock interrupt.
> >
> > In theory, this change lifts a long-standing restriction that required that
> > if
> > interrupts were disabled across a call to rcu_read_unlock() that the
> > matching
> > rcu_read_lock() also be contained within that interrupts-disabled region of
> > code. Because the reporting of the corresponding RCU-preempt quiescent
> > state is now deferred until after interrupts have been enabled, it is no
> > longer
> > possible for this situation to result in deadlocks involving the scheduler's
> > runqueue and priority-inheritance locks. This may allow some code
> > simplification that might reduce interrupt latency a bit. Unfortunately, in
> > practice this would also defer deboosting a low-priority task that had been
> > subjected to RCU priority boosting, so real-time-response considerations
> > might well force this restriction to remain in place.
> >
> > Because RCU-preempt grace periods are now blocked not only by RCU read-
> > side critical sections, but also by disabling of interrupts, preemption, and
> > softirqs, it will be possible to eliminate RCU-bh and RCU-sched in favor of
> > RCU-preempt in CONFIG_PREEMPT=y kernels. This may require some
> > additional plumbing to provide the network denial-of-service guarantees
> > that have been traditionally provided by RCU-bh. Once these are in place,
> > CONFIG_PREEMPT=n kernels will be able to fold RCU-bh into RCU-sched.
> > This would mean that all kernels would have but one flavor of RCU, which
> > would open the door to significant code cleanup.
> >
> > Moving to a single flavor of RCU would also have the beneficial effect of
> > reducing the NOCB kthreads by at least a factor of two.
> >
> > Signed-off-by: Paul E. McKenney [ paulmck:
> > Apply rcu_read_unlock_special() preempt_count() feedback
> > from Joel Fernandes. ]
> > [ paulmck: Adjust rcu_eqs_enter() call to rcu_preempt_deferred_qs() in
> > response to bug reports from kbuild test robot. ] [ paulmck: Fix bug
> > located
> > by kbuild test robot involving recursion
> > via rcu_preempt_deferred_qs(). ]
> > ---
> > .../RCU/Design/Requirements/Requirements.html | 50 +++---
> > include/linux/rcutiny.h | 5 +
> > kernel/rcu/tree.c | 9 ++
> > kernel/rcu/tree.h | 3 +
> > kernel/rcu/tree_exp.h | 71 +++--
> > kernel/rcu/tree_plugin.h | 144 +-
> > 6 files changed, 205 insertions(+), 77 deletions(-)
> >
>
> We started seeing the trace below in our regression system, after I bisected
> I found this is the offending commit.
> This appears immediately on boot.
> Please let me know if you need any additional details.
Interesting. Here is the offending function:
static void rcu_preempt_deferred_qs(struct task_struct *t)
{
unsigned long flags;
bool couldrecurse = t->rcu_read_lock_nesting >= 0;
if (!rcu_preempt_need_deferred_qs(t))
re
RE: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt quiescent states when disabled
Hi Paul and all, > -Original Message- > From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel- > ow...@vger.kernel.org] On Behalf Of Paul E. McKenney > Sent: Thursday, August 30, 2018 01:21 > To: linux-kernel@vger.kernel.org > Cc: mi...@kernel.org; jiangshan...@gmail.com; dipan...@in.ibm.com; > a...@linux-foundation.org; mathieu.desnoy...@efficios.com; > j...@joshtriplett.org; t...@linutronix.de; pet...@infradead.org; > rost...@goodmis.org; dhowe...@redhat.com; eduma...@google.com; > fweis...@gmail.com; o...@redhat.com; j...@joelfernandes.org; Paul E. > McKenney > Subject: [PATCH tip/core/rcu 02/19] rcu: Defer reporting RCU-preempt > quiescent states when disabled > > This commit defers reporting of RCU-preempt quiescent states at > rcu_read_unlock_special() time when any of interrupts, softirq, or > preemption are disabled. These deferred quiescent states are reported at a > later RCU_SOFTIRQ, context switch, idle entry, or CPU-hotplug offline > operation. Of course, if another RCU read-side critical section has started > in > the meantime, the reporting of the quiescent state will be further deferred. > > This also means that disabling preemption, interrupts, and/or softirqs will > act > as an RCU-preempt read-side critical section. > This is enforced by checking preempt_count() as needed. > > Some special cases must be handled on an ad-hoc basis, for example, > context switch is a quiescent state even though both the scheduler and > do_exit() disable preemption. In these cases, additional calls to > rcu_preempt_deferred_qs() override the preemption disabling. Similar logic > overrides disabled interrupts in rcu_preempt_check_callbacks() because in > this case the quiescent state happened just before the corresponding > scheduling-clock interrupt. > > In theory, this change lifts a long-standing restriction that required that if > interrupts were disabled across a call to rcu_read_unlock() that the matching > rcu_read_lock() also be contained within that interrupts-disabled region of > code. Because the reporting of the corresponding RCU-preempt quiescent > state is now deferred until after interrupts have been enabled, it is no > longer > possible for this situation to result in deadlocks involving the scheduler's > runqueue and priority-inheritance locks. This may allow some code > simplification that might reduce interrupt latency a bit. Unfortunately, in > practice this would also defer deboosting a low-priority task that had been > subjected to RCU priority boosting, so real-time-response considerations > might well force this restriction to remain in place. > > Because RCU-preempt grace periods are now blocked not only by RCU read- > side critical sections, but also by disabling of interrupts, preemption, and > softirqs, it will be possible to eliminate RCU-bh and RCU-sched in favor of > RCU-preempt in CONFIG_PREEMPT=y kernels. This may require some > additional plumbing to provide the network denial-of-service guarantees > that have been traditionally provided by RCU-bh. Once these are in place, > CONFIG_PREEMPT=n kernels will be able to fold RCU-bh into RCU-sched. > This would mean that all kernels would have but one flavor of RCU, which > would open the door to significant code cleanup. > > Moving to a single flavor of RCU would also have the beneficial effect of > reducing the NOCB kthreads by at least a factor of two. > > Signed-off-by: Paul E. McKenney [ paulmck: > Apply rcu_read_unlock_special() preempt_count() feedback > from Joel Fernandes. ] > [ paulmck: Adjust rcu_eqs_enter() call to rcu_preempt_deferred_qs() in > response to bug reports from kbuild test robot. ] [ paulmck: Fix bug located > by kbuild test robot involving recursion > via rcu_preempt_deferred_qs(). ] > --- > .../RCU/Design/Requirements/Requirements.html | 50 +++--- > include/linux/rcutiny.h | 5 + > kernel/rcu/tree.c | 9 ++ > kernel/rcu/tree.h | 3 + > kernel/rcu/tree_exp.h | 71 +++-- > kernel/rcu/tree_plugin.h | 144 +- > 6 files changed, 205 insertions(+), 77 deletions(-) > We started seeing the trace below in our regression system, after I bisected I found this is the offending commit. This appears immediately on boot. Please let me know if you need any additional details. Thanks, Ran [2018-10-27 05:53:07] [2018-10-27 05:53:07] UBSAN: Undefined behaviour in kernel/rcu/tree_plugin.h:620:28 [2018-10-27 05:53:07] signed integer overflow: [2018-10-27 05:53:07] 0 - -2147483648 cannot be represented in type 'int' [2018-10-27 05:53:07] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.19.0-for-upstream-dbg-2018-10-25_03-10-39-82 #1 [2018-10-27 05:53:07] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014 [2018-1
