2018-08-29 21:22:10

by Paul E. McKenney

[permalink] [raw]
Subject: [PATCH tip/core/rcu 0/3] SRCU updates for v4.20/v5.0

Hello!

This series allows call_srcu() to be used during very early boot:

1. Make call_srcu() available during very early boot.

2. Test early boot call_srcu().

3. Make early-boot call_srcu() reuse workqueue lists.

Thanx, Paul

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

include/linux/srcutiny.h | 4 -
include/linux/srcutree.h | 17 +++--
kernel/rcu/rcu.h | 6 ++
kernel/rcu/srcutiny.c | 39 ++++++++++---
kernel/rcu/srcutree.c | 36 +++++++++---
kernel/rcu/tiny.c | 1
kernel/rcu/tree.c | 1
kernel/rcu/update.c | 9 +++
tools/testing/selftests/rcutorture/configs/rcu/SRCU-P.boot | 1
tools/testing/selftests/rcutorture/configs/rcu/SRCU-u.boot | 1
tools/testing/selftests/rcutorture/configs/rcu/TREE05.boot | 1
11 files changed, 92 insertions(+), 24 deletions(-)



2018-08-29 21:24:53

by Paul E. McKenney

[permalink] [raw]
Subject: [PATCH tip/core/rcu 1/3] srcu: Make call_srcu() available during very early boot

Event tracing is moving to SRCU in order to take advantage of the fact
that SRCU may be safely used from idle and even offline CPUs. However,
event tracing can invoke call_srcu() very early in the boot process,
even before workqueue_init_early() is invoked (let alone rcu_init()).
Therefore, call_srcu()'s attempts to queue work fail miserably.

This commit therefore detects this situation, and refrains from attempting
to queue work before rcu_init() time, but does everything else that it
would have done, and in addition, adds the srcu_struct to a global list.
The rcu_init() function now invokes a new srcu_init() function, which
is empty if CONFIG_SRCU=n. Otherwise, srcu_init() queues work for
each srcu_struct on the list. This all happens early enough in boot
that there is but a single CPU with interrupts disabled, which allows
synchronization to be dispensed with.

Of course, the queued work won't actually be invoked until after
workqueue_init() is invoked, which happens shortly after the scheduler
is up and running. This means that although call_srcu() may be invoked
any time after per-CPU variables have been set up, there is still a very
narrow window when synchronize_srcu() won't work, and this window
extends from the time that the scheduler starts until the time that
workqueue_init() returns. This can be fixed in a manner similar to
the fix for synchronize_rcu_expedited() and friends, but until someone
actually needs to use synchronize_srcu() during this window, this fix
is added churn for no benefit.

Finally, note that Tree SRCU's new srcu_init() function invokes
queue_work() rather than the queue_delayed_work() function that is
invoked post-boot. The reason is that queue_delayed_work() will (as you
would expect) post a timer, and timers have not yet been initialized.
So use of queue_work() avoids the complaints about use of uninitialized
spinlocks that would otherwise result. Besides, some delay is already
provide by the aforementioned fact that the queued work won't actually
be invoked until after the scheduler is up and running.

Requested-by: Steven Rostedt <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
---
include/linux/srcutiny.h | 2 ++
include/linux/srcutree.h | 14 ++++++++------
kernel/rcu/rcu.h | 6 ++++++
kernel/rcu/srcutiny.c | 29 +++++++++++++++++++++++++++--
kernel/rcu/srcutree.c | 25 ++++++++++++++++++++++++-
kernel/rcu/tiny.c | 1 +
kernel/rcu/tree.c | 1 +
kernel/rcu/update.c | 9 +++++++++
8 files changed, 78 insertions(+), 9 deletions(-)

diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
index f41d2fb09f87..2b5c0822e683 100644
--- a/include/linux/srcutiny.h
+++ b/include/linux/srcutiny.h
@@ -36,6 +36,7 @@ struct srcu_struct {
struct rcu_head *srcu_cb_head; /* Pending callbacks: Head. */
struct rcu_head **srcu_cb_tail; /* Pending callbacks: Tail. */
struct work_struct srcu_work; /* For driving grace periods. */
+ struct list_head srcu_boot_entry; /* Early-boot callbacks. */
#ifdef CONFIG_DEBUG_LOCK_ALLOC
struct lockdep_map dep_map;
#endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
@@ -48,6 +49,7 @@ void srcu_drive_gp(struct work_struct *wp);
.srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq), \
.srcu_cb_tail = &name.srcu_cb_head, \
.srcu_work = __WORK_INITIALIZER(name.srcu_work, srcu_drive_gp), \
+ .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
__SRCU_DEP_MAP_INIT(name) \
}

diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
index 745d4ca4dd50..9cfa4610113a 100644
--- a/include/linux/srcutree.h
+++ b/include/linux/srcutree.h
@@ -94,6 +94,7 @@ struct srcu_struct {
/* callback for the barrier */
/* operation. */
struct delayed_work work;
+ struct list_head srcu_boot_entry; /* Early-boot callbacks. */
#ifdef CONFIG_DEBUG_LOCK_ALLOC
struct lockdep_map dep_map;
#endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
@@ -105,12 +106,13 @@ struct srcu_struct {
#define SRCU_STATE_SCAN2 2

#define __SRCU_STRUCT_INIT(name, pcpu_name) \
- { \
- .sda = &pcpu_name, \
- .lock = __SPIN_LOCK_UNLOCKED(name.lock), \
- .srcu_gp_seq_needed = 0 - 1, \
- __SRCU_DEP_MAP_INIT(name) \
- }
+{ \
+ .sda = &pcpu_name, \
+ .lock = __SPIN_LOCK_UNLOCKED(name.lock), \
+ .srcu_gp_seq_needed = -1UL, \
+ .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
+ __SRCU_DEP_MAP_INIT(name) \
+}

/*
* Define and initialize a srcu struct at build time.
diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index 4d04683c31b2..e1b5aec5ec1c 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -435,6 +435,12 @@ do { \

#endif /* #if defined(SRCU) || !defined(TINY_RCU) */

+#ifdef CONFIG_SRCU
+void srcu_init(void);
+#else /* #ifdef CONFIG_SRCU */
+static inline void srcu_init(void) { }
+#endif /* #else #ifdef CONFIG_SRCU */
+
#ifdef CONFIG_TINY_RCU
/* Tiny RCU doesn't expedite, as its purpose in life is instead to be tiny. */
static inline bool rcu_gp_is_normal(void) { return true; }
diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
index 04fc2ed71af8..d233f0c63f6f 100644
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -34,6 +34,8 @@
#include "rcu.h"

int rcu_scheduler_active __read_mostly;
+static LIST_HEAD(srcu_boot_list);
+static bool srcu_init_done;

static int init_srcu_struct_fields(struct srcu_struct *sp)
{
@@ -46,6 +48,7 @@ static int init_srcu_struct_fields(struct srcu_struct *sp)
sp->srcu_gp_waiting = false;
sp->srcu_idx = 0;
INIT_WORK(&sp->srcu_work, srcu_drive_gp);
+ INIT_LIST_HEAD(&sp->srcu_boot_entry);
return 0;
}

@@ -179,8 +182,12 @@ void call_srcu(struct srcu_struct *sp, struct rcu_head *rhp,
*sp->srcu_cb_tail = rhp;
sp->srcu_cb_tail = &rhp->next;
local_irq_restore(flags);
- if (!READ_ONCE(sp->srcu_gp_running))
- schedule_work(&sp->srcu_work);
+ if (!READ_ONCE(sp->srcu_gp_running)) {
+ if (likely(srcu_init_done))
+ schedule_work(&sp->srcu_work);
+ else if (list_empty(&sp->srcu_boot_entry))
+ list_add(&sp->srcu_boot_entry, &srcu_boot_list);
+ }
}
EXPORT_SYMBOL_GPL(call_srcu);

@@ -204,3 +211,21 @@ void __init rcu_scheduler_starting(void)
{
rcu_scheduler_active = RCU_SCHEDULER_RUNNING;
}
+
+/*
+ * Queue work for srcu_struct structures with early boot callbacks.
+ * The work won't actually execute until the workqueue initialization
+ * phase that takes place after the scheduler starts.
+ */
+void __init srcu_init(void)
+{
+ struct srcu_struct *sp;
+
+ srcu_init_done = true;
+ while (!list_empty(&srcu_boot_list)) {
+ sp = list_first_entry(&srcu_boot_list,
+ struct srcu_struct, srcu_boot_entry);
+ list_del_init(&sp->srcu_boot_entry);
+ schedule_work(&sp->srcu_work);
+ }
+}
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 6c9866a854b1..e79c1929328f 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -51,6 +51,10 @@ module_param(exp_holdoff, ulong, 0444);
static ulong counter_wrap_check = (ULONG_MAX >> 2);
module_param(counter_wrap_check, ulong, 0444);

+/* Early-boot callback-management, so early that no lock is required! */
+static LIST_HEAD(srcu_boot_list);
+static bool __read_mostly srcu_init_done;
+
static void srcu_invoke_callbacks(struct work_struct *work);
static void srcu_reschedule(struct srcu_struct *sp, unsigned long delay);
static void process_srcu(struct work_struct *work);
@@ -182,6 +186,7 @@ static int init_srcu_struct_fields(struct srcu_struct *sp, bool is_static)
mutex_init(&sp->srcu_barrier_mutex);
atomic_set(&sp->srcu_barrier_cpu_cnt, 0);
INIT_DELAYED_WORK(&sp->work, process_srcu);
+ INIT_LIST_HEAD(&sp->srcu_boot_entry);
if (!is_static)
sp->sda = alloc_percpu(struct srcu_data);
init_srcu_struct_nodes(sp, is_static);
@@ -701,7 +706,11 @@ static void srcu_funnel_gp_start(struct srcu_struct *sp, struct srcu_data *sdp,
rcu_seq_state(sp->srcu_gp_seq) == SRCU_STATE_IDLE) {
WARN_ON_ONCE(ULONG_CMP_GE(sp->srcu_gp_seq, sp->srcu_gp_seq_needed));
srcu_gp_start(sp);
- queue_delayed_work(rcu_gp_wq, &sp->work, srcu_get_delay(sp));
+ if (likely(srcu_init_done))
+ queue_delayed_work(rcu_gp_wq, &sp->work,
+ srcu_get_delay(sp));
+ else if (list_empty(&sp->srcu_boot_entry))
+ list_add(&sp->srcu_boot_entry, &srcu_boot_list);
}
spin_unlock_irqrestore_rcu_node(sp, flags);
}
@@ -1308,3 +1317,17 @@ static int __init srcu_bootup_announce(void)
return 0;
}
early_initcall(srcu_bootup_announce);
+
+void __init srcu_init(void)
+{
+ struct srcu_struct *sp;
+
+ srcu_init_done = true;
+ while (!list_empty(&srcu_boot_list)) {
+ sp = list_first_entry(&srcu_boot_list,
+ struct srcu_struct, srcu_boot_entry);
+ check_init_srcu_struct(sp);
+ list_del_init(&sp->srcu_boot_entry);
+ queue_work(rcu_gp_wq, &sp->work.work);
+ }
+}
diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
index befc9321a89c..101ed5bb836c 100644
--- a/kernel/rcu/tiny.c
+++ b/kernel/rcu/tiny.c
@@ -236,4 +236,5 @@ void __init rcu_init(void)
{
open_softirq(RCU_SOFTIRQ, rcu_process_callbacks);
rcu_early_boot_tests();
+ srcu_init();
}
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 0b760c1369f7..43c806291208 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -4164,6 +4164,7 @@ void __init rcu_init(void)
WARN_ON(!rcu_gp_wq);
rcu_par_gp_wq = alloc_workqueue("rcu_par_gp", WQ_MEM_RECLAIM, 0);
WARN_ON(!rcu_par_gp_wq);
+ srcu_init();
}

