Hello!
This series contains RCU torture-test updates:
1. Make rcutorture support srcu double call test, courtesy of Zqiang.
2. Fix rcu_torture_fwd_cb_cr() data race.
3. Add missing MODULE_DESCRIPTION() macros, courtesy of Jeff Johnson.
4. Add rcu-updaters.sh script.
Thanx, Paul
------------------------------------------------------------------------
b/kernel/rcu/rcuscale.c | 1
b/kernel/rcu/rcutorture.c | 46 ++++++++++++++++++++++----------------
b/kernel/rcu/refscale.c | 1
b/tools/rcu/rcu-updaters.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++
kernel/rcu/rcutorture.c | 3 +-
5 files changed, 82 insertions(+), 21 deletions(-)
From: Jeff Johnson <[email protected]>
Fix the following 'make W=1' warnings:
WARNING: modpost: missing MODULE_DESCRIPTION() in kernel/rcu/rcutorture.o
WARNING: modpost: missing MODULE_DESCRIPTION() in kernel/rcu/rcuscale.o
WARNING: modpost: missing MODULE_DESCRIPTION() in kernel/rcu/refscale.o
Signed-off-by: Jeff Johnson <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
---
kernel/rcu/rcuscale.c | 1 +
kernel/rcu/rcutorture.c | 1 +
kernel/rcu/refscale.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
index 8db4fedaaa1eb..b53a9e8f5904f 100644
--- a/kernel/rcu/rcuscale.c
+++ b/kernel/rcu/rcuscale.c
@@ -42,6 +42,7 @@
#include "rcu.h"
+MODULE_DESCRIPTION("Read-Copy Update module-based scalability-test facility");
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Paul E. McKenney <[email protected]>");
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index cafe047d046e8..08bf7c669dd3d 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -51,6 +51,7 @@
#include "rcu.h"
+MODULE_DESCRIPTION("Read-Copy Update module-based torture test facility");
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Paul E. McKenney <[email protected]> and Josh Triplett <[email protected]>");
diff --git a/kernel/rcu/refscale.c b/kernel/rcu/refscale.c
index 2c2648a3ad306..f4ea5b1ec068a 100644
--- a/kernel/rcu/refscale.c
+++ b/kernel/rcu/refscale.c
@@ -63,6 +63,7 @@ do { \
#define SCALEOUT_ERRSTRING(s, x...) pr_alert("%s" SCALE_FLAG "!!! " s "\n", scale_type, ## x)
+MODULE_DESCRIPTION("Scalability test for object reference mechanisms");
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Joel Fernandes (Google) <[email protected]>");
--
2.40.1
This commit adds a tools/rcu/rcu-updaters.sh script that uses bpftrace
to print a histogram of the RCU update-side primitives invoked during
the specified time interval, or until manually terminated if no interval
is specified.
Sample output on an idle laptop:
@counts[poll_state_synchronize_rcu]: 6
@counts[synchronize_srcu]: 13
@counts[call_rcu_tasks_trace]: 25
@counts[synchronize_rcu]: 54
@counts[kvfree_call_rcu]: 428
@counts[call_rcu]: 2134
Note that when run on a kernel missing one or more of the symbols, this
script will issue a diagnostic for each that is not found, but continue
normally for the rest of the functions.
Signed-off-by: Paul E. McKenney <[email protected]>
---
tools/rcu/rcu-updaters.sh | 52 +++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
create mode 100755 tools/rcu/rcu-updaters.sh
diff --git a/tools/rcu/rcu-updaters.sh b/tools/rcu/rcu-updaters.sh
new file mode 100755
index 0000000000000..4ef1397927bbf
--- /dev/null
+++ b/tools/rcu/rcu-updaters.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Run bpftrace to obtain a histogram of the types of primitives used to
+# initiate RCU grace periods. The count associated with rcu_gp_init()
+# is the number of normal (non-expedited) grace periods.
+#
+# Usage: rcu-updaters.sh [ duration-in-seconds ]
+#
+# Note that not all kernel builds have all of these functions. In those
+# that do not, this script will issue a diagnostic for each that is not
+# found, but continue normally for the rest of the functions.
+
+duration=${1}
+if test -n "${duration}"
+then
+ exitclause='interval:s:'"${duration}"' { exit(); }'
+else
+ echo 'Hit control-C to end sample and print results.'
+fi
+bpftrace -e 'kprobe:kvfree_call_rcu,
+ kprobe:call_rcu,
+ kprobe:call_rcu_tasks,
+ kprobe:call_rcu_tasks_rude,
+ kprobe:call_rcu_tasks_trace,
+ kprobe:call_srcu,
+ kprobe:rcu_barrier,
+ kprobe:rcu_barrier_tasks,
+ kprobe:rcu_barrier_tasks_rude,
+ kprobe:rcu_barrier_tasks_trace,
+ kprobe:srcu_barrier,
+ kprobe:synchronize_rcu,
+ kprobe:synchronize_rcu_expedited,
+ kprobe:synchronize_rcu_tasks,
+ kprobe:synchronize_rcu_tasks_rude,
+ kprobe:synchronize_rcu_tasks_trace,
+ kprobe:synchronize_srcu,
+ kprobe:synchronize_srcu_expedited,
+ kprobe:get_state_synchronize_rcu,
+ kprobe:get_state_synchronize_rcu_full,
+ kprobe:start_poll_synchronize_rcu,
+ kprobe:start_poll_synchronize_rcu_expedited,
+ kprobe:start_poll_synchronize_rcu_full,
+ kprobe:start_poll_synchronize_rcu_expedited_full,
+ kprobe:poll_state_synchronize_rcu,
+ kprobe:poll_state_synchronize_rcu_full,
+ kprobe:cond_synchronize_rcu,
+ kprobe:cond_synchronize_rcu_full,
+ kprobe:start_poll_synchronize_srcu,
+ kprobe:poll_state_synchronize_srcu,
+ kprobe:rcu_gp_init
+ { @counts[func] = count(); } '"${exitclause}"
--
2.40.1
On powerpc systems, spinlock acquisition does not order prior stores
against later loads. This means that this statement:
rfcp->rfc_next = NULL;
Can be reordered to follow this statement:
WRITE_ONCE(*rfcpp, rfcp);
Which is then a data race with rcu_torture_fwd_prog_cr(), specifically,
this statement:
rfcpn = READ_ONCE(rfcp->rfc_next)
KCSAN located this data race, which represents a real failure on powerpc.
Signed-off-by: Paul E. McKenney <[email protected]>
Cc: Marco Elver <[email protected]>
Cc: Andrey Konovalov <[email protected]>
Cc: <[email protected]>
---
kernel/rcu/rcutorture.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 44cc455e1b615..cafe047d046e8 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -2630,7 +2630,7 @@ static void rcu_torture_fwd_cb_cr(struct rcu_head *rhp)
spin_lock_irqsave(&rfp->rcu_fwd_lock, flags);
rfcpp = rfp->rcu_fwd_cb_tail;
rfp->rcu_fwd_cb_tail = &rfcp->rfc_next;
- WRITE_ONCE(*rfcpp, rfcp);
+ smp_store_release(rfcpp, rfcp);
WRITE_ONCE(rfp->n_launders_cb, rfp->n_launders_cb + 1);
i = ((jiffies - rfp->rcu_fwd_startat) / (HZ / FWD_CBS_HIST_DIV));
if (i >= ARRAY_SIZE(rfp->n_launders_hist))
--
2.40.1
From: Zqiang <[email protected]>
This commit allows rcutorture to test double-call_srcu() when the
CONFIG_DEBUG_OBJECTS_RCU_HEAD Kconfig option is enabled. The non-raw
sdp structure's ->spinlock will be acquired in call_srcu(), hence this
commit also removes the current IRQ and preemption disabling so as to
avoid lockdep complaints.
Link: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Zqiang <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
---
kernel/rcu/rcutorture.c | 46 +++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 20 deletions(-)
diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 807fbf6123a77..44cc455e1b615 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -390,6 +390,7 @@ struct rcu_torture_ops {
int extendables;
int slow_gps;
int no_pi_lock;
+ int debug_objects;
const char *name;
};
@@ -577,6 +578,7 @@ static struct rcu_torture_ops rcu_ops = {
.irq_capable = 1,
.can_boost = IS_ENABLED(CONFIG_RCU_BOOST),
.extendables = RCUTORTURE_MAX_EXTEND,
+ .debug_objects = 1,
.name = "rcu"
};
@@ -747,6 +749,7 @@ static struct rcu_torture_ops srcu_ops = {
.cbflood_max = 50000,
.irq_capable = 1,
.no_pi_lock = IS_ENABLED(CONFIG_TINY_SRCU),
+ .debug_objects = 1,
.name = "srcu"
};
@@ -786,6 +789,7 @@ static struct rcu_torture_ops srcud_ops = {
.cbflood_max = 50000,
.irq_capable = 1,
.no_pi_lock = IS_ENABLED(CONFIG_TINY_SRCU),
+ .debug_objects = 1,
.name = "srcud"
};
@@ -3455,7 +3459,6 @@ rcu_torture_cleanup(void)
cur_ops->gp_slow_unregister(NULL);
}
-#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
static void rcu_torture_leak_cb(struct rcu_head *rhp)
{
}
@@ -3473,7 +3476,6 @@ static void rcu_torture_err_cb(struct rcu_head *rhp)
*/
pr_alert("%s: duplicated callback was invoked.\n", KBUILD_MODNAME);
}
-#endif /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
/*
* Verify that double-free causes debug-objects to complain, but only
@@ -3482,39 +3484,43 @@ static void rcu_torture_err_cb(struct rcu_head *rhp)
*/
static void rcu_test_debug_objects(void)
{
-#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
struct rcu_head rh1;
struct rcu_head rh2;
+ int idx;
+
+ if (!IS_ENABLED(CONFIG_DEBUG_OBJECTS_RCU_HEAD)) {
+ pr_alert("%s: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_%s()\n",
+ KBUILD_MODNAME, cur_ops->name);
+ return;
+ }
+
+ if (WARN_ON_ONCE(cur_ops->debug_objects &&
+ (!cur_ops->call || !cur_ops->cb_barrier)))
+ return;
+
struct rcu_head *rhp = kmalloc(sizeof(*rhp), GFP_KERNEL);
init_rcu_head_on_stack(&rh1);
init_rcu_head_on_stack(&rh2);
- pr_alert("%s: WARN: Duplicate call_rcu() test starting.\n", KBUILD_MODNAME);
+ pr_alert("%s: WARN: Duplicate call_%s() test starting.\n", KBUILD_MODNAME, cur_ops->name);
/* Try to queue the rh2 pair of callbacks for the same grace period. */
- preempt_disable(); /* Prevent preemption from interrupting test. */
- rcu_read_lock(); /* Make it impossible to finish a grace period. */
- call_rcu_hurry(&rh1, rcu_torture_leak_cb); /* Start grace period. */
- local_irq_disable(); /* Make it harder to start a new grace period. */
- call_rcu_hurry(&rh2, rcu_torture_leak_cb);
- call_rcu_hurry(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
+ idx = cur_ops->readlock(); /* Make it impossible to finish a grace period. */
+ cur_ops->call(&rh1, rcu_torture_leak_cb); /* Start grace period. */
+ cur_ops->call(&rh2, rcu_torture_leak_cb);
+ cur_ops->call(&rh2, rcu_torture_err_cb); /* Duplicate callback. */
if (rhp) {
- call_rcu_hurry(rhp, rcu_torture_leak_cb);
- call_rcu_hurry(rhp, rcu_torture_err_cb); /* Another duplicate callback. */
+ cur_ops->call(rhp, rcu_torture_leak_cb);
+ cur_ops->call(rhp, rcu_torture_err_cb); /* Another duplicate callback. */
}
- local_irq_enable();
- rcu_read_unlock();
- preempt_enable();
+ cur_ops->readunlock(idx);
/* Wait for them all to get done so we can safely return. */
- rcu_barrier();
- pr_alert("%s: WARN: Duplicate call_rcu() test complete.\n", KBUILD_MODNAME);
+ cur_ops->cb_barrier();
+ pr_alert("%s: WARN: Duplicate call_%s() test complete.\n", KBUILD_MODNAME, cur_ops->name);
destroy_rcu_head_on_stack(&rh1);
destroy_rcu_head_on_stack(&rh2);
kfree(rhp);
-#else /* #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
- pr_alert("%s: !CONFIG_DEBUG_OBJECTS_RCU_HEAD, not testing duplicate call_rcu()\n", KBUILD_MODNAME);
-#endif /* #else #ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD */
}
static void rcutorture_sync(void)
--
2.40.1
On Wed, 5 Jun 2024 at 00:36, Paul E. McKenney <[email protected]> wrote:
>
> On powerpc systems, spinlock acquisition does not order prior stores
> against later loads. This means that this statement:
>
> rfcp->rfc_next = NULL;
>
> Can be reordered to follow this statement:
>
> WRITE_ONCE(*rfcpp, rfcp);
>
> Which is then a data race with rcu_torture_fwd_prog_cr(), specifically,
> this statement:
>
> rfcpn = READ_ONCE(rfcp->rfc_next)
>
> KCSAN located this data race, which represents a real failure on powerpc.
>
> Signed-off-by: Paul E. McKenney <[email protected]>
> Cc: Marco Elver <[email protected]>
> Cc: Andrey Konovalov <[email protected]>
> Cc: <[email protected]>
Nice find - was this found by KCSAN's weak memory modeling, i.e. the
report showed you that a reordered access resulted in a data race?
Acked-by: Marco Elver <[email protected]>
> ---
> kernel/rcu/rcutorture.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> index 44cc455e1b615..cafe047d046e8 100644
> --- a/kernel/rcu/rcutorture.c
> +++ b/kernel/rcu/rcutorture.c
> @@ -2630,7 +2630,7 @@ static void rcu_torture_fwd_cb_cr(struct rcu_head *rhp)
> spin_lock_irqsave(&rfp->rcu_fwd_lock, flags);
> rfcpp = rfp->rcu_fwd_cb_tail;
> rfp->rcu_fwd_cb_tail = &rfcp->rfc_next;
> - WRITE_ONCE(*rfcpp, rfcp);
> + smp_store_release(rfcpp, rfcp);
> WRITE_ONCE(rfp->n_launders_cb, rfp->n_launders_cb + 1);
> i = ((jiffies - rfp->rcu_fwd_startat) / (HZ / FWD_CBS_HIST_DIV));
> if (i >= ARRAY_SIZE(rfp->n_launders_hist))
> --
> 2.40.1
>
On Wed, Jun 05, 2024 at 09:56:41AM +0200, Marco Elver wrote:
> On Wed, 5 Jun 2024 at 00:36, Paul E. McKenney <[email protected]> wrote:
> >
> > On powerpc systems, spinlock acquisition does not order prior stores
> > against later loads. This means that this statement:
> >
> > rfcp->rfc_next = NULL;
> >
> > Can be reordered to follow this statement:
> >
> > WRITE_ONCE(*rfcpp, rfcp);
> >
> > Which is then a data race with rcu_torture_fwd_prog_cr(), specifically,
> > this statement:
> >
> > rfcpn = READ_ONCE(rfcp->rfc_next)
> >
> > KCSAN located this data race, which represents a real failure on powerpc.
> >
> > Signed-off-by: Paul E. McKenney <[email protected]>
> > Cc: Marco Elver <[email protected]>
> > Cc: Andrey Konovalov <[email protected]>
> > Cc: <[email protected]>
>
> Nice find - was this found by KCSAN's weak memory modeling, i.e. the
> report showed you that a reordered access resulted in a data race?
If I remember correctly, yes.
Even on x86, the compiler is free to reorder that WRITE_ONCE() with
unmarked accesses, so one can argue that this bug is not specific
to powerpc.
> Acked-by: Marco Elver <[email protected]>
I will apply on my next rebase, thank you!
Thanx, Paul
> > ---
> > kernel/rcu/rcutorture.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> > index 44cc455e1b615..cafe047d046e8 100644
> > --- a/kernel/rcu/rcutorture.c
> > +++ b/kernel/rcu/rcutorture.c
> > @@ -2630,7 +2630,7 @@ static void rcu_torture_fwd_cb_cr(struct rcu_head *rhp)
> > spin_lock_irqsave(&rfp->rcu_fwd_lock, flags);
> > rfcpp = rfp->rcu_fwd_cb_tail;
> > rfp->rcu_fwd_cb_tail = &rfcp->rfc_next;
> > - WRITE_ONCE(*rfcpp, rfcp);
> > + smp_store_release(rfcpp, rfcp);
> > WRITE_ONCE(rfp->n_launders_cb, rfp->n_launders_cb + 1);
> > i = ((jiffies - rfp->rcu_fwd_startat) / (HZ / FWD_CBS_HIST_DIV));
> > if (i >= ARRAY_SIZE(rfp->n_launders_hist))
> > --
> > 2.40.1
> >