Mailing List Archive: Linux: Kernel

Re: irq lock inversion

Index | Next | Previous | View Threaded

jkosina at suse

Nov 5, 2009, 6:31 AM

Post #1 of 17 (97 views)
Permalink

Re: irq lock inversion

[. 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[at]suse.cz>
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[at]gmail.com>
Signed-off-by: Jiri Kosina <jkosina[at]suse.cz>

---
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[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

tj at kernel

Nov 5, 2009, 9:53 PM

Post #2 of 17 (90 views)
Permalink

Re: irq lock inversion [In reply to]

Hello, Jiri.

Jiri Kosina wrote:
> 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
>
> ... ?

You need pcpu_mem_free() hit vfree() to trigger the warning by
allocating a lot of small percpu areas so that allocation map inside a
chunk becomes larger than 4k and then get extended once more.

> 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[at]suse.cz>
> 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[at]gmail.com>
> Signed-off-by: Jiri Kosina <jkosina[at]suse.cz>

Looks good to me. Ingo, what do you think?

Thanks.

--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

mingo at elte

Nov 5, 2009, 11:17 PM

Post #3 of 17 (88 views)
Permalink

Re: irq lock inversion [In reply to]

* Tejun Heo <tj[at]kernel.org> wrote:

> > From: Jiri Kosina <jkosina[at]suse.cz>
> > 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[at]gmail.com>
> > Signed-off-by: Jiri Kosina <jkosina[at]suse.cz>
>
> Looks good to me. Ingo, what do you think?

Ugh, this explanation is _BOGUS_. As i said, taking a lock with irqs
disabled does _NOT_ mark a lock as 'irq safe' - if it did, we'd have
false positives left and right.

Read the lockdep message please, consider all the backtraces it prints,
it says something different.

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

tj at kernel

Nov 5, 2009, 11:45 PM

Post #4 of 17 (88 views)
Permalink

Re: irq lock inversion [In reply to]

Ingo Molnar wrote:
>>> 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[at]gmail.com>
>>> Signed-off-by: Jiri Kosina <jkosina[at]suse.cz>
>> Looks good to me. Ingo, what do you think?
>
> Ugh, this explanation is _BOGUS_. As i said, taking a lock with irqs
> disabled does _NOT_ mark a lock as 'irq safe' - if it did, we'd have
> false positives left and right.
>
> Read the lockdep message please, consider all the backtraces it prints,
> it says something different.

Ah... okay, the pcpu_free() path is correctly marking the lock
irqsafe. I assumed this was caused by recent pcpu_alloc() change.
Sorry about that. The lock inversion problem has always been there,
it just never showed up because none has use allocation map that large
I suppose.

So, the correct fix would be either 1. push down irqsafeness down to
vmalloc locks or 2. the rather ugly unlock-lock dancing in
pcpu_extend_area_map() I posted earlier. For 2.6.32, I guess we'll
have to go with #2. For longer term, we'll probably have to do #1 as
it's required to implement atomic percpu allocations too.

I'll try to reproduce the problem here and verify the previous locking
dance patch.

Thanks.

--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

mingo at elte

Nov 5, 2009, 11:58 PM

Post #5 of 17 (88 views)
Permalink

Re: irq lock inversion [In reply to]

* Tejun Heo <tj[at]kernel.org> wrote:

> Ingo Molnar wrote:
> >>> 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[at]gmail.com>
> >>> Signed-off-by: Jiri Kosina <jkosina[at]suse.cz>
> >> Looks good to me. Ingo, what do you think?
> >
> > Ugh, this explanation is _BOGUS_. As i said, taking a lock with irqs
> > disabled does _NOT_ mark a lock as 'irq safe' - if it did, we'd have
> > false positives left and right.
> >
> > Read the lockdep message please, consider all the backtraces it prints,
> > it says something different.
>
> Ah... okay, the pcpu_free() path is correctly marking the lock
> irqsafe. I assumed this was caused by recent pcpu_alloc() change.
> Sorry about that. The lock inversion problem has always been there,
> it just never showed up because none has use allocation map that large
> I suppose.
>
> So, the correct fix would be either 1. push down irqsafeness down to
> vmalloc locks or 2. the rather ugly unlock-lock dancing in
> pcpu_extend_area_map() I posted earlier. For 2.6.32, I guess we'll
> have to go with #2. For longer term, we'll probably have to do #1 as
> it's required to implement atomic percpu allocations too.
>
> I'll try to reproduce the problem here and verify the previous locking
> dance patch.

I havent looked deeply but at first sight i'm not 100% sure that even
the lock dance hack is safe - doesnt vfree() do TLB flushes, which must
be done with irqs enabled in general? If yes, then the whole notion of
using the allocator from irqs-off sections is wrong and the flags
save/restore is misguided (or at least incomplete).

So the real problem right now i think is the use of the pcpu allocator
from within a BH section (and from irqs-off sections) - that usage
should be eliminated from .32, or the allocator should be fixed. (which
looks non-trivial vmalloc/vfree was never really intended to be used in
irq-atomic contexts)

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

tj at kernel

Nov 6, 2009, 12:24 AM

Post #6 of 17 (86 views)
Permalink

Re: irq lock inversion [In reply to]

Hello, Ingo.

Ingo Molnar wrote:
> I havent looked deeply but at first sight i'm not 100% sure that even
> the lock dance hack is safe - doesnt vfree() do TLB flushes, which must
> be done with irqs enabled in general? If yes, then the whole notion of
> using the allocator from irqs-off sections is wrong and the flags
> save/restore is misguided (or at least incomplete).

The only place where any v*() call is nested under pcpu_lock is in the
alloc path, specifically pcpu_extend_area_map() ends up calling
vfree(). pcpu_free() path which can be called from irq context never
calls any vmalloc function directly. The reclaiming is deferred to a
work. Breaking the single nesting completely decouples the two locks
and nobody would be calling vfree() with irq disabled, so I don't
think there will be any problem.

Thanks.

--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

mingo at elte

Nov 6, 2009, 12:40 AM

Post #7 of 17 (86 views)
Permalink

Re: irq lock inversion [In reply to]

* Tejun Heo <tj[at]kernel.org> wrote:

> Hello, Ingo.
>
> Ingo Molnar wrote:
> > I havent looked deeply but at first sight i'm not 100% sure that even
> > the lock dance hack is safe - doesnt vfree() do TLB flushes, which must
> > be done with irqs enabled in general? If yes, then the whole notion of
> > using the allocator from irqs-off sections is wrong and the flags
> > save/restore is misguided (or at least incomplete).
>
> The only place where any v*() call is nested under pcpu_lock is in the
> alloc path, specifically pcpu_extend_area_map() ends up calling
> vfree(). pcpu_free() path which can be called from irq context never
> calls any vmalloc function directly. The reclaiming is deferred to a
> work. Breaking the single nesting completely decouples the two locks
> and nobody would be calling vfree() with irq disabled, so I don't
> think there will be any problem.

My question is, why do we do flags save/restore in pcpu-alloc? Do we
ever call it with irqs disabled? If yes, then the vfree might be unsafe
due to vfree() potentially flushing TLBs (on all CPUs) and that act of
sending IPIs requiring irqs to be enabled.

( Now, Nick has optimized vfree recently to lazy-free areas, but that
was a statistical optimization: TLB flushes are still possible, just
done more rarely. So we might end up calling flush_tlb_kernel_range()
from vfree(). I've Cc:-ed Nick. )

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

tj at kernel

Nov 6, 2009, 12:52 AM

Post #8 of 17 (86 views)
Permalink

Re: irq lock inversion [In reply to]

Ingo Molnar wrote:
> My question is, why do we do flags save/restore in pcpu-alloc?

That's strictly for calls from sched_init().

> Do we ever call it with irqs disabled? If yes, then the vfree might
> be unsafe due to vfree() potentially flushing TLBs (on all CPUs) and
> that act of sending IPIs requiring irqs to be enabled.

And when called from sched_init(), it won't call vfree().

> ( Now, Nick has optimized vfree recently to lazy-free areas, but that
> was a statistical optimization: TLB flushes are still possible, just
> done more rarely. So we might end up calling flush_tlb_kernel_range()
> from vfree(). I've Cc:-ed Nick. )

Nevertheless, it would be nice to allow at least the free part to be
called from irqsafe context. vmalloc is doing a lot of things lazily
so deferring TLB flushes to a work wouldn't make much difference, I
suppose?

Thanks.

--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

jens.axboe at oracle

Nov 6, 2009, 1:59 AM

Post #9 of 17 (82 views)
Permalink

Re: irq lock inversion [In reply to]

On Fri, Nov 06 2009, Ingo Molnar wrote:
> Read the lockdep message please, consider all the backtraces it prints,
> it says something different.

In all honesty, reading and parsing lockdep messages requires a special
state of mind. IOW, readability is not its high point.

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

cl at linux-foundation

Nov 6, 2009, 8:08 AM

Post #10 of 17 (77 views)
Permalink

Re: irq lock inversion [In reply to]

On Fri, 6 Nov 2009, Tejun Heo wrote:

> Ingo Molnar wrote:
> > My question is, why do we do flags save/restore in pcpu-alloc?
>
> That's strictly for calls from sched_init().

Right its a hack for 2.6.32. Fix it the right way by making the per cpu
allocator take gfp flags like any other allocator in the kernel.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

tj at kernel

Nov 6, 2009, 8:38 AM

Post #11 of 17 (73 views)
Permalink

Re: irq lock inversion [In reply to]

Christoph Lameter wrote:
> On Fri, 6 Nov 2009, Tejun Heo wrote:
>
>> Ingo Molnar wrote:
>>> My question is, why do we do flags save/restore in pcpu-alloc?
>> That's strictly for calls from sched_init().
>
> Right its a hack for 2.6.32. Fix it the right way by making the per cpu
> allocator take gfp flags like any other allocator in the kernel.

vmalloc/vfree is an allocator in the kernel and can't be called from
irq context and doesn't take gfp flags. percpu allocator being
dependent on vmalloc area, it's gonna be a bit tricky. It's
definitely doable but I'm still not quite sure whether the benfit
would worth the added complexity. The only known use case is for lazy
allocation from memory allocator, right? How much does it hurt not to
have that lazy allocation?

Thanks.

--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

cl at linux-foundation

Nov 6, 2009, 9:03 AM

Post #12 of 17 (73 views)
Permalink

Re: irq lock inversion [In reply to]

On Sat, 7 Nov 2009, Tejun Heo wrote:

> vmalloc/vfree is an allocator in the kernel and can't be called from
> irq context and doesn't take gfp flags. percpu allocator being
> dependent on vmalloc area, it's gonna be a bit tricky. It's
> definitely doable but I'm still not quite sure whether the benfit
> would worth the added complexity. The only known use case is for lazy
> allocation from memory allocator, right? How much does it hurt not to
> have that lazy allocation?

Nick did some work recently on the vmalloc subsystem and one of the
intents was (well I think so at least) to make it usable for fsblock which
requires virtual mappings. Maybe even from an atomic context.

Potential use cases for atomic percpu allocs exist all over the kernel. If
you need to allocate a structure in an atomic context then attaching per
cpu information to that structure will require also per cpu allocation in
an atomic context.

We discourage atomic allocations and they are rare. Atomic allocs are
allowed to fail. But they are occasionally necessary.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

peterz at infradead

Nov 7, 2009, 8:13 AM

Post #13 of 17 (60 views)
Permalink

Re: irq lock inversion [In reply to]

On Fri, 2009-11-06 at 12:03 -0500, Christoph Lameter wrote:
> Atomic allocs are allowed to fail.

All allocs (except __GFP_NOFAIL, but those are a deadlock waiting to
happen) are allowed to fail, !__GFP_WAIT allocs simply fail more easily.

The fact that __GFP_WAIT && order <= PAGE_ALLOC_COSTLY_ORDER allocations
currently don't fail but keep hammering the allocator could be
considered a bug (the recent OOM threads in fact suggest this is a
serious issue for some people).

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

mingo at elte

Nov 8, 2009, 1:38 AM

Post #14 of 17 (52 views)
Permalink

Re: irq lock inversion [In reply to]

* Jens Axboe <jens.axboe[at]oracle.com> wrote:

> On Fri, Nov 06 2009, Ingo Molnar wrote:
> > Read the lockdep message please, consider all the backtraces it prints,
> > it says something different.
>
> In all honesty, reading and parsing lockdep messages requires a
> special state of mind. IOW, readability is not its high point.

We frequently do patches to improve the messages but there's a hard
limit: generally the messages mirror the complexity of the underlying
locking scenario.

Unfortunately lockdep cannot pretend something is simple when it is not.
There are two ways out of that: either to simplify the underlying
locking rules, or to understand them.

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

jens.axboe at oracle

Nov 9, 2009, 7:34 AM

Post #15 of 17 (41 views)
Permalink

Re: irq lock inversion [In reply to]

On Sun, Nov 08 2009, Ingo Molnar wrote:
>
> * Jens Axboe <jens.axboe[at]oracle.com> wrote:
>
> > On Fri, Nov 06 2009, Ingo Molnar wrote:
> > > Read the lockdep message please, consider all the backtraces it prints,
> > > it says something different.
> >
> > In all honesty, reading and parsing lockdep messages requires a
> > special state of mind. IOW, readability is not its high point.
>
> We frequently do patches to improve the messages but there's a hard
> limit: generally the messages mirror the complexity of the underlying
> locking scenario.
>
> Unfortunately lockdep cannot pretend something is simple when it is not.
> There are two ways out of that: either to simplify the underlying
> locking rules, or to understand them.

I think the primary problem is that it tries to condense too much
information, instead of just spelling it out. That may be obvious to a
person intimately familiar with lockdep, but not to others. Things like
the STATE line, for instance. It would read a lot easier if these things
were just spelled out.

I know this message isn't really productive, just tossing it out there.
I'll try to to back it up with a patch the next time it annoys me :-)

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

mingo at elte

Nov 9, 2009, 7:45 AM

Post #16 of 17 (41 views)
Permalink

Re: irq lock inversion [In reply to]

* Jens Axboe <jens.axboe[at]oracle.com> wrote:

> On Sun, Nov 08 2009, Ingo Molnar wrote:
> >
> > * Jens Axboe <jens.axboe[at]oracle.com> wrote:
> >
> > > On Fri, Nov 06 2009, Ingo Molnar wrote:
> > > > Read the lockdep message please, consider all the backtraces it prints,
> > > > it says something different.
> > >
> > > In all honesty, reading and parsing lockdep messages requires a
> > > special state of mind. IOW, readability is not its high point.
> >
> > We frequently do patches to improve the messages but there's a hard
> > limit: generally the messages mirror the complexity of the underlying
> > locking scenario.
> >
> > Unfortunately lockdep cannot pretend something is simple when it is not.
> > There are two ways out of that: either to simplify the underlying
> > locking rules, or to understand them.
>
> I think the primary problem is that it tries to condense too much
> information, instead of just spelling it out. That may be obvious to a
> person intimately familiar with lockdep, but not to others. Things
> like the STATE line, for instance. It would read a lot easier if these
> things were just spelled out.
>
> I know this message isn't really productive, just tossing it out
> there. I'll try to to back it up with a patch the next time it annoys
> me :-)

Well, previously lockdep spewed out a lot of info, which we condensed
down because people complained ;-)

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

jens.axboe at oracle

Nov 9, 2009, 7:49 AM

Post #17 of 17 (41 views)
Permalink

Re: irq lock inversion [In reply to]

On Mon, Nov 09 2009, Ingo Molnar wrote:
>
> * Jens Axboe <jens.axboe[at]oracle.com> wrote:
>
> > On Sun, Nov 08 2009, Ingo Molnar wrote:
> > >
> > > * Jens Axboe <jens.axboe[at]oracle.com> wrote:
> > >
> > > > On Fri, Nov 06 2009, Ingo Molnar wrote:
> > > > > Read the lockdep message please, consider all the backtraces it prints,
> > > > > it says something different.
> > > >
> > > > In all honesty, reading and parsing lockdep messages requires a
> > > > special state of mind. IOW, readability is not its high point.
> > >
> > > We frequently do patches to improve the messages but there's a hard
> > > limit: generally the messages mirror the complexity of the underlying
> > > locking scenario.
> > >
> > > Unfortunately lockdep cannot pretend something is simple when it is not.
> > > There are two ways out of that: either to simplify the underlying
> > > locking rules, or to understand them.
> >
> > I think the primary problem is that it tries to condense too much
> > information, instead of just spelling it out. That may be obvious to a
> > person intimately familiar with lockdep, but not to others. Things
> > like the STATE line, for instance. It would read a lot easier if these
> > things were just spelled out.
> >
> > I know this message isn't really productive, just tossing it out
> > there. I'll try to to back it up with a patch the next time it annoys
> > me :-)
>
> Well, previously lockdep spewed out a lot of info, which we condensed
> down because people complained ;-)

Heh, can't win 'em all! Stack traces are so large anyway that I don't
think saving 1-2 lines per lock in the trace would make much of a
difference. It's debug output, after all.

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo[at]vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact lists@gossamer-threads.com