#include "tree_exp.h"
diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index 39cb23d22109..7d057d0aaec4 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -888,11 +888,16 @@ static void test_callback(struct rcu_head *r)
pr_info("RCU test callback executed %d\n", rcu_self_test_counter);
}

+DEFINE_STATIC_SRCU(early_srcu);
+
static void early_boot_test_call_rcu(void)
{
static struct rcu_head head;
+ static struct rcu_head shead;

call_rcu(&head, test_callback);
+ if (IS_ENABLED(CONFIG_SRCU))
+ call_srcu(&early_srcu, &shead, test_callback);
}

static void early_boot_test_call_rcu_bh(void)
@@ -930,6 +935,10 @@ static int rcu_verify_early_boot_tests(void)
if (rcu_self_test) {
early_boot_test_counter++;
rcu_barrier();
+ if (IS_ENABLED(CONFIG_SRCU)) {
+ early_boot_test_counter++;
+ srcu_barrier(&early_srcu);
+ }
}
if (rcu_self_test_bh) {
early_boot_test_counter++;
--
2.17.1


2018-08-29 21:25:51

by Paul E. McKenney

[permalink] [raw]
Subject: [PATCH tip/core/rcu 2/3] rcutorture: Test early boot call_srcu()

Now that SRCU permits call_srcu() to be invoked at early boot, this
commit ensures that the rcutorture scripting tests early boot call_srcu().

Signed-off-by: Paul E. McKenney <[email protected]>
---
tools/testing/selftests/rcutorture/configs/rcu/SRCU-P.boot | 1 +
tools/testing/selftests/rcutorture/configs/rcu/SRCU-u.boot | 1 +
tools/testing/selftests/rcutorture/configs/rcu/TREE05.boot | 1 +
3 files changed, 3 insertions(+)

diff --git a/tools/testing/selftests/rcutorture/configs/rcu/SRCU-P.boot b/tools/testing/selftests/rcutorture/configs/rcu/SRCU-P.boot
index 84a7d51b7481..ce48c7b82673 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/SRCU-P.boot
+++ b/tools/testing/selftests/rcutorture/configs/rcu/SRCU-P.boot
@@ -1 +1,2 @@
rcutorture.torture_type=srcud
+rcupdate.rcu_self_test=1
diff --git a/tools/testing/selftests/rcutorture/configs/rcu/SRCU-u.boot b/tools/testing/selftests/rcutorture/configs/rcu/SRCU-u.boot
index 84a7d51b7481..ce48c7b82673 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/SRCU-u.boot
+++ b/tools/testing/selftests/rcutorture/configs/rcu/SRCU-u.boot
@@ -1 +1,2 @@
rcutorture.torture_type=srcud
+rcupdate.rcu_self_test=1
diff --git a/tools/testing/selftests/rcutorture/configs/rcu/TREE05.boot b/tools/testing/selftests/rcutorture/configs/rcu/TREE05.boot
index c7fd050dfcd9..dfebc82932ca 100644
--- a/tools/testing/selftests/rcutorture/configs/rcu/TREE05.boot
+++ b/tools/testing/selftests/rcutorture/configs/rcu/TREE05.boot
@@ -3,3 +3,4 @@ rcupdate.rcu_self_test_sched=1
rcutree.gp_preinit_delay=3
rcutree.gp_init_delay=3
rcutree.gp_cleanup_delay=3
+rcupdate.rcu_self_test=1
--
2.17.1


2018-08-29 21:28:56

by Paul E. McKenney

[permalink] [raw]
Subject: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

Allocating a list_head structure that is almost never used, and, when
used, is used only during early boot (rcu_init() and earlier), is a bit
wasteful. This commit therefore eliminates that list_head in favor of
the one in the work_struct structure. This is safe because the work_struct
structure cannot be used until after rcu_init() returns.

Reported-by: Steven Rostedt <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: Lai Jiangshan <[email protected]>
---
include/linux/srcutiny.h | 2 --
include/linux/srcutree.h | 3 +--
kernel/rcu/srcutiny.c | 10 +++++-----
kernel/rcu/srcutree.c | 11 +++++------
4 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
index 2b5c0822e683..f41d2fb09f87 100644
--- a/include/linux/srcutiny.h
+++ b/include/linux/srcutiny.h
@@ -36,7 +36,6 @@ struct srcu_struct {
struct rcu_head *srcu_cb_head; /* Pending callbacks: Head. */
struct rcu_head **srcu_cb_tail; /* Pending callbacks: Tail. */
struct work_struct srcu_work; /* For driving grace periods. */
- struct list_head srcu_boot_entry; /* Early-boot callbacks. */
#ifdef CONFIG_DEBUG_LOCK_ALLOC
struct lockdep_map dep_map;
#endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
@@ -49,7 +48,6 @@ void srcu_drive_gp(struct work_struct *wp);
.srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq), \
.srcu_cb_tail = &name.srcu_cb_head, \
.srcu_work = __WORK_INITIALIZER(name.srcu_work, srcu_drive_gp), \
- .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
__SRCU_DEP_MAP_INIT(name) \
}

diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
index 9cfa4610113a..0ae91b3a7406 100644
--- a/include/linux/srcutree.h
+++ b/include/linux/srcutree.h
@@ -94,7 +94,6 @@ struct srcu_struct {
/* callback for the barrier */
/* operation. */
struct delayed_work work;
- struct list_head srcu_boot_entry; /* Early-boot callbacks. */
#ifdef CONFIG_DEBUG_LOCK_ALLOC
struct lockdep_map dep_map;
#endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
@@ -110,7 +109,7 @@ struct srcu_struct {
.sda = &pcpu_name, \
.lock = __SPIN_LOCK_UNLOCKED(name.lock), \
.srcu_gp_seq_needed = -1UL, \
- .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
+ .work = __DELAYED_WORK_INITIALIZER(name.work, NULL, 0), \
__SRCU_DEP_MAP_INIT(name) \
}

diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
index d233f0c63f6f..b46e6683f8c9 100644
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -48,7 +48,7 @@ static int init_srcu_struct_fields(struct srcu_struct *sp)
sp->srcu_gp_waiting = false;
sp->srcu_idx = 0;
INIT_WORK(&sp->srcu_work, srcu_drive_gp);
- INIT_LIST_HEAD(&sp->srcu_boot_entry);
+ INIT_LIST_HEAD(&sp->srcu_work.entry);
return 0;
}

@@ -185,8 +185,8 @@ void call_srcu(struct srcu_struct *sp, struct rcu_head *rhp,
if (!READ_ONCE(sp->srcu_gp_running)) {
if (likely(srcu_init_done))
schedule_work(&sp->srcu_work);
- else if (list_empty(&sp->srcu_boot_entry))
- list_add(&sp->srcu_boot_entry, &srcu_boot_list);
+ else if (list_empty(&sp->srcu_work.entry))
+ list_add(&sp->srcu_work.entry, &srcu_boot_list);
}
}
EXPORT_SYMBOL_GPL(call_srcu);
@@ -224,8 +224,8 @@ void __init srcu_init(void)
srcu_init_done = true;
while (!list_empty(&srcu_boot_list)) {
sp = list_first_entry(&srcu_boot_list,
- struct srcu_struct, srcu_boot_entry);
- list_del_init(&sp->srcu_boot_entry);
+ struct srcu_struct, srcu_work.entry);
+ list_del_init(&sp->srcu_work.entry);
schedule_work(&sp->srcu_work);
}
}
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index e79c1929328f..36a2857c84e0 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -186,7 +186,6 @@ static int init_srcu_struct_fields(struct srcu_struct *sp, bool is_static)
mutex_init(&sp->srcu_barrier_mutex);
atomic_set(&sp->srcu_barrier_cpu_cnt, 0);
INIT_DELAYED_WORK(&sp->work, process_srcu);
- INIT_LIST_HEAD(&sp->srcu_boot_entry);
if (!is_static)
sp->sda = alloc_percpu(struct srcu_data);
init_srcu_struct_nodes(sp, is_static);
@@ -709,8 +708,8 @@ static void srcu_funnel_gp_start(struct srcu_struct *sp, struct srcu_data *sdp,
if (likely(srcu_init_done))
queue_delayed_work(rcu_gp_wq, &sp->work,
srcu_get_delay(sp));
- else if (list_empty(&sp->srcu_boot_entry))
- list_add(&sp->srcu_boot_entry, &srcu_boot_list);
+ else if (list_empty(&sp->work.work.entry))
+ list_add(&sp->work.work.entry, &srcu_boot_list);
}
spin_unlock_irqrestore_rcu_node(sp, flags);
}
@@ -1324,10 +1323,10 @@ void __init srcu_init(void)

