[PATCH rcu 1/9] rcu: Add lockdep_assert_in_rcu_read_lock() and friends

Paul E. McKenney posted 9 patches 1 year, 8 months ago
[PATCH rcu 1/9] rcu: Add lockdep_assert_in_rcu_read_lock() and friends
Posted by Paul E. McKenney 1 year, 8 months ago
There is no direct RCU counterpart to lockdep_assert_irqs_disabled()
and friends.  Although it is possible to construct them, it would
be more convenient to have the following lockdep assertions:

lockdep_assert_in_rcu_read_lock()
lockdep_assert_in_rcu_read_lock_bh()
lockdep_assert_in_rcu_read_lock_sched()
lockdep_assert_in_rcu_reader()

This commit therefore creates them.

Reported-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 include/linux/rcupdate.h | 60 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index dfd2399f2cde0..8470a85f65634 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -421,11 +421,71 @@ static inline void rcu_preempt_sleep_check(void) { }
 				 "Illegal context switch in RCU-sched read-side critical section"); \
 	} while (0)
 
+// See RCU_LOCKDEP_WARN() for an explanation of the double call to
+// debug_lockdep_rcu_enabled().
+static inline bool lockdep_assert_rcu_helper(bool c)
+{
+	return debug_lockdep_rcu_enabled() &&
+	       (c || !rcu_is_watching() || !rcu_lockdep_current_cpu_online()) &&
+	       debug_lockdep_rcu_enabled();
+}
+
+/**
+ * lockdep_assert_in_rcu_read_lock - WARN if not protected by rcu_read_lock()
+ *
+ * Splats if lockdep is enabled and there is no rcu_read_lock() in effect.
+ */
+#define lockdep_assert_in_rcu_read_lock() \
+	WARN_ON_ONCE(lockdep_assert_rcu_helper(!lock_is_held(&rcu_lock_map)))
+
+/**
+ * lockdep_assert_in_rcu_read_lock_bh - WARN if not protected by rcu_read_lock_bh()
+ *
+ * Splats if lockdep is enabled and there is no rcu_read_lock_bh() in effect.
+ * Note that local_bh_disable() and friends do not suffice here, instead an
+ * actual rcu_read_lock_bh() is required.
+ */
+#define lockdep_assert_in_rcu_read_lock_bh() \
+	WARN_ON_ONCE(lockdep_assert_rcu_helper(!lock_is_held(&rcu_bh_lock_map)))
+
+/**
+ * lockdep_assert_in_rcu_read_lock_sched - WARN if not protected by rcu_read_lock_sched()
+ *
+ * Splats if lockdep is enabled and there is no rcu_read_lock_sched()
+ * in effect.  Note that preempt_disable() and friends do not suffice here,
+ * instead an actual rcu_read_lock_sched() is required.
+ */
+#define lockdep_assert_in_rcu_read_lock_sched() \
+	WARN_ON_ONCE(lockdep_assert_rcu_helper(!lock_is_held(&rcu_sched_lock_map)))
+
+/**
+ * lockdep_assert_in_rcu_reader - WARN if not within some type of RCU reader
+ *
+ * Splats if lockdep is enabled and there is no RCU reader of any
+ * type in effect.  Note that regions of code protected by things like
+ * preempt_disable, local_bh_disable(), and local_irq_disable() all qualify
+ * as RCU readers.
+ *
+ * Note that this will never trigger in PREEMPT_NONE or PREEMPT_VOLUNTARY
+ * kernels that are not also built with PREEMPT_COUNT.  But if you have
+ * lockdep enabled, you might as well also enable PREEMPT_COUNT.
+ */
+#define lockdep_assert_in_rcu_reader()								\
+	WARN_ON_ONCE(lockdep_assert_rcu_helper(!lock_is_held(&rcu_lock_map) &&			\
+					       !lock_is_held(&rcu_bh_lock_map) &&		\
+					       !lock_is_held(&rcu_sched_lock_map) &&		\
+					       preemptible()))
+
 #else /* #ifdef CONFIG_PROVE_RCU */
 
 #define RCU_LOCKDEP_WARN(c, s) do { } while (0 && (c))
 #define rcu_sleep_check() do { } while (0)
 
