[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20231201154932.468d088b@gandalf.local.home>
Date: Fri, 1 Dec 2023 15:49:32 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: "Paul E. McKenney" <paulmck@...nel.org>
Cc: LKML <linux-kernel@...r.kernel.org>, rcu@...r.kernel.org,
Frederic Weisbecker <fweisbec@...il.com>,
Joel Fernandes <joel@...lfernandes.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Subject: [RCU] rcu_tasks_trace_qs(): trc_reader_special.b.need_qs value
incorrect likely()?
Paul,
I just started running my branch tracer (that checks all branches and also
gives likely and unlikely correctness). And I found this:
correct incorrect % Function File Line
------- --------- - -------- ---- ----
0 1217713 100 rcu_softirq_qs tree.c 247
Which comes down to this:
# define rcu_tasks_trace_qs(t) \
do { \
int ___rttq_nesting = READ_ONCE((t)->trc_reader_nesting); \
\
if (likely(!READ_ONCE((t)->trc_reader_special.b.need_qs)) && \
likely(!___rttq_nesting)) { \
rcu_trc_cmpxchg_need_qs((t), 0, TRC_NEED_QS_CHECKED); \
} else if (___rttq_nesting && ___rttq_nesting != INT_MIN && \
!READ_ONCE((t)->trc_reader_special.b.blocked)) { \
rcu_tasks_trace_qs_blkd(t); \
} \
} while (0)
I added just before the likely/unlikely to my test box and I see this:
trace_printk("need qs? %d %d\n", READ_ONCE((t)->trc_reader_special.b.need_qs), ___rttq_nesting); \
<idle>-0 [005] d.h1. 2.482412: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [002] d.h1. 2.482464: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [000] d.h1. 2.482766: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [001] d.h1. 2.482951: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [007] d.h1. 2.482958: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [005] d.h1. 2.483600: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [002] d.h1. 2.483624: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [000] d.h1. 2.483927: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [007] d.h1. 2.484068: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [001] d.h1. 2.484127: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [002] d.h1. 2.484723: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [005] d.h1. 2.484745: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [000] d.h1. 2.485015: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [007] d.h1. 2.485202: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [001] d.h1. 2.485258: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [002] d.h1. 2.485818: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [005] d.h1. 2.485929: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [000] d.h1. 2.486224: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [007] d.h1. 2.486370: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [001] d.h1. 2.486399: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [002] d.h1. 2.486895: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [005] d.h1. 2.487049: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [000] d.h1. 2.487318: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [007] d.h1. 2.487472: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [001] d.h1. 2.487522: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [002] d.h1. 2.488034: rcu_sched_clock_irq: need qs? 2 0
<idle>-0 [005] d.h1. 2.488220: rcu_sched_clock_irq: need qs? 2 0
Note, that "2" is the READ_ONCE() without the "!" to it. Thus:
if (likely(!READ_ONCE((t)->trc_reader_special.b.need_qs)) && \
Is unlikely to be true.
Was this supposed to be:
if (!likely(READ_ONCE((t)->trc_reader_special.b.need_qs)) && \
Or could it be converted to:
if (unlikely(!READ_ONCE((t)->trc_reader_special.b.need_qs)) && \
?
Note, the unlikely tracing is running on my production server v6.6.3.
The above trace is from my test box with latest Linus's tree.
-- Steve
Powered by blists - more mailing lists