Re: irq lock inversion

[Jiri Kosina]

[ added LKML to CC so that the lockdep message is at least indexed by 
  search engines in archives ]

On Thu, 5 Nov 2009, Tejun Heo wrote:

> > lockdep only considers a lock irq-safe if it was used from an irq 
> > context before.
> > 
> > _irqsave() API usage alone does not trigger this.
> 
> Thanks for the explanation.  It's about the same tho.  sched_init() is
> calling it with irq disabled but that's an exception to might_sleep()
> rule.  Maybe making lockdep recognize the same exception as
> might_sleep() so that lockdep doesn't consider a lock irq-safe if it's
> called with irq off but before system_state is set to SYSTEM_RUNNING
> works around the problem?

Hmm, I wonder why I don't see this lockdep warning myself with 
head on 1836d9592, even though I have 

	CONFIG_PROVE_LOCKING=y
	CONFIG_TRACE_IRQFLAGS=y

... ?

Anyway, how about something like this? (I can't verify myself that it even 
fixes the warning, as I don't see it for some odd reason)




From: Jiri Kosina <jkosina@xxxxxxx>
Subject: lockdep: avoid false positives about irq-safety

Commit 403a91b1 ("percpu: allow pcpu_alloc() to be called
with IRQs off") introduced this warning:

	=========================================================
	[ INFO: possible irq lock inversion dependency detected ]
	2.6.32-rc5-tip-04815-g12f0f93-dirty #745
	---------------------------------------------------------
	hub 1-3:1.0: state 7 ports 2 chg 0000 evt 0004
	ksoftirqd/65/199 just changed the state of lock:
	 (pcpu_lock){..-...}, at: [<ffffffff81130e04>] free_percpu+0x38/0x104
	but this lock took another, SOFTIRQ-unsafe lock in the past:
	 (vmap_area_lock){+.+...}

	and interrupts could create inverse lock ordering between them.

This warning is bogus -- sched_init() is being called very early with IRQs
disabled, and the irqsave/restore code paths in pcpu_alloc() are only for early
init. The path can never be called from irq context once the early init
finishes. Rationale for this is explained in changelog of the commit mentioned
above.

This problem can be encountered generally in any other early code running
with IRQs off and using irqsave/irqrestore.

Reported-by: Yinghai Lu <yhlu.kernel@xxxxxxxxx>
Signed-off-by: Jiri Kosina <jkosina@xxxxxxx>

--- 
 kernel/lockdep.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 9af5672..996b395 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -2487,6 +2487,14 @@ void lockdep_trace_alloc(gfp_t gfp_mask)
 
 static int mark_irqflags(struct task_struct *curr, struct held_lock *hlock)
 {
+	/* 
+	 * This is exception similar to the might_sleep() one. 
+	 * We don't care about irq-safety of the locks this early, as
+	 * it will produce false positives (sched_init() is called with
+	 * irqs off, but needs to use irqsave/irqrestore API)
+	 */
+	if (system_state != SYSTEM_RUNNING)
+		return 1;
 	/*
 	 * If non-trylock use in a hardirq or softirq context, then
 	 * mark the lock as used in these contexts:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/