srcu_init_done = true;
while (!list_empty(&srcu_boot_list)) {
- sp = list_first_entry(&srcu_boot_list,
- struct srcu_struct, srcu_boot_entry);
+ sp = list_first_entry(&srcu_boot_list, struct srcu_struct,
+ work.work.entry);
check_init_srcu_struct(sp);
- list_del_init(&sp->srcu_boot_entry);
+ list_del_init(&sp->work.work.entry);
queue_work(rcu_gp_wq, &sp->work.work);
}
}
--
2.17.1


2018-08-30 01:58:36

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

On Wed, 29 Aug 2018 14:23:13 -0700
"Paul E. McKenney" <[email protected]> wrote:

> Allocating a list_head structure that is almost never used, and, when
> used, is used only during early boot (rcu_init() and earlier), is a bit
> wasteful. This commit therefore eliminates that list_head in favor of
> the one in the work_struct structure. This is safe because the work_struct
> structure cannot be used until after rcu_init() returns.
>
> Reported-by: Steven Rostedt <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> Cc: Tejun Heo <[email protected]>
> Cc: Lai Jiangshan <[email protected]>
> ---
> include/linux/srcutiny.h | 2 --
> include/linux/srcutree.h | 3 +--
> kernel/rcu/srcutiny.c | 10 +++++-----
> kernel/rcu/srcutree.c | 11 +++++------
> 4 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
> index 2b5c0822e683..f41d2fb09f87 100644
> --- a/include/linux/srcutiny.h
> +++ b/include/linux/srcutiny.h
> @@ -36,7 +36,6 @@ struct srcu_struct {
> struct rcu_head *srcu_cb_head; /* Pending callbacks: Head. */
> struct rcu_head **srcu_cb_tail; /* Pending callbacks: Tail. */
> struct work_struct srcu_work; /* For driving grace periods. */
> - struct list_head srcu_boot_entry; /* Early-boot callbacks. */
> #ifdef CONFIG_DEBUG_LOCK_ALLOC
> struct lockdep_map dep_map;
> #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
> @@ -49,7 +48,6 @@ void srcu_drive_gp(struct work_struct *wp);
> .srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq), \
> .srcu_cb_tail = &name.srcu_cb_head, \
> .srcu_work = __WORK_INITIALIZER(name.srcu_work, srcu_drive_gp), \
> - .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
> __SRCU_DEP_MAP_INIT(name) \
> }
>
> diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> index 9cfa4610113a..0ae91b3a7406 100644
> --- a/include/linux/srcutree.h
> +++ b/include/linux/srcutree.h
> @@ -94,7 +94,6 @@ struct srcu_struct {
> /* callback for the barrier */
> /* operation. */
> struct delayed_work work;
> - struct list_head srcu_boot_entry; /* Early-boot callbacks. */
> #ifdef CONFIG_DEBUG_LOCK_ALLOC
> struct lockdep_map dep_map;
> #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
> @@ -110,7 +109,7 @@ struct srcu_struct {
> .sda = &pcpu_name, \
> .lock = __SPIN_LOCK_UNLOCKED(name.lock), \
> .srcu_gp_seq_needed = -1UL, \
> - .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
> + .work = __DELAYED_WORK_INITIALIZER(name.work, NULL, 0), \

Thanks!

-- Steve

> __SRCU_DEP_MAP_INIT(name) \
> }
>
> diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
> index d233f0c63f6f..b46e6683f8c9 100644
> --- a/kernel/rcu/srcutiny.c
> +++ b/kernel/rcu/srcutiny.c
> @@ -48,7 +48,7 @@ static int init_srcu_struct_fields(struct srcu_struct *sp)
> sp->srcu_gp_waiting = false;
> sp->srcu_idx = 0;
> INIT_WORK(&sp->srcu_work, srcu_drive_gp);
> - INIT_LIST_HEAD(&sp->srcu_boot_entry);
> + INIT_LIST_HEAD(&sp->srcu_work.entry);
> return 0;
> }
>
> @@ -185,8 +185,8 @@ void call_srcu(struct srcu_struct *sp, struct rcu_head *rhp,
> if (!READ_ONCE(sp->srcu_gp_running)) {
> if (likely(srcu_init_done))
> schedule_work(&sp->srcu_work);
> - else if (list_empty(&sp->srcu_boot_entry))
> - list_add(&sp->srcu_boot_entry, &srcu_boot_list);
> + else if (list_empty(&sp->srcu_work.entry))
> + list_add(&sp->srcu_work.entry, &srcu_boot_list);
> }
> }
> EXPORT_SYMBOL_GPL(call_srcu);
> @@ -224,8 +224,8 @@ void __init srcu_init(void)
> srcu_init_done = true;
> while (!list_empty(&srcu_boot_list)) {
> sp = list_first_entry(&srcu_boot_list,
> - struct srcu_struct, srcu_boot_entry);
> - list_del_init(&sp->srcu_boot_entry);
> + struct srcu_struct, srcu_work.entry);
> + list_del_init(&sp->srcu_work.entry);
> schedule_work(&sp->srcu_work);
> }
> }
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index e79c1929328f..36a2857c84e0 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -186,7 +186,6 @@ static int init_srcu_struct_fields(struct srcu_struct *sp, bool is_static)
> mutex_init(&sp->srcu_barrier_mutex);
> atomic_set(&sp->srcu_barrier_cpu_cnt, 0);
> INIT_DELAYED_WORK(&sp->work, process_srcu);
> - INIT_LIST_HEAD(&sp->srcu_boot_entry);
> if (!is_static)
> sp->sda = alloc_percpu(struct srcu_data);
> init_srcu_struct_nodes(sp, is_static);
> @@ -709,8 +708,8 @@ static void srcu_funnel_gp_start(struct srcu_struct *sp, struct srcu_data *sdp,
> if (likely(srcu_init_done))
> queue_delayed_work(rcu_gp_wq, &sp->work,
> srcu_get_delay(sp));
> - else if (list_empty(&sp->srcu_boot_entry))
> - list_add(&sp->srcu_boot_entry, &srcu_boot_list);
> + else if (list_empty(&sp->work.work.entry))
> + list_add(&sp->work.work.entry, &srcu_boot_list);
> }
> spin_unlock_irqrestore_rcu_node(sp, flags);
> }
> @@ -1324,10 +1323,10 @@ void __init srcu_init(void)
>
> srcu_init_done = true;
> while (!list_empty(&srcu_boot_list)) {
> - sp = list_first_entry(&srcu_boot_list,
> - struct srcu_struct, srcu_boot_entry);
> + sp = list_first_entry(&srcu_boot_list, struct srcu_struct,
> + work.work.entry);
> check_init_srcu_struct(sp);
> - list_del_init(&sp->srcu_boot_entry);
> + list_del_init(&sp->work.work.entry);
> queue_work(rcu_gp_wq, &sp->work.work);
> }
> }


2018-08-30 02:08:32

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