+#define lockdep_assert_in_rcu_read_lock() do { } while (0)
+#define lockdep_assert_in_rcu_read_lock_bh() do { } while (0)
+#define lockdep_assert_in_rcu_read_lock_sched() do { } while (0)
+#define lockdep_assert_in_rcu_reader() do { } while (0)
+
 #endif /* #else #ifdef CONFIG_PROVE_RCU */
 
 /*
-- 
2.40.1
Re: [PATCH rcu 1/9] rcu: Add lockdep_assert_in_rcu_read_lock() and friends
Posted by Jeff Johnson 11 months, 3 weeks ago
On 6/4/24 15:23, Paul E. McKenney wrote:
> There is no direct RCU counterpart to lockdep_assert_irqs_disabled()
> and friends.  Although it is possible to construct them, it would
> be more convenient to have the following lockdep assertions:
> 
> lockdep_assert_in_rcu_read_lock()
> lockdep_assert_in_rcu_read_lock_bh()
> lockdep_assert_in_rcu_read_lock_sched()
> lockdep_assert_in_rcu_reader()
> 
> This commit therefore creates them.

I'm looking at some downstream code that is trying to become
upstream compliant, and currently that code uses:

	RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "some message");

It seems like this would be a good use of one of these helper
functions, but I'm shocked to see that no upstream code is using
them yet.

Is there a reason to not use these helpers?

/jeff
Re: [PATCH rcu 1/9] rcu: Add lockdep_assert_in_rcu_read_lock() and friends
Posted by Paul E. McKenney 11 months, 3 weeks ago
On Thu, Feb 20, 2025 at 11:38:14AM -0800, Jeff Johnson wrote:
> On 6/4/24 15:23, Paul E. McKenney wrote:
> > There is no direct RCU counterpart to lockdep_assert_irqs_disabled()
> > and friends.  Although it is possible to construct them, it would
> > be more convenient to have the following lockdep assertions:
> > 
> > lockdep_assert_in_rcu_read_lock()
> > lockdep_assert_in_rcu_read_lock_bh()
> > lockdep_assert_in_rcu_read_lock_sched()
> > lockdep_assert_in_rcu_reader()
> > 
> > This commit therefore creates them.
> 
> I'm looking at some downstream code that is trying to become
> upstream compliant, and currently that code uses:
> 
> 	RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "some message");
> 
> It seems like this would be a good use of one of these helper
> functions, but I'm shocked to see that no upstream code is using
> them yet.
> 
> Is there a reason to not use these helpers?

In cases where there is no additional useful information that can be
placed in "some message", the new helpers should be just fine.

							Thanx, Paul
Re: [PATCH rcu 1/9] rcu: Add lockdep_assert_in_rcu_read_lock() and friends
Posted by Jeff Johnson 11 months, 3 weeks ago
On 2/20/2025 2:04 PM, Paul E. McKenney wrote:
> On Thu, Feb 20, 2025 at 11:38:14AM -0800, Jeff Johnson wrote:
>> On 6/4/24 15:23, Paul E. McKenney wrote:
>>> There is no direct RCU counterpart to lockdep_assert_irqs_disabled()
>>> and friends.  Although it is possible to construct them, it would
>>> be more convenient to have the following lockdep assertions:
>>>
>>> lockdep_assert_in_rcu_read_lock()
>>> lockdep_assert_in_rcu_read_lock_bh()
>>> lockdep_assert_in_rcu_read_lock_sched()
>>> lockdep_assert_in_rcu_reader()
>>>
>>> This commit therefore creates them.
>>
>> I'm looking at some downstream code that is trying to become
>> upstream compliant, and currently that code uses:
>>
>> 	RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "some message");
>>
>> It seems like this would be a good use of one of these helper
>> functions, but I'm shocked to see that no upstream code is using
>> them yet.
>>
>> Is there a reason to not use these helpers?
> 
> In cases where there is no additional useful information that can be
> placed in "some message", the new helpers should be just fine.

Thanks for the confirmation, Paul!

/jeff