[PATCH 0/7] rcu: cleanups

[PATCH 0/7] rcu: cleanups

[Lai Jiangshan]

All are minimal independent cleanups, expect that patch 3 depends
on patch 2.

Lai Jiangshan (7):
  rcu: fix incorrect conditional compilation
  rcu: fix tracepoint string when RCU CPU kthread runs
  rcu: trace_rcu_utilization() paired
  rcu: remove the declaration of call_rcu() in tree.h
  rcu: move gp_state_names[] and gp_state_getname() to tree_stall.h
  rcu: rename some CONFIG_PREEMPTION to CONFIG_PREEMPT_RCU
  rcu: splite tasks_rcu to tasks.c

 kernel/rcu/Makefile      |   1 +
 kernel/rcu/rcu.h         |   4 +-
 kernel/rcu/tasks.c       | 395 +++++++++++++++++++++++++++++++++++++++
 kernel/rcu/tree.c        |  19 +-
 kernel/rcu/tree.h        |  15 +-
 kernel/rcu/tree_plugin.h |   1 +
 kernel/rcu/tree_stall.h  |  28 ++-
 kernel/rcu/update.c      | 365 ------------------------------------
 8 files changed, 431 insertions(+), 397 deletions(-)
 create mode 100644 kernel/rcu/tasks.c

-- 
2.20.1

[PATCH 1/7] rcu: fix incorrect conditional compilation

[Lai Jiangshan]

DO NOT pick it to stable tree.
(Since the title has "fix", this statement may help stop
AI pick it to stable tree)

The tokens SRCU and TINY_RCU are not defined by any configurations,
they should be CONFIG_SRCU and CONFIG_TINY_RCU. But there is no
harm when "TINY_RCU" is wrongly used, which are always non-defined,
which makes "!defined(TINY_RCU)" always true, which means
the code block is always inclued, and the included code block
doesn't cause any compilation error so far when CONFIG_TINY_RCU.
It is also the reason this change doesn't need for stable.

Signed-off-by: Lai Jiangshan <jiangshanlai@gmail.com>
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 kernel/rcu/rcu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index a7ab2a023dd3..05f936ed167a 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -254,7 +254,7 @@ void rcu_test_sync_prims(void);
  */
 extern void resched_cpu(int cpu);
 
-#if defined(SRCU) || !defined(TINY_RCU)
+#if defined(CONFIG_SRCU) || !defined(CONFIG_TINY_RCU)
 
 #include <linux/rcu_node_tree.h>
 