On Wed, Aug 29, 2018 at 09:56:16PM -0400, Steven Rostedt wrote:
> On Wed, 29 Aug 2018 14:23:13 -0700
> "Paul E. McKenney" <[email protected]> wrote:
>
> > Allocating a list_head structure that is almost never used, and, when
> > used, is used only during early boot (rcu_init() and earlier), is a bit
> > wasteful. This commit therefore eliminates that list_head in favor of
> > the one in the work_struct structure. This is safe because the work_struct
> > structure cannot be used until after rcu_init() returns.
> >
> > Reported-by: Steven Rostedt <[email protected]>
> > Signed-off-by: Paul E. McKenney <[email protected]>
> > Cc: Tejun Heo <[email protected]>
> > Cc: Lai Jiangshan <[email protected]>
> > ---
> > include/linux/srcutiny.h | 2 --
> > include/linux/srcutree.h | 3 +--
> > kernel/rcu/srcutiny.c | 10 +++++-----
> > kernel/rcu/srcutree.c | 11 +++++------
> > 4 files changed, 11 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
> > index 2b5c0822e683..f41d2fb09f87 100644
> > --- a/include/linux/srcutiny.h
> > +++ b/include/linux/srcutiny.h
> > @@ -36,7 +36,6 @@ struct srcu_struct {
> > struct rcu_head *srcu_cb_head; /* Pending callbacks: Head. */
> > struct rcu_head **srcu_cb_tail; /* Pending callbacks: Tail. */
> > struct work_struct srcu_work; /* For driving grace periods. */
> > - struct list_head srcu_boot_entry; /* Early-boot callbacks. */
> > #ifdef CONFIG_DEBUG_LOCK_ALLOC
> > struct lockdep_map dep_map;
> > #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
> > @@ -49,7 +48,6 @@ void srcu_drive_gp(struct work_struct *wp);
> > .srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq), \
> > .srcu_cb_tail = &name.srcu_cb_head, \
> > .srcu_work = __WORK_INITIALIZER(name.srcu_work, srcu_drive_gp), \
> > - .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
> > __SRCU_DEP_MAP_INIT(name) \
> > }
> >
> > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > index 9cfa4610113a..0ae91b3a7406 100644
> > --- a/include/linux/srcutree.h
> > +++ b/include/linux/srcutree.h
> > @@ -94,7 +94,6 @@ struct srcu_struct {
> > /* callback for the barrier */
> > /* operation. */
> > struct delayed_work work;
> > - struct list_head srcu_boot_entry; /* Early-boot callbacks. */
> > #ifdef CONFIG_DEBUG_LOCK_ALLOC
> > struct lockdep_map dep_map;
> > #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
> > @@ -110,7 +109,7 @@ struct srcu_struct {
> > .sda = &pcpu_name, \
> > .lock = __SPIN_LOCK_UNLOCKED(name.lock), \
> > .srcu_gp_seq_needed = -1UL, \
> > - .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
> > + .work = __DELAYED_WORK_INITIALIZER(name.work, NULL, 0), \
>
> Thanks!

Glad you like it! Does it actually work for you? ;-)

Thanx, Paul

> -- Steve
>
> > __SRCU_DEP_MAP_INIT(name) \
> > }
> >
> > diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
> > index d233f0c63f6f..b46e6683f8c9 100644
> > --- a/kernel/rcu/srcutiny.c
> > +++ b/kernel/rcu/srcutiny.c
> > @@ -48,7 +48,7 @@ static int init_srcu_struct_fields(struct srcu_struct *sp)
> > sp->srcu_gp_waiting = false;
> > sp->srcu_idx = 0;
> > INIT_WORK(&sp->srcu_work, srcu_drive_gp);
> > - INIT_LIST_HEAD(&sp->srcu_boot_entry);
> > + INIT_LIST_HEAD(&sp->srcu_work.entry);
> > return 0;
> > }
> >
> > @@ -185,8 +185,8 @@ void call_srcu(struct srcu_struct *sp, struct rcu_head *rhp,
> > if (!READ_ONCE(sp->srcu_gp_running)) {
> > if (likely(srcu_init_done))
> > schedule_work(&sp->srcu_work);
> > - else if (list_empty(&sp->srcu_boot_entry))
> > - list_add(&sp->srcu_boot_entry, &srcu_boot_list);
> > + else if (list_empty(&sp->srcu_work.entry))
> > + list_add(&sp->srcu_work.entry, &srcu_boot_list);
> > }
> > }
> > EXPORT_SYMBOL_GPL(call_srcu);
> > @@ -224,8 +224,8 @@ void __init srcu_init(void)
> > srcu_init_done = true;
> > while (!list_empty(&srcu_boot_list)) {
> > sp = list_first_entry(&srcu_boot_list,
> > - struct srcu_struct, srcu_boot_entry);
> > - list_del_init(&sp->srcu_boot_entry);
> > + struct srcu_struct, srcu_work.entry);
> > + list_del_init(&sp->srcu_work.entry);
> > schedule_work(&sp->srcu_work);
> > }
> > }
> > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > index e79c1929328f..36a2857c84e0 100644
> > --- a/kernel/rcu/srcutree.c
> > +++ b/kernel/rcu/srcutree.c
> > @@ -186,7 +186,6 @@ static int init_srcu_struct_fields(struct srcu_struct *sp, bool is_static)
> > mutex_init(&sp->srcu_barrier_mutex);
> > atomic_set(&sp->srcu_barrier_cpu_cnt, 0);
> > INIT_DELAYED_WORK(&sp->work, process_srcu);
> > - INIT_LIST_HEAD(&sp->srcu_boot_entry);
> > if (!is_static)
> > sp->sda = alloc_percpu(struct srcu_data);
> > init_srcu_struct_nodes(sp, is_static);
> > @@ -709,8 +708,8 @@ static void srcu_funnel_gp_start(struct srcu_struct *sp, struct srcu_data *sdp,
> > if (likely(srcu_init_done))
> > queue_delayed_work(rcu_gp_wq, &sp->work,
> > srcu_get_delay(sp));
> > - else if (list_empty(&sp->srcu_boot_entry))
> > - list_add(&sp->srcu_boot_entry, &srcu_boot_list);
> > + else if (list_empty(&sp->work.work.entry))
> > + list_add(&sp->work.work.entry, &srcu_boot_list);
> > }
> > spin_unlock_irqrestore_rcu_node(sp, flags);
> > }
> > @@ -1324,10 +1323,10 @@ void __init srcu_init(void)
> >
> > srcu_init_done = true;
> > while (!list_empty(&srcu_boot_list)) {
> > - sp = list_first_entry(&srcu_boot_list,
> > - struct srcu_struct, srcu_boot_entry);
> > + sp = list_first_entry(&srcu_boot_list, struct srcu_struct,
> > + work.work.entry);
> > check_init_srcu_struct(sp);
> > - list_del_init(&sp->srcu_boot_entry);
> > + list_del_init(&sp->work.work.entry);
> > queue_work(rcu_gp_wq, &sp->work.work);
> > }
> > }
>


2018-08-30 02:47:34

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

On Wed, 29 Aug 2018 19:07:01 -0700
"Paul E. McKenney" <[email protected]> wrote:

> On Wed, Aug 29, 2018 at 09:56:16PM -0400, Steven Rostedt wrote:
> > On Wed, 29 Aug 2018 14:23:13 -0700
> > "Paul E. McKenney" <[email protected]> wrote:
> >
> > > Allocating a list_head structure that is almost never used, and, when
> > > used, is used only during early boot (rcu_init() and earlier), is a bit
> > > wasteful. This commit therefore eliminates that list_head in favor of
> > > the one in the work_struct structure. This is safe because the work_struct
> > > structure cannot be used until after rcu_init() returns.
> > >
> > > Reported-by: Steven Rostedt <[email protected]>
> > > Signed-off-by: Paul E. McKenney <[email protected]>
> > > Cc: Tejun Heo <[email protected]>
> > > Cc: Lai Jiangshan <[email protected]>
> > > ---
> > > include/linux/srcutiny.h | 2 --
> > > include/linux/srcutree.h | 3 +--
> > > kernel/rcu/srcutiny.c | 10 +++++-----
> > > kernel/rcu/srcutree.c | 11 +++++------
> > > 4 files changed, 11 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
> > > index 2b5c0822e683..f41d2fb09f87 100644
> > > --- a/include/linux/srcutiny.h
> > > +++ b/include/linux/srcutiny.h
> > > @@ -36,7 +36,6 @@ struct srcu_struct {
> > > struct rcu_head *srcu_cb_head; /* Pending callbacks: Head. */
> > > struct rcu_head **srcu_cb_tail; /* Pending callbacks: Tail. */
> > > struct work_struct srcu_work; /* For driving grace periods. */
> > > - struct list_head srcu_boot_entry; /* Early-boot callbacks. */
> > > #ifdef CONFIG_DEBUG_LOCK_ALLOC
> > > struct lockdep_map dep_map;
> > > #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
> > > @@ -49,7 +48,6 @@ void srcu_drive_gp(struct work_struct *wp);
> > > .srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq), \
> > > .srcu_cb_tail = &name.srcu_cb_head, \
> > > .srcu_work = __WORK_INITIALIZER(name.srcu_work, srcu_drive_gp), \
> > > - .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
> > > __SRCU_DEP_MAP_INIT(name) \
> > > }
> > >
> > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > index 9cfa4610113a..0ae91b3a7406 100644
> > > --- a/include/linux/srcutree.h
> > > +++ b/include/linux/srcutree.h
> > > @@ -94,7 +94,6 @@ struct srcu_struct {
> > > /* callback for the barrier */
> > > /* operation. */
> > > struct delayed_work work;
> > > - struct list_head srcu_boot_entry; /* Early-boot callbacks. */
> > > #ifdef CONFIG_DEBUG_LOCK_ALLOC
> > > struct lockdep_map dep_map;
> > > #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
> > > @@ -110,7 +109,7 @@ struct srcu_struct {
> > > .sda = &pcpu_name, \
> > > .lock = __SPIN_LOCK_UNLOCKED(name.lock), \
> > > .srcu_gp_seq_needed = -1UL, \
> > > - .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
> > > + .work = __DELAYED_WORK_INITIALIZER(name.work, NULL, 0), \
> >
> > Thanks!
>
> Glad you like it! Does it actually work for you? ;-)

Oh, you want me to actually test it too? ;-)

I'll try to add that in my todo list tomorrow.

-- Steve

>
> Thanx, Paul
>
> > -- Steve
> >
> > > __SRCU_DEP_MAP_INIT(name) \
> > > }
> > >
> > > diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
> > > index d233f0c63f6f..b46e6683f8c9 100644
> > > --- a/kernel/rcu/srcutiny.c
> > > +++ b/kernel/rcu/srcutiny.c
> > > @@ -48,7 +48,7 @@ static int init_srcu_struct_fields(struct srcu_struct *sp)
> > > sp->srcu_gp_waiting = false;
> > > sp->srcu_idx = 0;
> > > INIT_WORK(&sp->srcu_work, srcu_drive_gp);
> > > - INIT_LIST_HEAD(&sp->srcu_boot_entry);
> > > + INIT_LIST_HEAD(&sp->srcu_work.entry);
> > > return 0;
> > > }
> > >
> > > @@ -185,8 +185,8 @@ void call_srcu(struct srcu_struct *sp, struct rcu_head *rhp,
> > > if (!READ_ONCE(sp->srcu_gp_running)) {
> > > if (likely(srcu_init_done))
> > > schedule_work(&sp->srcu_work);
> > > - else if (list_empty(&sp->srcu_boot_entry))
> > > - list_add(&sp->srcu_boot_entry, &srcu_boot_list);
> > > + else if (list_empty(&sp->srcu_work.entry))
> > > + list_add(&sp->srcu_work.entry, &srcu_boot_list);
> > > }
> > > }
> > > EXPORT_SYMBOL_GPL(call_srcu);
> > > @@ -224,8 +224,8 @@ void __init srcu_init(void)
> > > srcu_init_done = true;
> > > while (!list_empty(&srcu_boot_list)) {
> > > sp = list_first_entry(&srcu_boot_list,
> > > - struct srcu_struct, srcu_boot_entry);
> > > - list_del_init(&sp->srcu_boot_entry);
> > > + struct srcu_struct, srcu_work.entry);
> > > + list_del_init(&sp->srcu_work.entry);
> > > schedule_work(&sp->srcu_work);
> > > }
> > > }
> > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > > index e79c1929328f..36a2857c84e0 100644
> > > --- a/kernel/rcu/srcutree.c
> > > +++ b/kernel/rcu/srcutree.c
> > > @@ -186,7 +186,6 @@ static int init_srcu_struct_fields(struct srcu_struct *sp, bool is_static)
> > > mutex_init(&sp->srcu_barrier_mutex);
> > > atomic_set(&sp->srcu_barrier_cpu_cnt, 0);
> > > INIT_DELAYED_WORK(&sp->work, process_srcu);
> > > - INIT_LIST_HEAD(&sp->srcu_boot_entry);
> > > if (!is_static)
> > > sp->sda = alloc_percpu(struct srcu_data);
> > > init_srcu_struct_nodes(sp, is_static);
> > > @@ -709,8 +708,8 @@ static void srcu_funnel_gp_start(struct srcu_struct *sp, struct srcu_data *sdp,
> > > if (likely(srcu_init_done))
> > > queue_delayed_work(rcu_gp_wq, &sp->work,
> > > srcu_get_delay(sp));
> > > - else if (list_empty(&sp->srcu_boot_entry))
> > > - list_add(&sp->srcu_boot_entry, &srcu_boot_list);
> > > + else if (list_empty(&sp->work.work.entry))
> > > + list_add(&sp->work.work.entry, &srcu_boot_list);
> > > }
> > > spin_unlock_irqrestore_rcu_node(sp, flags);
> > > }
> > > @@ -1324,10 +1323,10 @@ void __init srcu_init(void)
> > >
> > > srcu_init_done = true;
> > > while (!list_empty(&srcu_boot_list)) {
> > > - sp = list_first_entry(&srcu_boot_list,
> > > - struct srcu_struct, srcu_boot_entry);
> > > + sp = list_first_entry(&srcu_boot_list, struct srcu_struct,
> > > + work.work.entry);
> > > check_init_srcu_struct(sp);
> > > - list_del_init(&sp->srcu_boot_entry);
> > > + list_del_init(&sp->work.work.entry);
> > > queue_work(rcu_gp_wq, &sp->work.work);
> > > }
> > > }
> >


