[PATCH tip/core/rcu 0/3] Tiny RCU updates for 4.2

[PATCH tip/core/rcu 0/3] Tiny RCU updates for 4.2

[Paul E. McKenney]

Hello!

This series contains updates to Tiny RCU, including a low-probability but
quite real race condition.

1.	Further shrink Tiny RCU by making empty functions static inlines.

2.	Test both RCU-sched and RCU-bh for Tiny RCU, which located the
	race condition fixed by #3 below.

3.	Correctly handle non-empty Tiny RCU callback list with none ready.

							Thanx, Paul

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

 b/include/linux/rcupdate.h                                   |    4 -
 b/include/linux/rcutiny.h                                    |   16 ++++
 b/include/linux/rcutree.h                                    |    5 +
 b/kernel/rcu/tiny.c                                          |   38 +----------
 b/tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot |    1 
 5 files changed, 27 insertions(+), 37 deletions(-)

[PATCH tip/core/rcu 1/3] rcu: Further shrink Tiny RCU by making empty functions static inlines

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

The Tiny RCU counterparts to rcu_idle_enter(), rcu_idle_exit(),
rcu_irq_enter(), and rcu_irq_exit() are empty functions, but each
has EXPORT_SYMBOL_GPL(), which, in kernels built with module support,
needlessly consumes some memory.  This commit therefore moves these
functions to static inlines in rcutiny.h, removing the need for
exports.

This won't affect the size of the tiniest kernels, which are likely
built without module support, but might help semi-tiny kernels that
might include module support.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcupdate.h |  4 ----
 include/linux/rcutiny.h  | 16 ++++++++++++++++
 include/linux/rcutree.h  |  5 +++++
 kernel/rcu/tiny.c        | 33 ---------------------------------
 4 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 87bb0eee665b..1b3d7bcb3a6c 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -292,10 +292,6 @@ void rcu_sched_qs(void);
 void rcu_bh_qs(void);
 void rcu_check_callbacks(int user);
 struct notifier_block;
-void rcu_idle_enter(void);
-void rcu_idle_exit(void);
-void rcu_irq_enter(void);
-void rcu_irq_exit(void);
 int rcu_cpu_notify(struct notifier_block *self,
 		   unsigned long action, void *hcpu);
 
diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index 937edaeb150d..3df6c1ec4e25 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -159,6 +159,22 @@ 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_irq_enter(void)
+{
+}
+
+static inline void rcu_irq_exit(void)
+{
+}
+
 static inline void exit_rcu(void)
 {
 }
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index d2e583a6aaca..f22d83f49e56 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -93,6 +93,11 @@ void rcu_force_quiescent_state(void);
 void rcu_bh_force_quiescent_state(void);
 void rcu_sched_force_quiescent_state(void);
 
+void rcu_idle_enter(void);
+void rcu_idle_exit(void);
+void rcu_irq_enter(void);
+void rcu_irq_exit(void);
+
 void exit_rcu(void);
 
 void rcu_scheduler_starting(void);
diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
index 069742d61c68..a501b4ab9b1c 100644
--- a/kernel/rcu/tiny.c
+++ b/kernel/rcu/tiny.c
@@ -49,39 +49,6 @@ static void __call_rcu(struct rcu_head *head,
 
 #include "tiny_plugin.h"
 
-/*
- * Enter idle, which is an extended quiescent state if we have fully
- * entered that mode.
- */
-void rcu_idle_enter(void)
-{
-}
-EXPORT_SYMBOL_GPL(rcu_idle_enter);
-
-/*
- * Exit an interrupt handler towards idle.
- */
-void rcu_irq_exit(void)
-{
-}
-EXPORT_SYMBOL_GPL(rcu_irq_exit);
-
-/*
- * Exit idle, so that we are no longer in an extended quiescent state.
- */
-void rcu_idle_exit(void)
-{
-}
-EXPORT_SYMBOL_GPL(rcu_idle_exit);
-
-/*
- * Enter an interrupt handler, moving away from idle.
- */
-void rcu_irq_enter(void)
-{
-}
-EXPORT_SYMBOL_GPL(rcu_irq_enter);
-
 #if defined(CONFIG_DEBUG_LOCK_ALLOC) || defined(CONFIG_RCU_TRACE)
 
 /*
-- 
1.8.1.5

[PATCH tip/core/rcu 2/3] rcutorture: Test both RCU-sched and RCU-bh for Tiny RCU

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

Reported-by: "Ahmed, Iftekhar" <ahmedi@onid.oregonstate.edu>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot b/tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot
index 0f0802730014..6c1a292a65fb 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot
+++ b/tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot
@@ -1,2 +1,3 @@
 rcupdate.rcu_self_test=1
 rcupdate.rcu_self_test_bh=1
+rcutorture.torture_type=rcu_bh
-- 
1.8.1.5

[PATCH tip/core/rcu 3/3] rcu: Correctly handle non-empty Tiny RCU callback list with none ready

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

If, at the time __rcu_process_callbacks() is invoked,  there are callbacks
in Tiny RCU's callback list, but none of them are ready to be invoked,
the current list-management code will knit the non-ready callbacks out
of the list.  This can result in hangs and possibly worse.  This commit
therefore inserts a check for there being no callbacks that can be
invoked immediately.

This bug is unlikely to occur -- you have to get a new callback between
the time rcu_sched_qs() or rcu_bh_qs() was called, but before we get to
__rcu_process_callbacks().  It was detected by the addition of RCU-bh
testing to rcutorture, which in turn was instigated by Iftekhar Ahmed's
mutation testing.  Although this bug was made much more likely by
915e8a4fe45e (rcu: Remove fastpath from __rcu_process_callbacks()), this
did not cause the bug, but rather made it much more probable.   That
said, it takes more than 40 hours of rcutorture testing, on average,
for this bug to appear, so this fix cannot be considered an emergency.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: <stable@vger.kernel.org>
---
 kernel/rcu/tiny.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
index a501b4ab9b1c..591af0cb7b9f 100644
--- a/kernel/rcu/tiny.c
+++ b/kernel/rcu/tiny.c
@@ -137,6 +137,11 @@ static void __rcu_process_callbacks(struct rcu_ctrlblk *rcp)
 
 	/* Move the ready-to-invoke callbacks to a local list. */
 	local_irq_save(flags);
+	if (rcp->donetail == &rcp->rcucblist) {
+		/* No callbacks ready, so just leave. */
+		local_irq_restore(flags);
+		return;
+	}
 	RCU_TRACE(trace_rcu_batch_start(rcp->name, 0, rcp->qlen, -1));
 	list = rcp->rcucblist;
 	rcp->rcucblist = *rcp->donetail;
-- 
1.8.1.5

Re: [PATCH tip/core/rcu 1/3] rcu: Further shrink Tiny RCU by making empty functions static inlines

[josh]

On Tue, May 12, 2015 at 03:49:11PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> The Tiny RCU counterparts to rcu_idle_enter(), rcu_idle_exit(),
> rcu_irq_enter(), and rcu_irq_exit() are empty functions, but each
> has EXPORT_SYMBOL_GPL(), which, in kernels built with module support,
> needlessly consumes some memory.  This commit therefore moves these
> functions to static inlines in rcutiny.h, removing the need for
> exports.
> 
> This won't affect the size of the tiniest kernels, which are likely
> built without module support, but might help semi-tiny kernels that
> might include module support.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Even on kernels that don't include module support, this would shave a
few bytes.  Callers can't know a function in another .c file is actually
empty (without LTO, anyway), so callers have to actually *call* these
empty functions, and the functions themselves would need to be emitted.
Turning them into static inlines tells the callers that they can avoid
emitting any code.

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

>  include/linux/rcupdate.h |  4 ----
>  include/linux/rcutiny.h  | 16 ++++++++++++++++
>  include/linux/rcutree.h  |  5 +++++
>  kernel/rcu/tiny.c        | 33 ---------------------------------
>  4 files changed, 21 insertions(+), 37 deletions(-)
> 
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 87bb0eee665b..1b3d7bcb3a6c 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -292,10 +292,6 @@ void rcu_sched_qs(void);
>  void rcu_bh_qs(void);
>  void rcu_check_callbacks(int user);
>  struct notifier_block;
> -void rcu_idle_enter(void);
> -void rcu_idle_exit(void);
> -void rcu_irq_enter(void);
> -void rcu_irq_exit(void);
>  int rcu_cpu_notify(struct notifier_block *self,
>  		   unsigned long action, void *hcpu);
>  
> diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
> index 937edaeb150d..3df6c1ec4e25 100644
> --- a/include/linux/rcutiny.h
> +++ b/include/linux/rcutiny.h
> @@ -159,6 +159,22 @@ 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_irq_enter(void)
> +{
> +}
> +
> +static inline void rcu_irq_exit(void)
> +{
> +}
> +
>  static inline void exit_rcu(void)
>  {
>  }
> diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
> index d2e583a6aaca..f22d83f49e56 100644
> --- a/include/linux/rcutree.h
> +++ b/include/linux/rcutree.h
> @@ -93,6 +93,11 @@ void rcu_force_quiescent_state(void);
>  void rcu_bh_force_quiescent_state(void);
>  void rcu_sched_force_quiescent_state(void);
>  
> +void rcu_idle_enter(void);
> +void rcu_idle_exit(void);
> +void rcu_irq_enter(void);
> +void rcu_irq_exit(void);
> +
>  void exit_rcu(void);
>  
>  void rcu_scheduler_starting(void);
> diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> index 069742d61c68..a501b4ab9b1c 100644
> --- a/kernel/rcu/tiny.c
> +++ b/kernel/rcu/tiny.c
> @@ -49,39 +49,6 @@ static void __call_rcu(struct rcu_head *head,
>  
>  #include "tiny_plugin.h"
>  
> -/*
> - * Enter idle, which is an extended quiescent state if we have fully
> - * entered that mode.
> - */
> -void rcu_idle_enter(void)
> -{
> -}
> -EXPORT_SYMBOL_GPL(rcu_idle_enter);
> -
> -/*
> - * Exit an interrupt handler towards idle.
> - */
> -void rcu_irq_exit(void)
> -{
> -}
> -EXPORT_SYMBOL_GPL(rcu_irq_exit);
> -
> -/*
> - * Exit idle, so that we are no longer in an extended quiescent state.
> - */
> -void rcu_idle_exit(void)
> -{
> -}
> -EXPORT_SYMBOL_GPL(rcu_idle_exit);
> -
> -/*
> - * Enter an interrupt handler, moving away from idle.
> - */
> -void rcu_irq_enter(void)
> -{
> -}
> -EXPORT_SYMBOL_GPL(rcu_irq_enter);
> -
>  #if defined(CONFIG_DEBUG_LOCK_ALLOC) || defined(CONFIG_RCU_TRACE)
>  
>  /*
> -- 
> 1.8.1.5
> 

Re: [PATCH tip/core/rcu 3/3] rcu: Correctly handle non-empty Tiny RCU callback list with none ready

[josh]

On Tue, May 12, 2015 at 03:49:13PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> If, at the time __rcu_process_callbacks() is invoked,  there are callbacks
> in Tiny RCU's callback list, but none of them are ready to be invoked,
> the current list-management code will knit the non-ready callbacks out
> of the list.  This can result in hangs and possibly worse.  This commit
> therefore inserts a check for there being no callbacks that can be
> invoked immediately.
> 
> This bug is unlikely to occur -- you have to get a new callback between
> the time rcu_sched_qs() or rcu_bh_qs() was called, but before we get to
> __rcu_process_callbacks().  It was detected by the addition of RCU-bh
> testing to rcutorture, which in turn was instigated by Iftekhar Ahmed's
> mutation testing.  Although this bug was made much more likely by
> 915e8a4fe45e (rcu: Remove fastpath from __rcu_process_callbacks()), this
> did not cause the bug, but rather made it much more probable.   That
> said, it takes more than 40 hours of rcutorture testing, on average,
> for this bug to appear, so this fix cannot be considered an emergency.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: <stable@vger.kernel.org>

Ouch, subtle.

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

>  kernel/rcu/tiny.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> index a501b4ab9b1c..591af0cb7b9f 100644
> --- a/kernel/rcu/tiny.c
> +++ b/kernel/rcu/tiny.c
> @@ -137,6 +137,11 @@ static void __rcu_process_callbacks(struct rcu_ctrlblk *rcp)
>  
>  	/* Move the ready-to-invoke callbacks to a local list. */
>  	local_irq_save(flags);
> +	if (rcp->donetail == &rcp->rcucblist) {
> +		/* No callbacks ready, so just leave. */
> +		local_irq_restore(flags);
> +		return;
> +	}
>  	RCU_TRACE(trace_rcu_batch_start(rcp->name, 0, rcp->qlen, -1));
>  	list = rcp->rcucblist;
>  	rcp->rcucblist = *rcp->donetail;
> -- 
> 1.8.1.5
> 

Re: [PATCH tip/core/rcu 2/3] rcutorture: Test both RCU-sched and RCU-bh for Tiny RCU

[josh]

On Tue, May 12, 2015 at 03:49:12PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> Reported-by: "Ahmed, Iftekhar" <ahmedi@onid.oregonstate.edu>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Could you elaborate a bit more on this patch (ideally in its commit
message)?  I see an addition of a command-line parameter to test rcu_bh;
is rcu-sched already tested elsewhere by some other config, or does this
parameter somehow enable testing both?

>  tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot b/tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot
> index 0f0802730014..6c1a292a65fb 100644
> --- a/tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot
> +++ b/tools/testing/selftests/rcutorture/configs/rcu/TINY02.boot
> @@ -1,2 +1,3 @@
>  rcupdate.rcu_self_test=1
>  rcupdate.rcu_self_test_bh=1
> +rcutorture.torture_type=rcu_bh
> -- 
> 1.8.1.5
> 

Re: [PATCH tip/core/rcu 1/3] rcu: Further shrink Tiny RCU by making empty functions static inlines

[Paul E. McKenney]

On Tue, May 12, 2015 at 05:57:18PM -0700, josh@joshtriplett.org wrote:
> On Tue, May 12, 2015 at 03:49:11PM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > The Tiny RCU counterparts to rcu_idle_enter(), rcu_idle_exit(),
> > rcu_irq_enter(), and rcu_irq_exit() are empty functions, but each
> > has EXPORT_SYMBOL_GPL(), which, in kernels built with module support,
> > needlessly consumes some memory.  This commit therefore moves these
> > functions to static inlines in rcutiny.h, removing the need for
> > exports.
> > 
> > This won't affect the size of the tiniest kernels, which are likely
> > built without module support, but might help semi-tiny kernels that
> > might include module support.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Even on kernels that don't include module support, this would shave a
> few bytes.  Callers can't know a function in another .c file is actually
> empty (without LTO, anyway), so callers have to actually *call* these
> empty functions, and the functions themselves would need to be emitted.
> Turning them into static inlines tells the callers that they can avoid
> emitting any code.
> 
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>

Thank you, and good point!  The commit log now reads as follows:

	rcu: Further shrink Tiny RCU by making empty functions static inlines

	The Tiny RCU counterparts to rcu_idle_enter(), rcu_idle_exit(),
	rcu_irq_enter(), and rcu_irq_exit() are empty functions, but each
	has EXPORT_SYMBOL_GPL(), which needlessly consumes extra memory,
	especially in kernels built with module support.  This commit
	therefore moves these functions to static inlines in rcutiny.h,
	removing the need for exports.

	This won't affect the size of the tiniest kernels, which are
	likely built without module support, but might help semi-tiny
	kernels that might include module support.

	Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
	Reviewed-by: Josh Triplett <josh@joshtriplett.org>

Is that better?

							Thanx, Paul

> >  include/linux/rcupdate.h |  4 ----
> >  include/linux/rcutiny.h  | 16 ++++++++++++++++
> >  include/linux/rcutree.h  |  5 +++++
> >  kernel/rcu/tiny.c        | 33 ---------------------------------
> >  4 files changed, 21 insertions(+), 37 deletions(-)
> > 
> > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > index 87bb0eee665b..1b3d7bcb3a6c 100644
> > --- a/include/linux/rcupdate.h
> > +++ b/include/linux/rcupdate.h
> > @@ -292,10 +292,6 @@ void rcu_sched_qs(void);
> >  void rcu_bh_qs(void);
> >  void rcu_check_callbacks(int user);
> >  struct notifier_block;
> > -void rcu_idle_enter(void);
> > -void rcu_idle_exit(void);
> > -void rcu_irq_enter(void);
> > -void rcu_irq_exit(void);
> >  int rcu_cpu_notify(struct notifier_block *self,
> >  		   unsigned long action, void *hcpu);
> >  
> > diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
> > index 937edaeb150d..3df6c1ec4e25 100644
> > --- a/include/linux/rcutiny.h
> > +++ b/include/linux/rcutiny.h
> > @@ -159,6 +159,22 @@ 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_irq_enter(void)
> > +{
> > +}
> > +
> > +static inline void rcu_irq_exit(void)
> > +{
> > +}
> > +
> >  static inline void exit_rcu(void)
> >  {
> >  }
> > diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
> > index d2e583a6aaca..f22d83f49e56 100644
> > --- a/include/linux/rcutree.h
> > +++ b/include/linux/rcutree.h
> > @@ -93,6 +93,11 @@ void rcu_force_quiescent_state(void);
> >  void rcu_bh_force_quiescent_state(void);
> >  void rcu_sched_force_quiescent_state(void);
> >  
> > +void rcu_idle_enter(void);
> > +void rcu_idle_exit(void);
> > +void rcu_irq_enter(void);
> > +void rcu_irq_exit(void);
> > +
> >  void exit_rcu(void);
> >  
> >  void rcu_scheduler_starting(void);
> > diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> > index 069742d61c68..a501b4ab9b1c 100644
> > --- a/kernel/rcu/tiny.c
> > +++ b/kernel/rcu/tiny.c
> > @@ -49,39 +49,6 @@ static void __call_rcu(struct rcu_head *head,
> >  
> >  #include "tiny_plugin.h"
> >  
> > -/*
> > - * Enter idle, which is an extended quiescent state if we have fully
> > - * entered that mode.
> > - */
> > -void rcu_idle_enter(void)
> > -{
> > -}
> > -EXPORT_SYMBOL_GPL(rcu_idle_enter);
> > -
> > -/*
> > - * Exit an interrupt handler towards idle.
> > - */
> > -void rcu_irq_exit(void)
> > -{
> > -}
> > -EXPORT_SYMBOL_GPL(rcu_irq_exit);
> > -
> > -/*
> > - * Exit idle, so that we are no longer in an extended quiescent state.
> > - */
> > -void rcu_idle_exit(void)
> > -{
> > -}
> > -EXPORT_SYMBOL_GPL(rcu_idle_exit);
> > -
> > -/*
> > - * Enter an interrupt handler, moving away from idle.
> > - */
> > -void rcu_irq_enter(void)
> > -{
> > -}
> > -EXPORT_SYMBOL_GPL(rcu_irq_enter);
> > -
> >  #if defined(CONFIG_DEBUG_LOCK_ALLOC) || defined(CONFIG_RCU_TRACE)
> >  
> >  /*
> > -- 
> > 1.8.1.5
> > 
> 

Re: [PATCH tip/core/rcu 2/3] rcutorture: Test both RCU-sched and RCU-bh for Tiny RCU

[Steven Rostedt]

On Tue, 12 May 2015 17:59:29 -0700
josh@joshtriplett.org wrote:

> On Tue, May 12, 2015 at 03:49:12PM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > Reported-by: "Ahmed, Iftekhar" <ahmedi@onid.oregonstate.edu>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Could you elaborate a bit more on this patch (ideally in its commit
> message)?  I see an addition of a command-line parameter to test rcu_bh;
> is rcu-sched already tested elsewhere by some other config, or does this
> parameter somehow enable testing both?
> 

Agreed. Subject only commits should only be for typo fixes in comments.

-- Steve

Re: [PATCH tip/core/rcu 2/3] rcutorture: Test both RCU-sched and RCU-bh for Tiny RCU

[Paul E. McKenney]

On Tue, May 12, 2015 at 05:59:29PM -0700, josh@joshtriplett.org wrote:
> On Tue, May 12, 2015 at 03:49:12PM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > Reported-by: "Ahmed, Iftekhar" <ahmedi@onid.oregonstate.edu>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Could you elaborate a bit more on this patch (ideally in its commit
> message)?  I see an addition of a command-line parameter to test rcu_bh;
> is rcu-sched already tested elsewhere by some other config, or does this
> parameter somehow enable testing both?

The commit log now reads as follows, does that help?

							Thanx, Paul

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

rcutorture: Test both RCU-sched and RCU-bh for Tiny RCU

Tiny RCU supports both RCU-sched and RCU-bh, but only RCU-sched is
currently tested by the rcutorture scripts.  This commit therefore
changes the TINY02 configuration to test RCU-bh, with TINY01 continuing
to test RCU-sched.

This shortcoming of the current rcutorture tests was located by mutation
testing by Iftekhar.  The idea behind mutation testing is to automatically
mutate the code under test.  If a given mutant is not caught by testing,
this is a hint that the testing might need to be improved, as was the
case here.  Note that this is only a hint because it is possible to mutate
the code into something else that still works.  For example, a mutation
that removes (say) a WARN_ON() will not normally result in a test failure.

This change resulted in the test failure caused by list mishandling,
which is fixed by the next commit.

Reported-by: "Ahmed, Iftekhar" <ahmedi@onid.oregonstate.edu>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Re: [PATCH tip/core/rcu 3/3] rcu: Correctly handle non-empty Tiny RCU callback list with none ready

[Paul E. McKenney]

On Tue, May 12, 2015 at 05:58:21PM -0700, josh@joshtriplett.org wrote:
> On Tue, May 12, 2015 at 03:49:13PM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > 
> > If, at the time __rcu_process_callbacks() is invoked,  there are callbacks
> > in Tiny RCU's callback list, but none of them are ready to be invoked,
> > the current list-management code will knit the non-ready callbacks out
> > of the list.  This can result in hangs and possibly worse.  This commit
> > therefore inserts a check for there being no callbacks that can be
> > invoked immediately.
> > 
> > This bug is unlikely to occur -- you have to get a new callback between
> > the time rcu_sched_qs() or rcu_bh_qs() was called, but before we get to
> > __rcu_process_callbacks().  It was detected by the addition of RCU-bh
> > testing to rcutorture, which in turn was instigated by Iftekhar Ahmed's
> > mutation testing.  Although this bug was made much more likely by
> > 915e8a4fe45e (rcu: Remove fastpath from __rcu_process_callbacks()), this
> > did not cause the bug, but rather made it much more probable.   That
> > said, it takes more than 40 hours of rcutorture testing, on average,
> > for this bug to appear, so this fix cannot be considered an emergency.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: <stable@vger.kernel.org>
> 
> Ouch, subtle.

Indeed!  A bit of a cautionary tale for those who believe that bugs occur
only in concurrent code.  Of course, they could respond that this bug
was in fact due to a concurrent interrupt handler.  Still, I must confess
that this bug is a bit embarrassing.  ;-)

> Reviewed-by: Josh Triplett <josh@joshtriplett.org>

Thank you, applied!

								Thanx, Paul

> >  kernel/rcu/tiny.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> > index a501b4ab9b1c..591af0cb7b9f 100644
> > --- a/kernel/rcu/tiny.c
> > +++ b/kernel/rcu/tiny.c
> > @@ -137,6 +137,11 @@ static void __rcu_process_callbacks(struct rcu_ctrlblk *rcp)
> >  
> >  	/* Move the ready-to-invoke callbacks to a local list. */
> >  	local_irq_save(flags);
> > +	if (rcp->donetail == &rcp->rcucblist) {
> > +		/* No callbacks ready, so just leave. */
> > +		local_irq_restore(flags);
> > +		return;
> > +	}
> >  	RCU_TRACE(trace_rcu_batch_start(rcp->name, 0, rcp->qlen, -1));
> >  	list = rcp->rcucblist;
> >  	rcp->rcucblist = *rcp->donetail;
> > -- 
> > 1.8.1.5
> > 
> 

Re: [PATCH tip/core/rcu 2/3] rcutorture: Test both RCU-sched and RCU-bh for Tiny RCU

[Josh Triplett]

On Wed, May 13, 2015 at 06:07:28AM -0700, Paul E. McKenney wrote:
> On Tue, May 12, 2015 at 05:59:29PM -0700, josh@joshtriplett.org wrote:
> > On Tue, May 12, 2015 at 03:49:12PM -0700, Paul E. McKenney wrote:
> > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > 
> > > Reported-by: "Ahmed, Iftekhar" <ahmedi@onid.oregonstate.edu>
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > Could you elaborate a bit more on this patch (ideally in its commit
> > message)?  I see an addition of a command-line parameter to test rcu_bh;
> > is rcu-sched already tested elsewhere by some other config, or does this
> > parameter somehow enable testing both?
> 
> The commit log now reads as follows, does that help?
> 
> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> rcutorture: Test both RCU-sched and RCU-bh for Tiny RCU
> 
> Tiny RCU supports both RCU-sched and RCU-bh, but only RCU-sched is
> currently tested by the rcutorture scripts.  This commit therefore
> changes the TINY02 configuration to test RCU-bh, with TINY01 continuing
> to test RCU-sched.
> 
> This shortcoming of the current rcutorture tests was located by mutation
> testing by Iftekhar.  The idea behind mutation testing is to automatically
> mutate the code under test.  If a given mutant is not caught by testing,
> this is a hint that the testing might need to be improved, as was the
> case here.  Note that this is only a hint because it is possible to mutate
> the code into something else that still works.  For example, a mutation
> that removes (say) a WARN_ON() will not normally result in a test failure.
> 
> This change resulted in the test failure caused by list mishandling,
> which is fixed by the next commit.
> 
> Reported-by: "Ahmed, Iftekhar" <ahmedi@onid.oregonstate.edu>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
Much better, thanks.  In particular, the information about TINY01 and
TINY02 was not obvious from the patch.
Reviewed-by: Josh Triplett <josh@joshtriplett.org>