Re: [PATCH 07/10] rcu: Separate the RCU synchronization types and APIs into <linux/rcupdate_wait.h>

From: Paul McKenney
Date: Sat Feb 11 2017 - 14:25:46 EST

Next message: Bhumika Goyal: "[PATCH] clocksource: add __ro_after_init to cyclecounter"
Previous message: Bhumika Goyal: "[PATCH] kernel: ksysfs: add __ro_after_init to bin_attribute structure"
In reply to: Ingo Molnar: "[PATCH 07/10] rcu: Separate the RCU synchronization types and APIs into <linux/rcupdate_wait.h>"
Next in thread: Ingo Molnar: "[PATCH 09/10] mm/vmacache, sched/headers: Introduce 'struct vmacache' and move it from <linux/sched.h> to <linux/mm_types>"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Feb 8, 2017 at 10:34 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> So rcupdate.h is a pretty complex header, in particular it includes
> <linux/completion.h> which includes <linux/wait.h> - creating a
> dependency that includes <linux/wait.h> in <linux/sched.h>,
> which prevents the isolation of <linux/sched.h> from the derived
> <linux/wait.h> header.
>
> Solve part of the problem by decoupling rcupdate.h from completions:
> this can be done by separating out the rcu_synchronize types and APIs,
> and updating their usage sites.
>
> Since this is a mostly RCU-internal types this will not just simplify
> <linux/sched.h>'s dependencies, but will make all the hundreds of
> .c files that include rcupdate.h but not completions or wait.h build
> faster.

Indeed, rcupdate.h is overdue for a more sweeping overhaul.