2018-08-30 03:24:54

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

On Wed, Aug 29, 2018 at 10:46:09PM -0400, Steven Rostedt wrote:
> On Wed, 29 Aug 2018 19:07:01 -0700
> "Paul E. McKenney" <[email protected]> wrote:
>
> > On Wed, Aug 29, 2018 at 09:56:16PM -0400, Steven Rostedt wrote:
> > > On Wed, 29 Aug 2018 14:23:13 -0700
> > > "Paul E. McKenney" <[email protected]> wrote:
> > >
> > > > Allocating a list_head structure that is almost never used, and, when
> > > > used, is used only during early boot (rcu_init() and earlier), is a bit
> > > > wasteful. This commit therefore eliminates that list_head in favor of
> > > > the one in the work_struct structure. This is safe because the work_struct
> > > > structure cannot be used until after rcu_init() returns.
> > > >
> > > > Reported-by: Steven Rostedt <[email protected]>
> > > > Signed-off-by: Paul E. McKenney <[email protected]>
> > > > Cc: Tejun Heo <[email protected]>
> > > > Cc: Lai Jiangshan <[email protected]>
> > > > ---
> > > > include/linux/srcutiny.h | 2 --
> > > > include/linux/srcutree.h | 3 +--
> > > > kernel/rcu/srcutiny.c | 10 +++++-----
> > > > kernel/rcu/srcutree.c | 11 +++++------
> > > > 4 files changed, 11 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
> > > > index 2b5c0822e683..f41d2fb09f87 100644
> > > > --- a/include/linux/srcutiny.h
> > > > +++ b/include/linux/srcutiny.h
> > > > @@ -36,7 +36,6 @@ struct srcu_struct {
> > > > struct rcu_head *srcu_cb_head; /* Pending callbacks: Head. */
> > > > struct rcu_head **srcu_cb_tail; /* Pending callbacks: Tail. */
> > > > struct work_struct srcu_work; /* For driving grace periods. */
> > > > - struct list_head srcu_boot_entry; /* Early-boot callbacks. */
> > > > #ifdef CONFIG_DEBUG_LOCK_ALLOC
> > > > struct lockdep_map dep_map;
> > > > #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
> > > > @@ -49,7 +48,6 @@ void srcu_drive_gp(struct work_struct *wp);
> > > > .srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq), \
> > > > .srcu_cb_tail = &name.srcu_cb_head, \
> > > > .srcu_work = __WORK_INITIALIZER(name.srcu_work, srcu_drive_gp), \
> > > > - .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
> > > > __SRCU_DEP_MAP_INIT(name) \
> > > > }
> > > >
> > > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
> > > > index 9cfa4610113a..0ae91b3a7406 100644
> > > > --- a/include/linux/srcutree.h
> > > > +++ b/include/linux/srcutree.h
> > > > @@ -94,7 +94,6 @@ struct srcu_struct {
> > > > /* callback for the barrier */
> > > > /* operation. */
> > > > struct delayed_work work;
> > > > - struct list_head srcu_boot_entry; /* Early-boot callbacks. */
> > > > #ifdef CONFIG_DEBUG_LOCK_ALLOC
> > > > struct lockdep_map dep_map;
> > > > #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
> > > > @@ -110,7 +109,7 @@ struct srcu_struct {
> > > > .sda = &pcpu_name, \
> > > > .lock = __SPIN_LOCK_UNLOCKED(name.lock), \
> > > > .srcu_gp_seq_needed = -1UL, \
> > > > - .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
> > > > + .work = __DELAYED_WORK_INITIALIZER(name.work, NULL, 0), \
> > >
> > > Thanks!
> >
> > Glad you like it! Does it actually work for you? ;-)
>
> Oh, you want me to actually test it too? ;-)

;-) ;-) ;-)

> I'll try to add that in my todo list tomorrow.

Much appreciated!

Thanx, Paul

> -- Steve
>
> >
> > Thanx, Paul
> >
> > > -- Steve
> > >
> > > > __SRCU_DEP_MAP_INIT(name) \
> > > > }
> > > >
> > > > diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
> > > > index d233f0c63f6f..b46e6683f8c9 100644
> > > > --- a/kernel/rcu/srcutiny.c
> > > > +++ b/kernel/rcu/srcutiny.c
> > > > @@ -48,7 +48,7 @@ static int init_srcu_struct_fields(struct srcu_struct *sp)
> > > > sp->srcu_gp_waiting = false;
> > > > sp->srcu_idx = 0;
> > > > INIT_WORK(&sp->srcu_work, srcu_drive_gp);
> > > > - INIT_LIST_HEAD(&sp->srcu_boot_entry);
> > > > + INIT_LIST_HEAD(&sp->srcu_work.entry);
> > > > return 0;
> > > > }
> > > >
> > > > @@ -185,8 +185,8 @@ void call_srcu(struct srcu_struct *sp, struct rcu_head *rhp,
> > > > if (!READ_ONCE(sp->srcu_gp_running)) {
> > > > if (likely(srcu_init_done))
> > > > schedule_work(&sp->srcu_work);
> > > > - else if (list_empty(&sp->srcu_boot_entry))
> > > > - list_add(&sp->srcu_boot_entry, &srcu_boot_list);
> > > > + else if (list_empty(&sp->srcu_work.entry))
> > > > + list_add(&sp->srcu_work.entry, &srcu_boot_list);
> > > > }
> > > > }
> > > > EXPORT_SYMBOL_GPL(call_srcu);
> > > > @@ -224,8 +224,8 @@ void __init srcu_init(void)
> > > > srcu_init_done = true;
> > > > while (!list_empty(&srcu_boot_list)) {
> > > > sp = list_first_entry(&srcu_boot_list,
> > > > - struct srcu_struct, srcu_boot_entry);
> > > > - list_del_init(&sp->srcu_boot_entry);
> > > > + struct srcu_struct, srcu_work.entry);
> > > > + list_del_init(&sp->srcu_work.entry);
> > > > schedule_work(&sp->srcu_work);
> > > > }
> > > > }
> > > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > > > index e79c1929328f..36a2857c84e0 100644
> > > > --- a/kernel/rcu/srcutree.c
> > > > +++ b/kernel/rcu/srcutree.c
> > > > @@ -186,7 +186,6 @@ static int init_srcu_struct_fields(struct srcu_struct *sp, bool is_static)
> > > > mutex_init(&sp->srcu_barrier_mutex);
> > > > atomic_set(&sp->srcu_barrier_cpu_cnt, 0);
> > > > INIT_DELAYED_WORK(&sp->work, process_srcu);
> > > > - INIT_LIST_HEAD(&sp->srcu_boot_entry);
> > > > if (!is_static)
> > > > sp->sda = alloc_percpu(struct srcu_data);
> > > > init_srcu_struct_nodes(sp, is_static);
> > > > @@ -709,8 +708,8 @@ static void srcu_funnel_gp_start(struct srcu_struct *sp, struct srcu_data *sdp,
> > > > if (likely(srcu_init_done))
> > > > queue_delayed_work(rcu_gp_wq, &sp->work,
> > > > srcu_get_delay(sp));
> > > > - else if (list_empty(&sp->srcu_boot_entry))
> > > > - list_add(&sp->srcu_boot_entry, &srcu_boot_list);
> > > > + else if (list_empty(&sp->work.work.entry))
> > > > + list_add(&sp->work.work.entry, &srcu_boot_list);
> > > > }
> > > > spin_unlock_irqrestore_rcu_node(sp, flags);
> > > > }
> > > > @@ -1324,10 +1323,10 @@ void __init srcu_init(void)
> > > >
> > > > srcu_init_done = true;
> > > > while (!list_empty(&srcu_boot_list)) {
> > > > - sp = list_first_entry(&srcu_boot_list,
> > > > - struct srcu_struct, srcu_boot_entry);
> > > > + sp = list_first_entry(&srcu_boot_list, struct srcu_struct,
> > > > + work.work.entry);
> > > > check_init_srcu_struct(sp);
> > > > - list_del_init(&sp->srcu_boot_entry);
> > > > + list_del_init(&sp->work.work.entry);
> > > > queue_work(rcu_gp_wq, &sp->work.work);
> > > > }
> > > > }
> > >
>


