lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