> ( For rcutiny this means that two dependent APIs have to be uninlined,
> but that shouldn't be much of a problem as they are rare variants. )

Do people still care about Tiny kernel? If so, I can queue a patch
that leaves rcu_barrier_bh() and rcu_barrier_sched() inline, but
creates an rcu_barrier_generic() or some such, so that there is
space taken up by only one EXPORT_SYMBOL() instead of two.
(0day Test Robot yells at me every time I add one...)

Other than that, I don't see any problems with this. I will do some
testing.

Thanx, Paul

> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Cc: Mike Galbraith <efault@xxxxxx>
> Cc: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
> ---
> fs/autofs4/autofs_i.h | 1 +
> include/linux/dcache.h | 1 +
> include/linux/rcupdate.h | 40 ----------------------------------------
> include/linux/rcupdate_wait.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/rcutiny.h | 11 ++---------
> kernel/rcu/srcu.c | 2 +-
> kernel/rcu/tiny.c | 14 +++++++++++++-
> kernel/rcu/tree.c | 2 +-
> kernel/rcu/update.c | 1 +
> kernel/sched/core.c | 1 +
> 10 files changed, 71 insertions(+), 52 deletions(-)
>
> diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
> index c885daae68c8..beef981aa54f 100644
> --- a/fs/autofs4/autofs_i.h
> +++ b/fs/autofs4/autofs_i.h
> @@ -14,6 +14,7 @@
> #include <linux/mutex.h>
> #include <linux/spinlock.h>
> #include <linux/list.h>
> +#include <linux/completion.h>
>
> /* This is the range of ioctl() numbers we claim as ours */
> #define AUTOFS_IOC_FIRST AUTOFS_IOC_READY
> diff --git a/include/linux/dcache.h b/include/linux/dcache.h
> index c965e4469499..16948defb568 100644
> --- a/include/linux/dcache.h
> +++ b/include/linux/dcache.h
> @@ -11,6 +11,7 @@
> #include <linux/rcupdate.h>
> #include <linux/lockref.h>
> #include <linux/stringhash.h>
> +#include <linux/wait.h>
>
> struct path;
> struct vfsmount;
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 01f71e1d2e94..1f476b63c596 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -40,7 +40,6 @@
> #include <linux/cpumask.h>
> #include <linux/seqlock.h>
> #include <linux/lockdep.h>
> -#include <linux/completion.h>
> #include <linux/debugobjects.h>
> #include <linux/bug.h>
> #include <linux/compiler.h>
> @@ -226,45 +225,6 @@ void call_rcu_sched(struct rcu_head *head,
>
> void synchronize_sched(void);
>
> -/*
> - * Structure allowing asynchronous waiting on RCU.
> - */
> -struct rcu_synchronize {
> - struct rcu_head head;
> - struct completion completion;
> -};

> -void wakeme_after_rcu(struct rcu_head *head);> -
> -void __wait_rcu_gp(bool checktiny, int n, call_rcu_func_t *crcu_array,
> - struct rcu_synchronize *rs_array);
> -
> -#define _wait_rcu_gp(checktiny, ...) \
> -do { \
> - call_rcu_func_t __crcu_array[] = { __VA_ARGS__ }; \
> - struct rcu_synchronize __rs_array[ARRAY_SIZE(__crcu_array)]; \
> - __wait_rcu_gp(checktiny, ARRAY_SIZE(__crcu_array), \
> - __crcu_array, __rs_array); \
> -} while (0)
> -
> -#define wait_rcu_gp(...) _wait_rcu_gp(false, __VA_ARGS__)
> -
> -/**
> - * synchronize_rcu_mult - Wait concurrently for multiple grace periods
> - * @...: List of call_rcu() functions for the flavors to wait on.
> - *
> - * This macro waits concurrently for multiple flavors of RCU grace periods.
> - * For example, synchronize_rcu_mult(call_rcu, call_rcu_bh) would wait
> - * on concurrent RCU and RCU-bh grace periods. Waiting on a give SRCU
> - * domain requires you to write a wrapper function for that SRCU domain's
> - * call_srcu() function, supplying the corresponding srcu_struct.
> - *
> - * If Tiny RCU, tell _wait_rcu_gp() not to bother waiting for RCU
> - * or RCU-bh, given that anywhere synchronize_rcu_mult() can be called
> - * is automatically a grace period.
> - */
> -#define synchronize_rcu_mult(...) \
> - _wait_rcu_gp(IS_ENABLED(CONFIG_TINY_RCU), __VA_ARGS__)
> -
> /**
> * call_rcu_tasks() - Queue an RCU for invocation task-based grace period
> * @head: structure to be used for queueing the RCU updates.
> diff --git a/include/linux/rcupdate_wait.h b/include/linux/rcupdate_wait.h
> new file mode 100644
> index 000000000000..e774b4f5f220
> --- /dev/null
> +++ b/include/linux/rcupdate_wait.h
> @@ -0,0 +1,50 @@
> +#ifndef _LINUX_SCHED_RCUPDATE_WAIT_H
> +#define _LINUX_SCHED_RCUPDATE_WAIT_H
> +
> +/*
> + * RCU synchronization types and methods:
> + */
> +
> +#include <linux/rcupdate.h>
> +#include <linux/completion.h>
> +
> +/*
> + * Structure allowing asynchronous waiting on RCU.
> + */
> +struct rcu_synchronize {
> + struct rcu_head head;
> + struct completion completion;
> +};
> +void wakeme_after_rcu(struct rcu_head *head);
> +
> +void __wait_rcu_gp(bool checktiny, int n, call_rcu_func_t *crcu_array,
> + struct rcu_synchronize *rs_array);
> +
> +#define _wait_rcu_gp(checktiny, ...) \
> +do { \
> + call_rcu_func_t __crcu_array[] = { __VA_ARGS__ }; \
> + struct rcu_synchronize __rs_array[ARRAY_SIZE(__crcu_array)]; \
> + __wait_rcu_gp(checktiny, ARRAY_SIZE(__crcu_array), \
> + __crcu_array, __rs_array); \
> +} while (0)
> +
> +#define wait_rcu_gp(...) _wait_rcu_gp(false, __VA_ARGS__)
> +
> +/**
> + * synchronize_rcu_mult - Wait concurrently for multiple grace periods
> + * @...: List of call_rcu() functions for the flavors to wait on.
> + *
> + * This macro waits concurrently for multiple flavors of RCU grace periods.
> + * For example, synchronize_rcu_mult(call_rcu, call_rcu_bh) would wait
> + * on concurrent RCU and RCU-bh grace periods. Waiting on a give SRCU
> + * domain requires you to write a wrapper function for that SRCU domain's
> + * call_srcu() function, supplying the corresponding srcu_struct.
> + *
> + * If Tiny RCU, tell _wait_rcu_gp() not to bother waiting for RCU
> + * or RCU-bh, given that anywhere synchronize_rcu_mult() can be called
> + * is automatically a grace period.
> + */
> +#define synchronize_rcu_mult(...) \
> + _wait_rcu_gp(IS_ENABLED(CONFIG_TINY_RCU), __VA_ARGS__)
> +
> +#endif /* _LINUX_SCHED_RCUPDATE_WAIT_H */
> diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
> index ac81e4063b40..861ab86ae302 100644
> --- a/include/linux/rcutiny.h
> +++ b/include/linux/rcutiny.h
> @@ -47,15 +47,8 @@ static inline void cond_synchronize_sched(unsigned long oldstate)
> might_sleep();
> }
>
> -static inline void rcu_barrier_bh(void)
> -{
> - wait_rcu_gp(call_rcu_bh);
> -}
> -
> -static inline void rcu_barrier_sched(void)
> -{
> - wait_rcu_gp(call_rcu_sched);
> -}
> +extern void rcu_barrier_bh(void);
> +extern void rcu_barrier_sched(void);
>
> static inline void synchronize_rcu_expedited(void)
> {
> diff --git a/kernel/rcu/srcu.c b/kernel/rcu/srcu.c
> index 9b9cdd549caa..c05855490b54 100644
> --- a/kernel/rcu/srcu.c
> +++ b/kernel/rcu/srcu.c
> @@ -30,7 +30,7 @@
> #include <linux/mutex.h>
> #include <linux/percpu.h>
> #include <linux/preempt.h>
> -#include <linux/rcupdate.h>
> +#include <linux/rcupdate_wait.h>
> #include <linux/sched.h>
> #include <linux/smp.h>
> #include <linux/delay.h>
> diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> index b23a4d076f3d..55770f10342e 100644
> --- a/kernel/rcu/tiny.c
> +++ b/kernel/rcu/tiny.c
> @@ -25,7 +25,7 @@
> #include <linux/completion.h>
> #include <linux/interrupt.h>
> #include <linux/notifier.h>
> -#include <linux/rcupdate.h>
> +#include <linux/rcupdate_wait.h>
> #include <linux/kernel.h>
> #include <linux/export.h>
> #include <linux/mutex.h>
> @@ -49,6 +49,18 @@ static void __call_rcu(struct rcu_head *head,
>
> #include "tiny_plugin.h"
>
> +void rcu_barrier_bh(void)
> +{
> + wait_rcu_gp(call_rcu_bh);
> +}
> +EXPORT_SYMBOL(rcu_barrier_bh);
> +
> +void rcu_barrier_sched(void)
> +{
> + wait_rcu_gp(call_rcu_sched);
> +}
> +EXPORT_SYMBOL(rcu_barrier_sched);
> +
> #if defined(CONFIG_DEBUG_LOCK_ALLOC) || defined(CONFIG_RCU_TRACE)
>
> /*
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index cb4e2056ccf3..2d87cb3ad7c6 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -32,7 +32,7 @@
> #include <linux/init.h>
> #include <linux/spinlock.h>
> #include <linux/smp.h>
> -#include <linux/rcupdate.h>
> +#include <linux/rcupdate_wait.h>
> #include <linux/interrupt.h>
> #include <linux/sched.h>
> #include <linux/nmi.h>
> diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
> index 4f6db7e6a117..2730e5c9ad2e 100644
> --- a/kernel/rcu/update.c
> +++ b/kernel/rcu/update.c
> @@ -49,6 +49,7 @@
> #include <linux/moduleparam.h>
> #include <linux/kthread.h>
> #include <linux/tick.h>
> +#include <linux/rcupdate_wait.h>
>
> #define CREATE_TRACE_POINTS
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 258af6d00682..f3ce7cf60002 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -10,6 +10,7 @@
> #include <linux/delayacct.h>
> #include <linux/init_task.h>
> #include <linux/context_tracking.h>
> +#include <linux/rcupdate_wait.h>
>
> #include <linux/blkdev.h>
> #include <linux/kprobes.h>
> --
> 2.7.4
>

Next message: Bhumika Goyal: "[PATCH] clocksource: add __ro_after_init to cyclecounter"
Previous message: Bhumika Goyal: "[PATCH] kernel: ksysfs: add __ro_after_init to bin_attribute structure"
In reply to: Ingo Molnar: "[PATCH 07/10] rcu: Separate the RCU synchronization types and APIs into <linux/rcupdate_wait.h>"
Next in thread: Ingo Molnar: "[PATCH 09/10] mm/vmacache, sched/headers: Introduce 'struct vmacache' and move it from <linux/sched.h> to <linux/mm_types>"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]