2018-08-30 16:46:24

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

On Wed, 29 Aug 2018 20:23:15 -0700
"Paul E. McKenney" <[email protected]> wrote:

> > > Glad you like it! Does it actually work for you? ;-)
> >
> > Oh, you want me to actually test it too? ;-)
>
> ;-) ;-) ;-)
>
> > I'll try to add that in my todo list tomorrow.
>
> Much appreciated!

I reverted the change that prevents calling call_srcu() early:

I checked out v4.19-rc1 and applied these three patches, then did:

git show f8a79d5c7ef47c62d97a30e16064caf2ef91f648 | patch -p1 -R

But still triggered the following:

WARNING: CPU: 0 PID: 0 at /work/git/linux-trace.git/kernel/rcu/srcutree.c:242 check_init_srcu_struct+0x85/0x90
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.19.0-rc1-test+ #1194
Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016
RIP: 0010:check_init_srcu_struct+0x85/0x90
Code: 16 25 00 f6 83 70 06 00 00 03 74 0d be 01 00 00 00 48 89 df e8 6c f6 ff ff 5b 4c 89 ee 4c 89 e7 5d 41 5c 41 5d e9 db 4a d7 00 <0f> 0b eb 9a 0f 1f 80 00 00 00 00 0f 1f 44 00 00 41 57 48 8d 87 90
RSP: 0000:ffff8800d3c07d90 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffffffffb9d4d960 RCX: ffffffffb81dab0a
RDX: 0000000000000007 RSI: dffffc0000000000 RDI: ffffffffba1cd854
RBP: ffffffffb827de70 R08: fffffbfff76a9525 R09: fffffbfff76a9524
R10: ffff8800d3c07db0 R11: fffffbfff76a9525 R12: ffff8800cfacba88
R13: ffffffffb9d4d960 R14: 000000000000000a R15: ffff8800d3c07eb0
FS: 0000000000000000(0000) GS:ffff8800d3c00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffff88011e7ff000 CR3: 00000000a2c14001 CR4: 00000000001606f0
Call Trace:
<IRQ>
? rcu_free_old_probes+0x20/0x20
? for_each_kernel_tracepoint+0x50/0x50
__call_srcu+0x29/0x570
? rcu_process_callbacks+0x403/0xcd0
? for_each_kernel_tracepoint+0x50/0x50
rcu_process_callbacks+0x44f/0xcd0
? __bpf_trace_timer_class+0x10/0x10
? sched_clock+0x5/0x10
? note_gp_changes+0xf0/0xf0
? __lock_is_held+0x26/0xf0
__do_softirq+0x13b/0x561
irq_exit+0x12c/0x140
smp_apic_timer_interrupt+0xd4/0x2f0
apic_timer_interrupt+0xf/0x20
</IRQ>
RIP: 0010:mwait_idle+0x83/0x260
Code: 48 89 d1 48 89 d8 0f 01 c8 48 89 df e8 f6 d5 4d ff 48 8b 03 a8 08 0f 85 c0 01 00 00 e8 f6 dc 35 ff 31 c0 48 89 c1 fb 0f 01 c9 <e8> 68 ee 85 ff 41 89 c4 0f 1f 44 00 00 65 48 8b 04 25 c0 ed 01 00
RSP: 0000:ffffffffb9c07d28 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: 0000000000000000 RBX: ffffffffb9c1f9c0 RCX: 0000000000000000
RDX: 0000000000000007 RSI: dffffc0000000000 RDI: ffffffffb9c2028c
RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: ffffffffba1cd740 R14: 0000000000000000 R15: 0000000000000000
? mwait_idle+0x7a/0x260
default_idle_call+0x2d/0x50
do_idle+0x251/0x310
? arch_cpu_idle_exit+0x40/0x40
cpu_startup_entry+0xc2/0xd0
? cpu_in_idle+0x20/0x20
? preempt_count_sub+0xaa/0x100
start_kernel+0x640/0x67d
? thread_stack_cache_init+0x6/0x6
? load_ucode_intel_bsp+0x5f/0xa5
? load_ucode_intel_bsp+0x5f/0xa5
? init_intel_microcode+0xb0/0xb0
? load_ucode_bsp+0xbb/0x156
secondary_startup_64+0xa4/0xb0
irq event stamp: 21271
hardirqs last enabled at (21270): [<ffffffffb81e1e52>] rcu_process_callbacks+0x982/0xcd0
hardirqs last disabled at (21271): [<ffffffffb800447f>] trace_hardirqs_off_thunk+0x1a/0x1c
softirqs last enabled at (21252): [<ffffffffb811529c>] irq_enter+0x7c/0x80
softirqs last disabled at (21253): [<ffffffffb81153cc>] irq_exit+0x12c/0x140
---[ end trace 64bab84b86e8ec96 ]---

Attached is my config, and my kernel command line has:

trace_event=sched_switch

-- Steve


Attachments:
(No filename) (3.70 kB)
config (131.64 kB)
Download all attachments

2018-08-30 17:36:56

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

On Thu, Aug 30, 2018 at 12:44:44PM -0400, Steven Rostedt wrote:
> On Wed, 29 Aug 2018 20:23:15 -0700
> "Paul E. McKenney" <[email protected]> wrote:
>
> > > > Glad you like it! Does it actually work for you? ;-)
> > >
> > > Oh, you want me to actually test it too? ;-)
> >
> > ;-) ;-) ;-)
> >
> > > I'll try to add that in my todo list tomorrow.
> >
> > Much appreciated!
>
> I reverted the change that prevents calling call_srcu() early:
>
> I checked out v4.19-rc1 and applied these three patches, then did:
>
> git show f8a79d5c7ef47c62d97a30e16064caf2ef91f648 | patch -p1 -R
>
> But still triggered the following:
>
> WARNING: CPU: 0 PID: 0 at /work/git/linux-trace.git/kernel/rcu/srcutree.c:242 check_init_srcu_struct+0x85/0x90

Gah!!! I needed to have removed that WARN_ON_ONCE(), didn't I?
In fact, I should have removed that once I started using workqueues,
quite some time back.

Thank you even more for testing this!!! Glad I asked. ;-)

Now to figure out why my testing didn't hit this... I do invoke
call_srcu() at rcu_init() time and verify that the callback is
eventually invoked. A printk() placed there really does print...

Thanx, Paul

> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.19.0-rc1-test+ #1194
> Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016
> RIP: 0010:check_init_srcu_struct+0x85/0x90
> Code: 16 25 00 f6 83 70 06 00 00 03 74 0d be 01 00 00 00 48 89 df e8 6c f6 ff ff 5b 4c 89 ee 4c 89 e7 5d 41 5c 41 5d e9 db 4a d7 00 <0f> 0b eb 9a 0f 1f 80 00 00 00 00 0f 1f 44 00 00 41 57 48 8d 87 90
> RSP: 0000:ffff8800d3c07d90 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffffffffb9d4d960 RCX: ffffffffb81dab0a
> RDX: 0000000000000007 RSI: dffffc0000000000 RDI: ffffffffba1cd854
> RBP: ffffffffb827de70 R08: fffffbfff76a9525 R09: fffffbfff76a9524
> R10: ffff8800d3c07db0 R11: fffffbfff76a9525 R12: ffff8800cfacba88
> R13: ffffffffb9d4d960 R14: 000000000000000a R15: ffff8800d3c07eb0
> FS: 0000000000000000(0000) GS:ffff8800d3c00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffff88011e7ff000 CR3: 00000000a2c14001 CR4: 00000000001606f0
> Call Trace:
> <IRQ>
> ? rcu_free_old_probes+0x20/0x20
> ? for_each_kernel_tracepoint+0x50/0x50
> __call_srcu+0x29/0x570
> ? rcu_process_callbacks+0x403/0xcd0
> ? for_each_kernel_tracepoint+0x50/0x50
> rcu_process_callbacks+0x44f/0xcd0
> ? __bpf_trace_timer_class+0x10/0x10
> ? sched_clock+0x5/0x10
> ? note_gp_changes+0xf0/0xf0
> ? __lock_is_held+0x26/0xf0
> __do_softirq+0x13b/0x561
> irq_exit+0x12c/0x140
> smp_apic_timer_interrupt+0xd4/0x2f0
> apic_timer_interrupt+0xf/0x20
> </IRQ>
> RIP: 0010:mwait_idle+0x83/0x260
> Code: 48 89 d1 48 89 d8 0f 01 c8 48 89 df e8 f6 d5 4d ff 48 8b 03 a8 08 0f 85 c0 01 00 00 e8 f6 dc 35 ff 31 c0 48 89 c1 fb 0f 01 c9 <e8> 68 ee 85 ff 41 89 c4 0f 1f 44 00 00 65 48 8b 04 25 c0 ed 01 00
> RSP: 0000:ffffffffb9c07d28 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
> RAX: 0000000000000000 RBX: ffffffffb9c1f9c0 RCX: 0000000000000000
> RDX: 0000000000000007 RSI: dffffc0000000000 RDI: ffffffffb9c2028c
> RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> R13: ffffffffba1cd740 R14: 0000000000000000 R15: 0000000000000000
> ? mwait_idle+0x7a/0x260
> default_idle_call+0x2d/0x50
> do_idle+0x251/0x310
> ? arch_cpu_idle_exit+0x40/0x40
> cpu_startup_entry+0xc2/0xd0
> ? cpu_in_idle+0x20/0x20
> ? preempt_count_sub+0xaa/0x100
> start_kernel+0x640/0x67d
> ? thread_stack_cache_init+0x6/0x6
> ? load_ucode_intel_bsp+0x5f/0xa5
> ? load_ucode_intel_bsp+0x5f/0xa5
> ? init_intel_microcode+0xb0/0xb0
> ? load_ucode_bsp+0xbb/0x156
> secondary_startup_64+0xa4/0xb0
> irq event stamp: 21271
> hardirqs last enabled at (21270): [<ffffffffb81e1e52>] rcu_process_callbacks+0x982/0xcd0
> hardirqs last disabled at (21271): [<ffffffffb800447f>] trace_hardirqs_off_thunk+0x1a/0x1c
> softirqs last enabled at (21252): [<ffffffffb811529c>] irq_enter+0x7c/0x80
> softirqs last disabled at (21253): [<ffffffffb81153cc>] irq_exit+0x12c/0x140
> ---[ end trace 64bab84b86e8ec96 ]---
>
> Attached is my config, and my kernel command line has:
>
> trace_event=sched_switch
>
> -- Steve