@@ -391,7 +391,7 @@ do {									\
 #define raw_lockdep_assert_held_rcu_node(p)				\
 	lockdep_assert_held(&ACCESS_PRIVATE(p, lock))
 
-#endif /* #if defined(SRCU) || !defined(TINY_RCU) */
+#endif /* #if defined(CONFIG_SRCU) || !defined(CONFIG_TINY_RCU) */
 
 #ifdef CONFIG_SRCU
 void srcu_init(void);
-- 
2.20.1

[PATCH 2/7] rcu: fix tracepoint string when RCU CPU kthread runs

[Lai Jiangshan]

"rcu_wait" is incorrct here, use "rcu_run" instead.

Signed-off-by: Lai Jiangshan <jiangshanlai@gmail.com>
Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
---
 kernel/rcu/tree.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 278798e58698..c351fc280945 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2485,7 +2485,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
 	int spincnt;
 
 	for (spincnt = 0; spincnt < 10; spincnt++) {
-		trace_rcu_utilization(TPS("Start CPU kthread@rcu_wait"));
+		trace_rcu_utilization(TPS("Start CPU kthread@rcu_run"));
 		local_bh_disable();
 		*statusp = RCU_KTHREAD_RUNNING;
 		local_irq_disable();
@@ -2496,7 +2496,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
 			rcu_core();
 		local_bh_enable();
 		if (*workp == 0) {
-			trace_rcu_utilization(TPS("End CPU kthread@rcu_wait"));
+			trace_rcu_utilization(TPS("End CPU kthread@rcu_run"));
 			*statusp = RCU_KTHREAD_WAITING;
 			return;
 		}
-- 
2.20.1

Re: [PATCH 1/7] rcu: fix incorrect conditional compilation

[Paul E. McKenney]

On Tue, Oct 15, 2019 at 10:23:56AM +0000, Lai Jiangshan wrote:
> DO NOT pick it to stable tree.
> (Since the title has "fix", this statement may help stop
> AI pick it to stable tree)
> 
> The tokens SRCU and TINY_RCU are not defined by any configurations,
> they should be CONFIG_SRCU and CONFIG_TINY_RCU. But there is no
> harm when "TINY_RCU" is wrongly used, which are always non-defined,
> which makes "!defined(TINY_RCU)" always true, which means
> the code block is always inclued, and the included code block
> doesn't cause any compilation error so far when CONFIG_TINY_RCU.
> It is also the reason this change doesn't need for stable.
> 
> Signed-off-by: Lai Jiangshan <jiangshanlai@gmail.com>
> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>

Applied for review and testing, thank you!

							Thanx, Paul

> ---
>  kernel/rcu/rcu.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
> index a7ab2a023dd3..05f936ed167a 100644
> --- a/kernel/rcu/rcu.h
> +++ b/kernel/rcu/rcu.h
> @@ -254,7 +254,7 @@ void rcu_test_sync_prims(void);
>   */
>  extern void resched_cpu(int cpu);
>  
> -#if defined(SRCU) || !defined(TINY_RCU)
> +#if defined(CONFIG_SRCU) || !defined(CONFIG_TINY_RCU)
>  
>  #include <linux/rcu_node_tree.h>
>  
> @@ -391,7 +391,7 @@ do {									\
>  #define raw_lockdep_assert_held_rcu_node(p)				\
>  	lockdep_assert_held(&ACCESS_PRIVATE(p, lock))
>  
> -#endif /* #if defined(SRCU) || !defined(TINY_RCU) */
> +#endif /* #if defined(CONFIG_SRCU) || !defined(CONFIG_TINY_RCU) */
>  
>  #ifdef CONFIG_SRCU
>  void srcu_init(void);
> -- 
> 2.20.1
> 

Re: [PATCH 2/7] rcu: fix tracepoint string when RCU CPU kthread runs

[Paul E. McKenney]

On Tue, Oct 15, 2019 at 10:23:57AM +0000, Lai Jiangshan wrote:
> "rcu_wait" is incorrct here, use "rcu_run" instead.
> 
> Signed-off-by: Lai Jiangshan <jiangshanlai@gmail.com>
> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
> ---
>  kernel/rcu/tree.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 278798e58698..c351fc280945 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2485,7 +2485,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
>  	int spincnt;
>  
>  	for (spincnt = 0; spincnt < 10; spincnt++) {
> -		trace_rcu_utilization(TPS("Start CPU kthread@rcu_wait"));
> +		trace_rcu_utilization(TPS("Start CPU kthread@rcu_run"));
>  		local_bh_disable();
>  		*statusp = RCU_KTHREAD_RUNNING;
>  		local_irq_disable();
> @@ -2496,7 +2496,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
>  			rcu_core();
>  		local_bh_enable();
>  		if (*workp == 0) {
> -			trace_rcu_utilization(TPS("End CPU kthread@rcu_wait"));
> +			trace_rcu_utilization(TPS("End CPU kthread@rcu_run"));

This one needs to stay as it was because this is where we wait when out
of work.

So I took the first hunk and dropped this second hunk.

Please let me know if I am missing something.

							Thanx, Paul

>  			*statusp = RCU_KTHREAD_WAITING;
>  			return;
>  		}
> -- 
> 2.20.1
> 

Re: [PATCH 2/7] rcu: fix tracepoint string when RCU CPU kthread runs

[Lai Jiangshan]

On 2019/10/16 11:38 上午, Paul E. McKenney wrote:
> On Tue, Oct 15, 2019 at 10:23:57AM +0000, Lai Jiangshan wrote:
>> "rcu_wait" is incorrct here, use "rcu_run" instead.
>>
>> Signed-off-by: Lai Jiangshan <jiangshanlai@gmail.com>
>> Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
>> ---
>>   kernel/rcu/tree.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
>> index 278798e58698..c351fc280945 100644
>> --- a/kernel/rcu/tree.c
>> +++ b/kernel/rcu/tree.c
>> @@ -2485,7 +2485,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
>>   	int spincnt;
>>   
>>   	for (spincnt = 0; spincnt < 10; spincnt++) {
>> -		trace_rcu_utilization(TPS("Start CPU kthread@rcu_wait"));
>> +		trace_rcu_utilization(TPS("Start CPU kthread@rcu_run"));
>>   		local_bh_disable();
>>   		*statusp = RCU_KTHREAD_RUNNING;
>>   		local_irq_disable();
>> @@ -2496,7 +2496,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
>>   			rcu_core();
>>   		local_bh_enable();
>>   		if (*workp == 0) {
>> -			trace_rcu_utilization(TPS("End CPU kthread@rcu_wait"));
>> +			trace_rcu_utilization(TPS("End CPU kthread@rcu_run"));
> 
> This one needs to stay as it was because this is where we wait when out
> of work.

I don't fully understand those TPS marks.

If it is all about "where we wait when out of work", it ought to
be "Start ... wait", rather than "End ... wait". The later one
("End ... wait") should be put before
"for (spincnt = 0; spincnt < 10; spincnt++)" and remove
the whole "rcu_run" as this patch suggested. To be honest,
"rcu_run" is redundant since we already has TPS("Start RCU core").

Any ways, patch2&3 lose their relevance and should be dropped.
Looking forward to your improved version.

Thanks,
Lai

> 
> So I took the first hunk and dropped this second hunk.
> 
> Please let me know if I am missing something.
> 
> 							Thanx, Paul
> 
>>   			*statusp = RCU_KTHREAD_WAITING;
>>   			return;
>>   		}
>> -- 
>> 2.20.1
>>

Re: [PATCH 2/7] rcu: fix tracepoint string when RCU CPU kthread runs

[Paul E. McKenney]

On Wed, Oct 16, 2019 at 12:24:09PM +0800, Lai Jiangshan wrote:
> 
> 
> On 2019/10/16 11:38 上午, Paul E. McKenney wrote:
> > On Tue, Oct 15, 2019 at 10:23:57AM +0000, Lai Jiangshan wrote:
> > > "rcu_wait" is incorrct here, use "rcu_run" instead.
> > > 
> > > Signed-off-by: Lai Jiangshan <jiangshanlai@gmail.com>
> > > Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com>
> > > ---
> > >   kernel/rcu/tree.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > index 278798e58698..c351fc280945 100644
> > > --- a/kernel/rcu/tree.c
> > > +++ b/kernel/rcu/tree.c
> > > @@ -2485,7 +2485,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
> > >   	int spincnt;
> > >   	for (spincnt = 0; spincnt < 10; spincnt++) {
> > > -		trace_rcu_utilization(TPS("Start CPU kthread@rcu_wait"));
> > > +		trace_rcu_utilization(TPS("Start CPU kthread@rcu_run"));
> > >   		local_bh_disable();
> > >   		*statusp = RCU_KTHREAD_RUNNING;
> > >   		local_irq_disable();
> > > @@ -2496,7 +2496,7 @@ static void rcu_cpu_kthread(unsigned int cpu)
> > >   			rcu_core();
> > >   		local_bh_enable();
> > >   		if (*workp == 0) {
> > > -			trace_rcu_utilization(TPS("End CPU kthread@rcu_wait"));
> > > +			trace_rcu_utilization(TPS("End CPU kthread@rcu_run"));
> > 
> > This one needs to stay as it was because this is where we wait when out
> > of work.
> 
> I don't fully understand those TPS marks.
> 
> If it is all about "where we wait when out of work", it ought to
> be "Start ... wait", rather than "End ... wait". The later one
> ("End ... wait") should be put before
> "for (spincnt = 0; spincnt < 10; spincnt++)" and remove
> the whole "rcu_run" as this patch suggested. To be honest,
> "rcu_run" is redundant since we already has TPS("Start RCU core").
> 
> Any ways, patch2&3 lose their relevance and should be dropped.
> Looking forward to your improved version.

Given that most of RCU's overhead is now in kthreads and in RCU_SOFTIRQ,
perhaps trace_rcu_utilization() has outlived its usefulness, especially
given the prospect of an RCU_SOFTIRQ-specific kthread.

							Thanx, Paul