lockdep: Add lightweight IRQs disabled/enabled check

PATCH In review Frederic Weisbecker 4 participants 19 messages Oct 20, 2017 – Nov 3, 2017

Subsystems (12)

Networking x86 perf events Scheduler +8 more subsystems

lore

Share & tools

lore

Frederic Weisbecker

author maintainer

2017-10-20 00:56 UTC #0 lore raw

cover lockdep: Add lightweight IRQs disabled/enabled check

Checking whether IRQs are disabled or enabled is often an important sanity check. Now we can be sometimes reluctant to have it on fastpath because reading the flags can be somewhat costly.

As suggested by Peter Zijlstra some time ago, this would be a nice lockdep extension: it's the natural concurrency sanity checker and it already tracks down the irqflags state.

So here it is.

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
lockdep/irq

HEAD: 1cb58314de50c73eaf0b0322615ecb7a8ee39592

Thanks,
Frederic
---

Frederic Weisbecker (14):
lockdep: Add IRQs disabled/enabled assertion
irq: Use lockdep to assert IRQs are disabled/enabled
workqueue: Use lockdep to assert IRQs are disabled/enabled
nohz: Use lockdep to assert IRQs are disabled/enabled
hrtimer: Use lockdep to assert IRQs are disabled/enabled
smp: Use lockdep to assert IRQs are disabled/enabled
x86: Use lockdep to assert IRQs are disabled/enabled
perf: Use lockdep to assert IRQs are disabled/enabled irq/timings: Use lockdep to assert IRQs are disabled/enabled
irq_work: Use lockdep to assert IRQs are disabled/enabled
sched: Use lockdep to assert IRQs are disabled/enabled
posix-cpu-timers: Use lockdep to assert IRQs are disabled/enabled
netpoll: Use lockdep to assert IRQs are disabled/enabled
rcu: Use lockdep to assert IRQs are disabled/enabled

 arch/x86/entry/common.c        |  2 +-
 arch/x86/kernel/smpboot.c      |  2 +-
 include/linux/lockdep.h        | 13 +++++++++++++
 kernel/events/core.c           | 12 ++++++------
 kernel/irq/timings.c           |  2 +-
 kernel/irq_work.c              |  2 +-
 kernel/rcu/tree.c              | 16 ++++++++--------
 kernel/rcu/tree_plugin.h       | 10 +++++-----
 kernel/sched/clock.c           |  2 +-
 kernel/sched/cputime.c         |  3 +--
 kernel/smp.c                   |  2 +-
 kernel/softirq.c               | 10 +++++-----
 kernel/time/hrtimer.c          |  4 +---
 kernel/time/posix-cpu-timers.c |  6 +++---
 kernel/time/tick-sched.c       |  5 ++---
 kernel/workqueue.c             |  2 +-
 net/core/netpoll.c             |  2 +-
 17 files changed, 52 insertions(+), 43 deletions(-)

Frederic Weisbecker

author maintainer

2017-10-20 00:56 UTC #1 lore raw

PATCH 1/14 lockdep: Add IRQs disabled/enabled assertion

Checking whether IRQs are enabled or disabled is a very common sanity check, however not free of overhead especially on fastpath where such assertion is very common.

Lockdep is a good host for such concurrency correctness check and it even already tracks down IRQs disablement state. Just reuse its machinery. This will allow us to get rid of the flags pop and check overhead from fast path when kernel is built for production.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

---
 include/linux/lockdep.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

include/linux/lockdep.h +13 -0

index bfa8e0b..fbe0127 100644
@@ -719,9 +719,22 @@ do {									\
 	lock_acquire(&(lock)->dep_map, 0, 0, 1, 1, NULL, _THIS_IP_);	\
 	lock_release(&(lock)->dep_map, 0, _THIS_IP_);			\
 } while (0)