2018-08-30 17:39:34

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

On Thu, Aug 30, 2018 at 10:35:09AM -0700, Paul E. McKenney wrote:
> On Thu, Aug 30, 2018 at 12:44:44PM -0400, Steven Rostedt wrote:
> > On Wed, 29 Aug 2018 20:23:15 -0700
> > "Paul E. McKenney" <[email protected]> wrote:
> >
> > > > > Glad you like it! Does it actually work for you? ;-)
> > > >
> > > > Oh, you want me to actually test it too? ;-)
> > >
> > > ;-) ;-) ;-)
> > >
> > > > I'll try to add that in my todo list tomorrow.
> > >
> > > Much appreciated!
> >
> > I reverted the change that prevents calling call_srcu() early:
> >
> > I checked out v4.19-rc1 and applied these three patches, then did:
> >
> > git show f8a79d5c7ef47c62d97a30e16064caf2ef91f648 | patch -p1 -R
> >
> > But still triggered the following:
> >
> > WARNING: CPU: 0 PID: 0 at /work/git/linux-trace.git/kernel/rcu/srcutree.c:242 check_init_srcu_struct+0x85/0x90
>
> Gah!!! I needed to have removed that WARN_ON_ONCE(), didn't I?
> In fact, I should have removed that once I started using workqueues,
> quite some time back.

As in the below patch.

Thanx, Paul

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

diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 75571ff09c62..a8846ed7f352 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -239,7 +239,6 @@ static void check_init_srcu_struct(struct srcu_struct *sp)
{
unsigned long flags;

- WARN_ON_ONCE(rcu_scheduler_active == RCU_SCHEDULER_INIT);
/* The smp_load_acquire() pairs with the smp_store_release(). */
if (!rcu_seq_state(smp_load_acquire(&sp->srcu_gp_seq_needed))) /*^^^*/
return; /* Already initialized. */


2018-08-30 18:26:11

by Paul E. McKenney

[permalink] [raw]
Subject: [PATCH tip/core/rcu v2 1/3] srcu: Make call_srcu() available during very early boot

[ Updated to remove obsolete warning. ]

Event tracing is moving to SRCU in order to take advantage of the fact
that SRCU may be safely used from idle and even offline CPUs. However,
event tracing can invoke call_srcu() very early in the boot process,
even before workqueue_init_early() is invoked (let alone rcu_init()).
Therefore, call_srcu()'s attempts to queue work fail miserably.

This commit therefore detects this situation, and refrains from attempting
to queue work before rcu_init() time, but does everything else that it
would have done, and in addition, adds the srcu_struct to a global list.
The rcu_init() function now invokes a new srcu_init() function, which
is empty if CONFIG_SRCU=n. Otherwise, srcu_init() queues work for
each srcu_struct on the list. This all happens early enough in boot
that there is but a single CPU with interrupts disabled, which allows
synchronization to be dispensed with.

Of course, the queued work won't actually be invoked until after
workqueue_init() is invoked, which happens shortly after the scheduler
is up and running. This means that although call_srcu() may be invoked
any time after per-CPU variables have been set up, there is still a very
narrow window when synchronize_srcu() won't work, and this window
extends from the time that the scheduler starts until the time that
workqueue_init() returns. This can be fixed in a manner similar to
the fix for synchronize_rcu_expedited() and friends, but until someone
actually needs to use synchronize_srcu() during this window, this fix
is added churn for no benefit.

Finally, note that Tree SRCU's new srcu_init() function invokes
queue_work() rather than the queue_delayed_work() function that is
invoked post-boot. The reason is that queue_delayed_work() will (as you
would expect) post a timer, and timers have not yet been initialized.
So use of queue_work() avoids the complaints about use of uninitialized
spinlocks that would otherwise result. Besides, some delay is already
provide by the aforementioned fact that the queued work won't actually
be invoked until after the scheduler is up and running.

Requested-by: Steven Rostedt <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>

diff --git a/include/linux/srcutiny.h b/include/linux/srcutiny.h
index f41d2fb09f87..2b5c0822e683 100644
--- a/include/linux/srcutiny.h
+++ b/include/linux/srcutiny.h
@@ -36,6 +36,7 @@ struct srcu_struct {
struct rcu_head *srcu_cb_head; /* Pending callbacks: Head. */
struct rcu_head **srcu_cb_tail; /* Pending callbacks: Tail. */
struct work_struct srcu_work; /* For driving grace periods. */
+ struct list_head srcu_boot_entry; /* Early-boot callbacks. */
#ifdef CONFIG_DEBUG_LOCK_ALLOC
struct lockdep_map dep_map;
#endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
@@ -48,6 +49,7 @@ void srcu_drive_gp(struct work_struct *wp);
.srcu_wq = __SWAIT_QUEUE_HEAD_INITIALIZER(name.srcu_wq), \
.srcu_cb_tail = &name.srcu_cb_head, \
.srcu_work = __WORK_INITIALIZER(name.srcu_work, srcu_drive_gp), \
+ .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
__SRCU_DEP_MAP_INIT(name) \
}

diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h
index 745d4ca4dd50..9cfa4610113a 100644
--- a/include/linux/srcutree.h
+++ b/include/linux/srcutree.h
@@ -94,6 +94,7 @@ struct srcu_struct {
/* callback for the barrier */
/* operation. */
struct delayed_work work;
+ struct list_head srcu_boot_entry; /* Early-boot callbacks. */
#ifdef CONFIG_DEBUG_LOCK_ALLOC
struct lockdep_map dep_map;
#endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
@@ -105,12 +106,13 @@ struct srcu_struct {
#define SRCU_STATE_SCAN2 2

#define __SRCU_STRUCT_INIT(name, pcpu_name) \
- { \
- .sda = &pcpu_name, \
- .lock = __SPIN_LOCK_UNLOCKED(name.lock), \
- .srcu_gp_seq_needed = 0 - 1, \
- __SRCU_DEP_MAP_INIT(name) \
- }
+{ \
+ .sda = &pcpu_name, \
+ .lock = __SPIN_LOCK_UNLOCKED(name.lock), \
+ .srcu_gp_seq_needed = -1UL, \
+ .srcu_boot_entry = LIST_HEAD_INIT(name.srcu_boot_entry), \
+ __SRCU_DEP_MAP_INIT(name) \
+}

/*
* Define and initialize a srcu struct at build time.
diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index 4d04683c31b2..e1b5aec5ec1c 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -435,6 +435,12 @@ do { \

#endif /* #if defined(SRCU) || !defined(TINY_RCU) */

+#ifdef CONFIG_SRCU
+void srcu_init(void);
+#else /* #ifdef CONFIG_SRCU */
+static inline void srcu_init(void) { }
+#endif /* #else #ifdef CONFIG_SRCU */
+
#ifdef CONFIG_TINY_RCU
/* Tiny RCU doesn't expedite, as its purpose in life is instead to be tiny. */
static inline bool rcu_gp_is_normal(void) { return true; }
diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
index 04fc2ed71af8..d233f0c63f6f 100644
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -34,6 +34,8 @@
#include "rcu.h"

int rcu_scheduler_active __read_mostly;
+static LIST_HEAD(srcu_boot_list);
+static bool srcu_init_done;

static int init_srcu_struct_fields(struct srcu_struct *sp)
{
@@ -46,6 +48,7 @@ static int init_srcu_struct_fields(struct srcu_struct *sp)
sp->srcu_gp_waiting = false;
sp->srcu_idx = 0;
INIT_WORK(&sp->srcu_work, srcu_drive_gp);
+ INIT_LIST_HEAD(&sp->srcu_boot_entry);
return 0;
}

@@ -179,8 +182,12 @@ void call_srcu(struct srcu_struct *sp, struct rcu_head *rhp,
*sp->srcu_cb_tail = rhp;
sp->srcu_cb_tail = &rhp->next;
local_irq_restore(flags);
- if (!READ_ONCE(sp->srcu_gp_running))
- schedule_work(&sp->srcu_work);
+ if (!READ_ONCE(sp->srcu_gp_running)) {
+ if (likely(srcu_init_done))
+ schedule_work(&sp->srcu_work);
+ else if (list_empty(&sp->srcu_boot_entry))
+ list_add(&sp->srcu_boot_entry, &srcu_boot_list);
+ }
}
EXPORT_SYMBOL_GPL(call_srcu);

@@ -204,3 +211,21 @@ void __init rcu_scheduler_starting(void)
{
rcu_scheduler_active = RCU_SCHEDULER_RUNNING;
}
+
+/*
+ * Queue work for srcu_struct structures with early boot callbacks.
+ * The work won't actually execute until the workqueue initialization
+ * phase that takes place after the scheduler starts.
+ */
+void __init srcu_init(void)
+{
+ struct srcu_struct *sp;
+
+ srcu_init_done = true;
+ while (!list_empty(&srcu_boot_list)) {
+ sp = list_first_entry(&srcu_boot_list,
+ struct srcu_struct, srcu_boot_entry);
+ list_del_init(&sp->srcu_boot_entry);
+ schedule_work(&sp->srcu_work);
+ }
+}
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 6c9866a854b1..2e7f6b460150 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -51,6 +51,10 @@ module_param(exp_holdoff, ulong, 0444);
static ulong counter_wrap_check = (ULONG_MAX >> 2);
module_param(counter_wrap_check, ulong, 0444);

+/* Early-boot callback-management, so early that no lock is required! */
+static LIST_HEAD(srcu_boot_list);
+static bool __read_mostly srcu_init_done;
+
static void srcu_invoke_callbacks(struct work_struct *work);
static void srcu_reschedule(struct srcu_struct *sp, unsigned long delay);
static void process_srcu(struct work_struct *work);
@@ -182,6 +186,7 @@ static int init_srcu_struct_fields(struct srcu_struct *sp, bool is_static)
mutex_init(&sp->srcu_barrier_mutex);
atomic_set(&sp->srcu_barrier_cpu_cnt, 0);
INIT_DELAYED_WORK(&sp->work, process_srcu);
+ INIT_LIST_HEAD(&sp->srcu_boot_entry);
if (!is_static)
sp->sda = alloc_percpu(struct srcu_data);
init_srcu_struct_nodes(sp, is_static);
@@ -235,7 +240,6 @@ static void check_init_srcu_struct(struct srcu_struct *sp)
{
unsigned long flags;

- WARN_ON_ONCE(rcu_scheduler_active == RCU_SCHEDULER_INIT);
/* The smp_load_acquire() pairs with the smp_store_release(). */
if (!rcu_seq_state(smp_load_acquire(&sp->srcu_gp_seq_needed))) /*^^^*/
return; /* Already initialized. */
@@ -701,7 +705,11 @@ static void srcu_funnel_gp_start(struct srcu_struct *sp, struct srcu_data *sdp,
rcu_seq_state(sp->srcu_gp_seq) == SRCU_STATE_IDLE) {
WARN_ON_ONCE(ULONG_CMP_GE(sp->srcu_gp_seq, sp->srcu_gp_seq_needed));
srcu_gp_start(sp);
- queue_delayed_work(rcu_gp_wq, &sp->work, srcu_get_delay(sp));
+ if (likely(srcu_init_done))
+ queue_delayed_work(rcu_gp_wq, &sp->work,
+ srcu_get_delay(sp));
+ else if (list_empty(&sp->srcu_boot_entry))
+ list_add(&sp->srcu_boot_entry, &srcu_boot_list);
}
spin_unlock_irqrestore_rcu_node(sp, flags);
}
@@ -1308,3 +1316,17 @@ static int __init srcu_bootup_announce(void)
return 0;
}
early_initcall(srcu_bootup_announce);
+
+void __init srcu_init(void)
+{
+ struct srcu_struct *sp;
+
+ srcu_init_done = true;
+ while (!list_empty(&srcu_boot_list)) {
+ sp = list_first_entry(&srcu_boot_list,
+ struct srcu_struct, srcu_boot_entry);
+ check_init_srcu_struct(sp);
+ list_del_init(&sp->srcu_boot_entry);
+ queue_work(rcu_gp_wq, &sp->work.work);
+ }
+}
diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
index befc9321a89c..101ed5bb836c 100644
--- a/kernel/rcu/tiny.c
+++ b/kernel/rcu/tiny.c
@@ -236,4 +236,5 @@ void __init rcu_init(void)
{
open_softirq(RCU_SOFTIRQ, rcu_process_callbacks);
rcu_early_boot_tests();
+ srcu_init();
}
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 0b760c1369f7..43c806291208 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -4164,6 +4164,7 @@ void __init rcu_init(void)
WARN_ON(!rcu_gp_wq);
rcu_par_gp_wq = alloc_workqueue("rcu_par_gp", WQ_MEM_RECLAIM, 0);
WARN_ON(!rcu_par_gp_wq);
+ srcu_init();
}

#include "tree_exp.h"
diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index 39cb23d22109..7d057d0aaec4 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -888,11 +888,16 @@ static void test_callback(struct rcu_head *r)
pr_info("RCU test callback executed %d\n", rcu_self_test_counter);
}

