From: Ingo Molnar <mingo@elte.hu>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Andrew Morton <akpm@osdl.org>,
linux-kernel@vger.kernel.org,
Alexander Viro <viro@ftp.linux.org.uk>
Subject: Re: [patch, validator] fix proc_subdir_lock related deadlock
Date: Wed, 25 Jan 2006 19:08:11 +0100 [thread overview]
Message-ID: <20060125180811.GA12762@elte.hu> (raw)
In-Reply-To: <1138209283.6695.55.camel@localhost.localdomain>
* Steven Rostedt <rostedt@goodmis.org> wrote:
> > proc_subdir_lock can also be used from softirq (tasklet) context, which
> > may lead to deadlocks.
> >
> > This bug was found via the lock validator:
> >
>
> Thanks Ingo,
>
> I stressed in sending the patch that there was a big assumption that
> the calls would not be done in (soft)irq context. I just didn't want
> to add overhead if it wasn't needed. But I guess that this is needed
> until we can remove all the instances that use it in softirq context.
> But that's for a later patch.
the validator just found another problem with this lock, pointing out
that files_lock nests inside of proc_subdir_lock, and that files_lock is
a softirq-unsafe lock, creating another (unlikely but possible) deadlock
scenario:
=====================================
[ BUG: lock inversion bug detected! ]
-------------------------------------
grep/2290 just changed the state of lock {proc_subdir_lock} at:
[<c0196e53>] remove_proc_entry+0x33/0x1f0
but this lock took lock {files_lock} in the past,
acquired at: [<c0196ece>] remove_proc_entry+0xae/0x1f0
and interrupts could create an inverse lock dependency between them,
which could lead to deadlocks!
other info that might help in debugging this:
------------------------------
| showing all locks held by: | (grep/2290 [c321c790, 125]):
------------------------------
[<c010432d>] show_trace+0xd/0x10
[<c0104347>] dump_stack+0x17/0x20
[<c0137b11>] check_no_lock_2_mask+0x131/0x180
[<c0137ffb>] mark_lock+0xfb/0x2a0
[<c01387b3>] debug_lock_chain+0x613/0x10d0
[<c01392ad>] debug_lock_chain_spin+0x3d/0x60
[<c02656ed>] _raw_spin_lock+0x2d/0x90
[<c04d88d2>] _spin_lock_bh+0x12/0x20
[<c0196e53>] remove_proc_entry+0x33/0x1f0
[<c01427c9>] unregister_handler_proc+0x19/0x20
[<c0141f8b>] free_irq+0x7b/0xe0
[<c02f2302>] floppy_release_irq_and_dma+0x1b2/0x210
[<c02f07f7>] set_dor+0xc7/0x1b0
[<c02f3871>] motor_off_callback+0x21/0x30
[<c01273a5>] run_timer_softirq+0xf5/0x1f0
[<c0122cf7>] __do_softirq+0x97/0x130
[<c0105519>] do_softirq+0x69/0x100
=======================
[<c01229a9>] irq_exit+0x39/0x50
[<c010f4cc>] smp_apic_timer_interrupt+0x4c/0x50
[<c010393b>] apic_timer_interrupt+0x27/0x2c
to solve this we must either change files_lock to be softirq-safe too
(bleh!), or we must forbid remove_proc_entry() use from softirq
contexts. Neither is a happy solution - remove_proc_entry() is used
within free_irq(), and who knows how many drivers do free_irq() in
softirq/tasklet context ...
Andrew, this needs to be resolved before v2.6.16, correct? Steve's patch
solves a real bug in the upstream kernel.
Ingo
next prev parent reply other threads:[~2006-01-25 18:07 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-01-25 17:03 [patch, validator] fix proc_subdir_lock related deadlock Ingo Molnar
2006-01-25 17:14 ` Steven Rostedt
2006-01-25 18:08 ` Ingo Molnar [this message]
2006-01-25 18:14 ` [patch, validator] fix files_lock " Ingo Molnar
2006-01-25 18:23 ` [patch, validator] fix proc_subdir_lock " Andrew Morton
2006-01-25 20:21 ` Ingo Molnar
2006-01-26 0:02 ` [patch, lock validator] fix proc_inum_lock " Ingo Molnar
2006-01-26 0:11 ` Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20060125180811.GA12762@elte.hu \
--to=mingo@elte.hu \
--cc=akpm@osdl.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=viro@ftp.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.