+
+#define lockdep_assert_irqs_enabled()	({				\
+		!WARN_ONCE(debug_locks && !current->hardirqs_enabled,	\
+			   "IRQs not enabled as expected\n");		\
+	})
+
+#define lockdep_assert_irqs_disabled()	({				\
+		!WARN_ONCE(debug_locks && current->hardirqs_enabled,	\
+			   "IRQs not disabled as expected\n");		\
+	})
+
 #else
 # define might_lock(lock) do { } while (0)
 # define might_lock_read(lock) do { } while (0)
+static inline int lockdep_assert_irqs_enabled(void) { return 1; }
+static inline int lockdep_assert_irqs_disabled(void) { return 1; }
 #endif
 
 #ifdef CONFIG_LOCKDEP

Signature · 1 line

2.7.4

Frederic Weisbecker

author maintainer

2017-10-20 00:56 UTC #2 lore raw

PATCH 2/14 irq: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

---
 kernel/softirq.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

kernel/softirq.c +5 -5

index 4e09821..662f7b1b 100644
@@ -137,7 +137,7 @@ EXPORT_SYMBOL(__local_bh_disable_ip);
 
 static void __local_bh_enable(unsigned int cnt)
 {
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	if (softirq_count() == (cnt & SOFTIRQ_MASK))
 		trace_softirqs_on(_RET_IP_);

@@ -158,7 +158,8 @@ EXPORT_SYMBOL(_local_bh_enable);
 
 void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
 {
-	WARN_ON_ONCE(in_irq() || irqs_disabled());
+	WARN_ON_ONCE(in_irq());
+	lockdep_assert_irqs_enabled();
 #ifdef CONFIG_TRACE_IRQFLAGS
 	local_irq_disable();
 #endif

@@ -396,9 +397,8 @@ void irq_exit(void)
 #ifndef __ARCH_IRQ_EXIT_IRQS_DISABLED
 	local_irq_disable();
 #else
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 #endif
-
 	account_irq_exit_time(current);
 	preempt_count_sub(HARDIRQ_OFFSET);
 	if (!in_interrupt() && local_softirq_pending())

@@ -488,7 +488,7 @@ EXPORT_SYMBOL(__tasklet_hi_schedule);
 
 void __tasklet_hi_schedule_first(struct tasklet_struct *t)
 {
-	BUG_ON(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	t->next = __this_cpu_read(tasklet_hi_vec.head);
 	__this_cpu_write(tasklet_hi_vec.head, t);

Signature · 1 line

2.7.4

Frederic Weisbecker

author maintainer

2017-10-20 00:56 UTC #3 lore raw

PATCH 3/14 workqueue: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

Cc Tejun Heo <[email protected]>

Cc Lai Jiangshan <[email protected]>

---
 kernel/workqueue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/workqueue.c +1 -1

index c77fdf6..4c6809e 100644
@@ -1375,7 +1375,7 @@ static void __queue_work(int cpu, struct workqueue_struct *wq,
 	 * queued or lose PENDING.  Grabbing PENDING and queueing should
 	 * happen with IRQ disabled.
 	 */
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	debug_work_activate(work);

Signature · 1 line

2.7.4

Tejun Heo

maintainer Acked-by on v1 3/14

2017-10-21 15:39 UTC #4 lore raw

Re: [PATCH 03/14] workqueue: Use lockdep to assert IRQs are disabled/enabled

Quoted 11 lines · Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.
Signed-off-by Frederic Weisbecker <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Tejun Heo <[email protected]>
Cc Lai Jiangshan <[email protected]>

Acked-by Tejun Heo <[email protected]>

Please feel free to route with other patches.

Thanks.

Signature · 1 line

tejun

Frederic Weisbecker

author maintainer

2017-10-20 00:59 UTC #5 lore raw

PATCH 4/14 nohz: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

---
 kernel/time/tick-sched.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

kernel/time/tick-sched.c +2 -3

index 7b258c5..e179336 100644
@@ -199,7 +199,7 @@ static bool check_tick_dependency(atomic_t *dep)
 
 static bool can_stop_full_tick(int cpu, struct tick_sched *ts)
 {
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	if (unlikely(!cpu_online(cpu)))
 		return false;

@@ -962,8 +962,7 @@ void tick_nohz_idle_enter(void)
 {
 	struct tick_sched *ts;
 
-	WARN_ON_ONCE(irqs_disabled());
-
+	lockdep_assert_irqs_enabled();
 	/*
 	 * Update the idle state in the scheduler domain hierarchy
 	 * when tick_nohz_stop_sched_tick() is called from the idle loop.

Signature · 1 line

2.7.4

Frederic Weisbecker

author maintainer

2017-10-20 00:58 UTC #6 lore raw

PATCH 5/14 hrtimer: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

---
 kernel/time/hrtimer.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

kernel/time/hrtimer.c +1 -3

index 88f75f9..d325208 100644
@@ -758,9 +758,7 @@ void clock_was_set(void)
  */
 void hrtimers_resume(void)
 {
-	WARN_ONCE(!irqs_disabled(),
-		  KERN_INFO "hrtimers_resume() called with IRQs enabled!");
-
+	lockdep_assert_irqs_disabled();
 	/* Retrigger on the local CPU */
 	retrigger_next_event(NULL);
 	/* And schedule a retrigger for all others */

Signature · 1 line

2.7.4

Frederic Weisbecker

author maintainer

2017-10-20 00:56 UTC #7 lore raw

PATCH 6/14 smp: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

---
 kernel/smp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/smp.c +1 -1

index c94dd85..084c8b3 100644
@@ -213,7 +213,7 @@ static void flush_smp_call_function_queue(bool warn_cpu_offline)
 	call_single_data_t *csd, *csd_next;
 	static bool warned;
 
-	WARN_ON(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	head = this_cpu_ptr(&call_single_queue);
 	entry = llist_del_all(head);

Signature · 1 line

2.7.4

Frederic Weisbecker

author maintainer

2017-10-20 00:56 UTC #8 lore raw

PATCH 7/14 x86: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

---
 arch/x86/entry/common.c   | 2 +-
 arch/x86/kernel/smpboot.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

arch/x86/entry/common.c +1 -1

index 03505ff..b4f3a55 100644
@@ -186,7 +186,7 @@ __visible inline void prepare_exit_to_usermode(struct pt_regs *regs)
 
 	addr_limit_user_check();
 
-	if (IS_ENABLED(CONFIG_PROVE_LOCKING) && WARN_ON(!irqs_disabled()))
+	if (!lockdep_assert_irqs_disabled())
 		local_irq_disable();
 
 	lockdep_sys_exit();

arch/x86/kernel/smpboot.c +1 -1

index aed1460..aee08a4 100644
@@ -1095,7 +1095,7 @@ int native_cpu_up(unsigned int cpu, struct task_struct *tidle)
 	unsigned long flags;
 	int err, ret = 0;
 
-	WARN_ON(irqs_disabled());
+	lockdep_assert_irqs_enabled();
 
 	pr_debug("++++++++++++++++++++=_---CPU UP  %u\n", cpu);

Signature · 1 line

2.7.4

Peter Zijlstra

maintainer

2017-10-22 09:20 UTC #9 lore raw

Re: [PATCH 07/14] x86: Use lockdep to assert IRQs are disabled/enabled

arch/x86/entry/common.c __visible inline void prepare_exit_to_usermode(struct pt_regs *regs) · line 192

Quoted 13 lines · Frederic Weisbecker

arch/x86/entry/common.c +1 -1

index 03505ff..b4f3a55 100644
@@ -186,7 +186,7 @@ __visible inline void prepare_exit_to_usermode(struct pt_regs *regs)
 
 	addr_limit_user_check();
 
-	if (IS_ENABLED(CONFIG_PROVE_LOCKING) && WARN_ON(!irqs_disabled()))
+	if (!lockdep_assert_irqs_disabled())
 		local_irq_disable();
 
 	lockdep_sys_exit();

So this is the only site that ever uses the return value; and for this you've chosen the wrong value for !CONFIG_PROVE_LOCKING (namely 1), resulting in an unconditional CLI here for !lockdep kernels.

How about we replace that whole thing with a simple:

	lockdep_assert_irqs_disabled();

And leave it at that, allowing us to remove the return value thing entirely.

The whole if !disabled, disable logic is uber paranoid programming, but I don't think we've ever seen that WARN trigger, and if it does (and then burns the kernel) we at least know wtf happend.

Frederic Weisbecker

author maintainer

2017-11-03 14:27 UTC #10 lore raw

Re: [PATCH 07/14] x86: Use lockdep to assert IRQs are disabled/enabled

2017-10-22 11:20 UTC+02:00, Peter Zijlstra <[email protected]>:

Quoted 14 lines · Frederic Weisbecker

Quoted 13 lines

arch/x86/entry/common.c +1 -1

index 03505ff..b4f3a55 100644
@@ -186,7 +186,7 @@ __visible inline void prepare_exit_to_usermode(struct
 pt_regs *regs)
 
 	addr_limit_user_check();
 
-	if (IS_ENABLED(CONFIG_PROVE_LOCKING) && WARN_ON(!irqs_disabled()))
+	if (!lockdep_assert_irqs_disabled())
 		local_irq_disable();

Quoted 5 lines

Quoted 1 line
lockdep_sys_exit();
So this is the only site that ever uses the return value; and for this you've chosen the wrong value for !CONFIG_PROVE_LOCKING (namely 1), resulting in an unconditional CLI here for !lockdep kernels.

Hmm, it should be the opposite. The assertion maps to 1 for !CONFIG_PROVE_LOCKING
but we are testing with "!".

Quoted 10 lines

How about we replace that whole thing with a simple:
	lockdep_assert_irqs_disabled();
And leave it at that, allowing us to remove the return value thing entirely.
The whole if !disabled, disable logic is uber paranoid programming, but I don't think we've ever seen that WARN trigger, and if it does (and then burns the kernel) we at least know wtf happend.

I certainly don't mind personally, as long as we get the warning. Now I've seen different
opinions on matters such as this. Some prefer to have the
auto-correction, some don't.

I'll try as you say and see if anybody gets angry :)

Frederic Weisbecker

author maintainer

2017-10-20 00:58 UTC #11 lore raw

PATCH 8/14 perf: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

---
 kernel/events/core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

kernel/events/core.c +6 -6

index 04989fb..f57cde0e 100644
@@ -209,7 +209,7 @@ static int event_function(void *info)
 	struct perf_event_context *task_ctx = cpuctx->task_ctx;
 	int ret = 0;
 
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	perf_ctx_lock(cpuctx, task_ctx);
 	/*

@@ -306,7 +306,7 @@ static void event_function_local(struct perf_event *event, event_f func, void *d
 	struct task_struct *task = READ_ONCE(ctx->task);
 	struct perf_event_context *task_ctx = NULL;
 
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	if (task) {
 		if (task == TASK_TOMBSTONE)

@@ -1004,7 +1004,7 @@ static enum hrtimer_restart perf_mux_hrtimer_handler(struct hrtimer *hr)
 	struct perf_cpu_context *cpuctx;
 	int rotations = 0;
 
-	WARN_ON(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	cpuctx = container_of(hr, struct perf_cpu_context, hrtimer);
 	rotations = perf_rotate_context(cpuctx);

@@ -1091,7 +1091,7 @@ static void perf_event_ctx_activate(struct perf_event_context *ctx)
 {
 	struct list_head *head = this_cpu_ptr(&active_ctx_list);
 
-	WARN_ON(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	WARN_ON(!list_empty(&ctx->active_ctx_list));

@@ -1100,7 +1100,7 @@ static void perf_event_ctx_activate(struct perf_event_context *ctx)
 
 static void perf_event_ctx_deactivate(struct perf_event_context *ctx)
 {
-	WARN_ON(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	WARN_ON(list_empty(&ctx->active_ctx_list));

@@ -3521,7 +3521,7 @@ void perf_event_task_tick(void)
 	struct perf_event_context *ctx, *tmp;
 	int throttled;
 
-	WARN_ON(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	__this_cpu_inc(perf_throttled_seq);
 	throttled = __this_cpu_xchg(perf_throttled_count, 0);

Signature · 1 line

2.7.4

Frederic Weisbecker

author maintainer

2017-10-20 00:56 UTC #12 lore raw

PATCH 9/14 irq/timings: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

---
 kernel/irq/timings.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/irq/timings.c +1 -1

index c8c1d07..e0923fa 100644
@@ -264,7 +264,7 @@ u64 irq_timings_next_event(u64 now)
 	 * order to prevent the timings circular buffer to be updated
 	 * while we are reading it.
 	 */
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	/*
 	 * Number of elements in the circular buffer: If it happens it

Signature · 1 line

2.7.4

Frederic Weisbecker

author maintainer

2017-10-20 00:57 UTC #13 lore raw

PATCH 10/14 irq_work: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

---
 kernel/irq_work.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/irq_work.c +1 -1

index bcf107c..8995796 100644
@@ -188,7 +188,7 @@ void irq_work_tick(void)
  */
 void irq_work_sync(struct irq_work *work)
 {
-	WARN_ON_ONCE(irqs_disabled());
+	lockdep_assert_irqs_enabled();
 
 	while (work->flags & IRQ_WORK_BUSY)
 		cpu_relax();

Signature · 1 line

2.7.4

Frederic Weisbecker

author maintainer

2017-10-20 00:56 UTC #14 lore raw

PATCH 11/14 sched: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

---
 kernel/sched/clock.c   | 2 +-
 kernel/sched/cputime.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

kernel/sched/clock.c +1 -1

index ca0f8fc..e086bab 100644
@@ -388,7 +388,7 @@ void sched_clock_tick(void)
 	if (unlikely(!sched_clock_running))
 		return;
 
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	scd = this_scd();
 	__scd_stamp(scd);

kernel/sched/cputime.c +1 -2

index 14d2dbf..9be8b68 100644
@@ -259,8 +259,7 @@ static inline u64 account_other_time(u64 max)
 {
 	u64 accounted;
 
-	/* Shall be converted to a lockdep-enabled lightweight check */
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	accounted = steal_account_process_time(max);

Signature · 1 line

2.7.4

Frederic Weisbecker

author maintainer

2017-10-20 00:57 UTC #15 lore raw

PATCH 12/14 posix-cpu-timers: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

---
 kernel/time/posix-cpu-timers.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

kernel/time/posix-cpu-timers.c +3 -3

index 8585ad6..8fafb87 100644
@@ -602,7 +602,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags,
 	/*
 	 * Disarm any old timer after extracting its expiry time.
 	 */
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	ret = 0;
 	old_incr = timer->it.cpu.incr;

@@ -1033,7 +1033,7 @@ static void posix_cpu_timer_rearm(struct k_itimer *timer)
 	/*
 	 * Now re-arm for the new expiry time.
 	 */
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 	arm_timer(timer);
 unlock:
 	unlock_task_sighand(p, &flags);

@@ -1124,7 +1124,7 @@ void run_posix_cpu_timers(struct task_struct *tsk)
 	struct k_itimer *timer, *next;
 	unsigned long flags;
 
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	/*
 	 * The fast path checks that there are no expired thread or thread

Signature · 1 line

2.7.4

Frederic Weisbecker

author maintainer

2017-10-20 00:56 UTC #16 lore raw

PATCH 13/14 netpoll: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Use lockdep to check that IRQs are enabled or disabled as expected. This way the sanity check only shows overhead when concurrency correctness debug code is enabled.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

Cc David S. Miller <[email protected]>

---
 net/core/netpoll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

net/core/netpoll.c +1 -1

index 912731b..57557a6 100644
@@ -334,7 +334,7 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
 	/* It is up to the caller to keep npinfo alive. */
 	struct netpoll_info *npinfo;
 
-	WARN_ON_ONCE(!irqs_disabled());
+	lockdep_assert_irqs_disabled();
 
 	npinfo = rcu_dereference_bh(np->dev->npinfo);
 	if (!npinfo || !netif_running(dev) || !netif_device_present(dev)) {

Signature · 1 line

2.7.4

Frederic Weisbecker

author maintainer

2017-10-20 00:56 UTC #17 lore raw

PATCH 14/14 rcu: Use lockdep to assert IRQs are disabled/enabled

In reply to Frederic Weisbecker

Lockdep now has an integrated IRQs disabled/enabled sanity check. Just use it instead of the ad-hoc RCU version.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Paul E. McKenney <[email protected]>

---
 kernel/rcu/tree.c        | 16 ++++++++--------
 kernel/rcu/tree_plugin.h | 10 +++++-----
 2 files changed, 13 insertions(+), 13 deletions(-)

kernel/rcu/tree.c +8 -8

index b0ad62b..7661887 100644
@@ -734,7 +734,7 @@ static int rcu_future_needs_gp(struct rcu_state *rsp)
 	int idx = (READ_ONCE(rnp->completed) + 1) & 0x1;
 	int *fp = &rnp->need_future_gp[idx];
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_future_needs_gp() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	return READ_ONCE(*fp);
 }

@@ -746,7 +746,7 @@ static int rcu_future_needs_gp(struct rcu_state *rsp)
 static bool
 cpu_needs_another_gp(struct rcu_state *rsp, struct rcu_data *rdp)
 {
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "cpu_needs_another_gp() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	if (rcu_gp_in_progress(rsp))
 		return false;  /* No, a grace period is already in progress. */
 	if (rcu_future_needs_gp(rsp))

@@ -773,7 +773,7 @@ static void rcu_eqs_enter_common(bool user)
 	struct rcu_data *rdp;
 	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_eqs_enter_common() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	trace_rcu_dyntick(TPS("Start"), rdtp->dynticks_nesting, 0);
 	if (IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
 	    !user && !is_idle_task(current)) {

@@ -840,7 +840,7 @@ static void rcu_eqs_enter(bool user)
  */
 void rcu_idle_enter(void)
 {
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_idle_enter() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	rcu_eqs_enter(false);
 }

@@ -855,7 +855,7 @@ void rcu_idle_enter(void)
  */
 void rcu_user_enter(void)
 {
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_user_enter() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	rcu_eqs_enter(true);
 }
 #endif /* CONFIG_NO_HZ_FULL */

@@ -880,7 +880,7 @@ void rcu_irq_exit(void)
 {
 	struct rcu_dynticks *rdtp;
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_irq_exit() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	rdtp = this_cpu_ptr(&rcu_dynticks);
 
 	/* Page faults can happen in NMI handlers, so check... */

@@ -947,7 +947,7 @@ static void rcu_eqs_exit(bool user)
 	struct rcu_dynticks *rdtp;
 	long long oldval;
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_eqs_exit() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	rdtp = this_cpu_ptr(&rcu_dynticks);
 	oldval = rdtp->dynticks_nesting;
 	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && oldval < 0);

@@ -1018,7 +1018,7 @@ void rcu_irq_enter(void)
 	struct rcu_dynticks *rdtp;
 	long long oldval;
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_irq_enter() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	rdtp = this_cpu_ptr(&rcu_dynticks);
 
 	/* Page faults can happen in NMI handlers, so check... */

kernel/rcu/tree_plugin.h +5 -5

index e012b9b..df08e5c 100644
@@ -325,7 +325,7 @@ static void rcu_preempt_note_context_switch(bool preempt)
 	struct rcu_data *rdp;
 	struct rcu_node *rnp;
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_preempt_note_context_switch() invoked with interrupts enabled!!!\n");
+	lockdep_assert_irqs_disabled();
 	WARN_ON_ONCE(!preempt && t->rcu_read_lock_nesting > 0);
 	if (t->rcu_read_lock_nesting > 0 &&
 	    !t->rcu_read_unlock_special.b.blocked) {

@@ -1421,7 +1421,7 @@ int rcu_needs_cpu(u64 basemono, u64 *nextevt)
 	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
 	unsigned long dj;
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_needs_cpu() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 
 	/* Snapshot to detect later posting of non-lazy callback. */
 	rdtp->nonlazy_posted_snap = rdtp->nonlazy_posted;

@@ -1470,7 +1470,7 @@ static void rcu_prepare_for_idle(void)
 	struct rcu_state *rsp;
 	int tne;
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_prepare_for_idle() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	if (rcu_is_nocb_cpu(smp_processor_id()))
 		return;

@@ -1525,7 +1525,7 @@ static void rcu_prepare_for_idle(void)
  */
 static void rcu_cleanup_after_idle(void)
 {
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_cleanup_after_idle() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	if (rcu_is_nocb_cpu(smp_processor_id()))
 		return;
 	if (rcu_try_advance_all_cbs())

@@ -2012,7 +2012,7 @@ static bool __maybe_unused rcu_nocb_adopt_orphan_cbs(struct rcu_data *my_rdp,
 						     struct rcu_data *rdp,
 						     unsigned long flags)
 {
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_nocb_adopt_orphan_cbs() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	if (!rcu_is_nocb_cpu(smp_processor_id()))
 		return false; /* Not NOCBs CPU, caller must migrate CBs. */
 	__call_rcu_nocb_enqueue(my_rdp, rcu_segcblist_head(&rdp->cblist),

Signature · 1 line

2.7.4

Paul E. McKenney

Acked-by on v1 14/14

2017-10-21 21:36 UTC #18 lore raw

Re: [PATCH 14/14] rcu: Use lockdep to assert IRQs are disabled/enabled

Quoted 8 lines · Frederic Weisbecker

Lockdep now has an integrated IRQs disabled/enabled sanity check. Just use it instead of the ad-hoc RCU version.
Signed-off-by Frederic Weisbecker <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Paul E. McKenney <[email protected]>

Looks good to me and passes light rcutorture testing. Feel free to push this up with the rest of this series.

Acked-by Paul E. McKenney <[email protected]>

Quoted 133 lines

---
 kernel/rcu/tree.c        | 16 ++++++++--------
 kernel/rcu/tree_plugin.h | 10 +++++-----
 2 files changed, 13 insertions(+), 13 deletions(-)

kernel/rcu/tree.c +8 -8

index b0ad62b..7661887 100644
@@ -734,7 +734,7 @@ static int rcu_future_needs_gp(struct rcu_state *rsp)
 	int idx = (READ_ONCE(rnp->completed) + 1) & 0x1;
 	int *fp = &rnp->need_future_gp[idx];
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_future_needs_gp() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	return READ_ONCE(*fp);
 }

@@ -746,7 +746,7 @@ static int rcu_future_needs_gp(struct rcu_state *rsp)
 static bool
 cpu_needs_another_gp(struct rcu_state *rsp, struct rcu_data *rdp)
 {
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "cpu_needs_another_gp() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	if (rcu_gp_in_progress(rsp))
 		return false;  /* No, a grace period is already in progress. */
 	if (rcu_future_needs_gp(rsp))

@@ -773,7 +773,7 @@ static void rcu_eqs_enter_common(bool user)
 	struct rcu_data *rdp;
 	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_eqs_enter_common() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	trace_rcu_dyntick(TPS("Start"), rdtp->dynticks_nesting, 0);
 	if (IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
 	    !user && !is_idle_task(current)) {

@@ -840,7 +840,7 @@ static void rcu_eqs_enter(bool user)
  */
 void rcu_idle_enter(void)
 {
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_idle_enter() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	rcu_eqs_enter(false);
 }

@@ -855,7 +855,7 @@ void rcu_idle_enter(void)
  */
 void rcu_user_enter(void)
 {
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_user_enter() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	rcu_eqs_enter(true);
 }
 #endif /* CONFIG_NO_HZ_FULL */

@@ -880,7 +880,7 @@ void rcu_irq_exit(void)
 {
 	struct rcu_dynticks *rdtp;
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_irq_exit() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	rdtp = this_cpu_ptr(&rcu_dynticks);
 
 	/* Page faults can happen in NMI handlers, so check... */

@@ -947,7 +947,7 @@ static void rcu_eqs_exit(bool user)
 	struct rcu_dynticks *rdtp;
 	long long oldval;
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_eqs_exit() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	rdtp = this_cpu_ptr(&rcu_dynticks);
 	oldval = rdtp->dynticks_nesting;
 	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && oldval < 0);

@@ -1018,7 +1018,7 @@ void rcu_irq_enter(void)
 	struct rcu_dynticks *rdtp;
 	long long oldval;
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_irq_enter() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	rdtp = this_cpu_ptr(&rcu_dynticks);
 
 	/* Page faults can happen in NMI handlers, so check... */

kernel/rcu/tree_plugin.h +5 -5

index e012b9b..df08e5c 100644
@@ -325,7 +325,7 @@ static void rcu_preempt_note_context_switch(bool preempt)
 	struct rcu_data *rdp;
 	struct rcu_node *rnp;
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_preempt_note_context_switch() invoked with interrupts enabled!!!\n");
+	lockdep_assert_irqs_disabled();
 	WARN_ON_ONCE(!preempt && t->rcu_read_lock_nesting > 0);
 	if (t->rcu_read_lock_nesting > 0 &&
 	    !t->rcu_read_unlock_special.b.blocked) {

@@ -1421,7 +1421,7 @@ int rcu_needs_cpu(u64 basemono, u64 *nextevt)
 	struct rcu_dynticks *rdtp = this_cpu_ptr(&rcu_dynticks);
 	unsigned long dj;
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_needs_cpu() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 
 	/* Snapshot to detect later posting of non-lazy callback. */
 	rdtp->nonlazy_posted_snap = rdtp->nonlazy_posted;

@@ -1470,7 +1470,7 @@ static void rcu_prepare_for_idle(void)
 	struct rcu_state *rsp;
 	int tne;
 
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_prepare_for_idle() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	if (rcu_is_nocb_cpu(smp_processor_id()))
 		return;

@@ -1525,7 +1525,7 @@ static void rcu_prepare_for_idle(void)
  */
 static void rcu_cleanup_after_idle(void)
 {
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_cleanup_after_idle() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	if (rcu_is_nocb_cpu(smp_processor_id()))
 		return;
 	if (rcu_try_advance_all_cbs())

@@ -2012,7 +2012,7 @@ static bool __maybe_unused rcu_nocb_adopt_orphan_cbs(struct rcu_data *my_rdp,
 						     struct rcu_data *rdp,
 						     unsigned long flags)
 {
-	RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_nocb_adopt_orphan_cbs() invoked with irqs enabled!!!");
+	lockdep_assert_irqs_disabled();
 	if (!rcu_is_nocb_cpu(smp_processor_id()))
 		return false; /* Not NOCBs CPU, caller must migrate CBs. */
 	__call_rcu_nocb_enqueue(my_rdp, rcu_segcblist_head(&rdp->cblist),

Signature · 1 line

2.7.4