+DEFINE_STATIC_SRCU(early_srcu);
+
static void early_boot_test_call_rcu(void)
{
static struct rcu_head head;
+ static struct rcu_head shead;

call_rcu(&head, test_callback);
+ if (IS_ENABLED(CONFIG_SRCU))
+ call_srcu(&early_srcu, &shead, test_callback);
}

static void early_boot_test_call_rcu_bh(void)
@@ -930,6 +935,10 @@ static int rcu_verify_early_boot_tests(void)
if (rcu_self_test) {
early_boot_test_counter++;
rcu_barrier();
+ if (IS_ENABLED(CONFIG_SRCU)) {
+ early_boot_test_counter++;
+ srcu_barrier(&early_srcu);
+ }
}
if (rcu_self_test_bh) {
early_boot_test_counter++;


2018-08-30 18:37:48

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

On Thu, 30 Aug 2018 10:37:42 -0700
"Paul E. McKenney" <[email protected]> wrote:


> > > But still triggered the following:
> > >
> > > WARNING: CPU: 0 PID: 0 at /work/git/linux-trace.git/kernel/rcu/srcutree.c:242 check_init_srcu_struct+0x85/0x90
> >
> > Gah!!! I needed to have removed that WARN_ON_ONCE(), didn't I?
> > In fact, I should have removed that once I started using workqueues,
> > quite some time back.
>
> As in the below patch.
>
> Thanx, Paul
>
> ------------------------------------------------------------------------
>
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index 75571ff09c62..a8846ed7f352 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -239,7 +239,6 @@ static void check_init_srcu_struct(struct srcu_struct *sp)
> {
> unsigned long flags;
>
> - WARN_ON_ONCE(rcu_scheduler_active == RCU_SCHEDULER_INIT);

Well it boots without warning ;-)

Tested-by: Steven Rostedt (VMware) <[email protected]>

-- Steve

> /* The smp_load_acquire() pairs with the smp_store_release(). */
> if (!rcu_seq_state(smp_load_acquire(&sp->srcu_gp_seq_needed))) /*^^^*/
> return; /* Already initialized. */


2018-08-30 23:13:43

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

On Thu, Aug 30, 2018 at 02:36:17PM -0400, Steven Rostedt wrote:
> On Thu, 30 Aug 2018 10:37:42 -0700
> "Paul E. McKenney" <[email protected]> wrote:
>
>
> > > > But still triggered the following:
> > > >
> > > > WARNING: CPU: 0 PID: 0 at /work/git/linux-trace.git/kernel/rcu/srcutree.c:242 check_init_srcu_struct+0x85/0x90
> > >
> > > Gah!!! I needed to have removed that WARN_ON_ONCE(), didn't I?
> > > In fact, I should have removed that once I started using workqueues,
> > > quite some time back.
> >
> > As in the below patch.
> >
> > Thanx, Paul
> >
> > ------------------------------------------------------------------------
> >
> > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > index 75571ff09c62..a8846ed7f352 100644
> > --- a/kernel/rcu/srcutree.c
> > +++ b/kernel/rcu/srcutree.c
> > @@ -239,7 +239,6 @@ static void check_init_srcu_struct(struct srcu_struct *sp)
> > {
> > unsigned long flags;
> >
> > - WARN_ON_ONCE(rcu_scheduler_active == RCU_SCHEDULER_INIT);
>
> Well it boots without warning ;-)
>
> Tested-by: Steven Rostedt (VMware) <[email protected]>

Thank you again! I applied this to 1/3 and 3/3 on the assumption that
you did not run rcutorture. ;-)

Thanx, Paul

> -- Steve
>
> > /* The smp_load_acquire() pairs with the smp_store_release(). */
> > if (!rcu_seq_state(smp_load_acquire(&sp->srcu_gp_seq_needed))) /*^^^*/
> > return; /* Already initialized. */
>


2018-08-30 23:54:51

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

On Thu, 30 Aug 2018 16:12:11 -0700
"Paul E. McKenney" <[email protected]> wrote:


> > > - WARN_ON_ONCE(rcu_scheduler_active == RCU_SCHEDULER_INIT);
> >
> > Well it boots without warning ;-)
> >
> > Tested-by: Steven Rostedt (VMware) <[email protected]>
>
> Thank you again! I applied this to 1/3 and 3/3 on the assumption that
> you did not run rcutorture. ;-)
>

No rcutorture. Just reverted the tracepoint SRCU delay patch and it
boots with the trace event enabled.

-- Steve

2018-08-31 00:12:20

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

On Thu, Aug 30, 2018 at 07:53:22PM -0400, Steven Rostedt wrote:
> On Thu, 30 Aug 2018 16:12:11 -0700
> "Paul E. McKenney" <[email protected]> wrote:
>
>
> > > > - WARN_ON_ONCE(rcu_scheduler_active == RCU_SCHEDULER_INIT);
> > >
> > > Well it boots without warning ;-)
> > >
> > > Tested-by: Steven Rostedt (VMware) <[email protected]>
> >
> > Thank you again! I applied this to 1/3 and 3/3 on the assumption that
> > you did not run rcutorture. ;-)
>
> No rcutorture. Just reverted the tracepoint SRCU delay patch and it
> boots with the trace event enabled.

Very good, then I did get it right. 1/3 and 3/3, but not 2/3.

Thanx, Paul


2018-08-31 00:57:01

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH tip/core/rcu 3/3] srcu: Make early-boot call_srcu() reuse workqueue lists

On Thu, 30 Aug 2018 17:09:52 -0700
"Paul E. McKenney" <[email protected]> wrote:

> On Thu, Aug 30, 2018 at 07:53:22PM -0400, Steven Rostedt wrote:
> > On Thu, 30 Aug 2018 16:12:11 -0700
> > "Paul E. McKenney" <[email protected]> wrote:
> >
> >
> > > > > - WARN_ON_ONCE(rcu_scheduler_active == RCU_SCHEDULER_INIT);
> > > >
> > > > Well it boots without warning ;-)
> > > >
> > > > Tested-by: Steven Rostedt (VMware) <[email protected]>
> > >
> > > Thank you again! I applied this to 1/3 and 3/3 on the assumption that
> > > you did not run rcutorture. ;-)
> >
> > No rcutorture. Just reverted the tracepoint SRCU delay patch and it
> > boots with the trace event enabled.
>
> Very good, then I did get it right. 1/3 and 3/3, but not 2/3.
>

I did apply 2, but didn't enable rcutorture. I just applied the entire
series.

-- Steve