07 May, 2009
1 commit
-
Merge reason: tracing/core was on a .30-rc1 base and was missing out on
on a handful of tracing fixes present in .30-rc5-almost.Signed-off-by: Ingo Molnar
18 Apr, 2009
1 commit
-
Steven Rostedt reported:
> OK, I think I figured this bug out. This is a lockdep issue with respect
> to tracepoints.
>
> The trace points in lockdep are called all the time. Outside the lockdep
> logic. But if lockdep were to trigger an error / warning (which this run
> did) we might be in trouble. For new locks, like the dentry->d_lock, that
> are created, they will not get a name:
>
> void lockdep_init_map(struct lockdep_map *lock, const char *name,
> struct lock_class_key *key, int subclass)
> {
> if (unlikely(!debug_locks))
> return;
>
> When a problem is found by lockdep, debug_locks becomes false. Thus we
> stop allocating names for locks. This dentry->d_lock I had, now has no
> name. Worse yet, I have CONFIG_DEBUG_VM set, that scrambles non
> initialized memory. Thus, when the trace point was hit, it had junk for
> the lock->name, and the machine crashed.Ah, nice catch. I think we should put at least the name in regardless.
Ensure we at least initialize the trivial entries of the depmap so that
they can be relied upon, even when lockdep itself decided to pack up and
go home.[ Impact: fix lock tracing after lockdep warnings. ]
Reported-by: Steven Rostedt
Signed-off-by: Peter Zijlstra
Acked-by: Steven Rostedt
Cc: Andrew Morton
Cc: Frederic Weisbecker
LKML-Reference:
Signed-off-by: Ingo Molnar
15 Apr, 2009
2 commits
-
Impact: clean up
Create a sub directory in include/trace called events to keep the
trace point headers in their own separate directory. Only headers that
declare trace points should be defined in this directory.Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: Neil Horman
Cc: Zhao Lei
Cc: Eduard - Gabriel Munteanu
Cc: Pekka Enberg
Signed-off-by: Steven Rostedt -
This patch lowers the number of places a developer must modify to add
new tracepoints. The current method to add a new tracepoint
into an existing system is to write the trace point macro in the
trace header with one of the macros TRACE_EVENT, TRACE_FORMAT or
DECLARE_TRACE, then they must add the same named item into the C file
with the macro DEFINE_TRACE(name) and then add the trace point.This change cuts out the needing to add the DEFINE_TRACE(name).
Every file that uses the tracepoint must still include the trace/.h
file, but the one C file must also add a define before the including
of that file.#define CREATE_TRACE_POINTS
#includeThis will cause the trace/mytrace.h file to also produce the C code
necessary to implement the trace point.Note, if more than one trace/.h is used to create the C code
it is best to list them all together.#define CREATE_TRACE_POINTS
#include
#include
#includeThanks to Mathieu Desnoyers and Christoph Hellwig for coming up with
the cleaner solution of the define above the includes over my first
design to have the C code include a "special" header.This patch converts sched, irq and lockdep and skb to use this new
method.Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: Neil Horman
Cc: Zhao Lei
Cc: Eduard - Gabriel Munteanu
Cc: Pekka Enberg
Signed-off-by: Steven Rostedt
10 Apr, 2009
1 commit
-
While trying to optimize the new lock on reiserfs to replace
the bkl, I find the lock tracing very useful though it lacks
something important for performance (and latency) instrumentation:
the time a task waits for a lock.That's what this patch implements:
bash-4816 [000] 202.652815: lock_contended: lock_contended: &sb->s_type->i_mutex_key
bash-4816 [000] 202.652819: lock_acquired: &rq->lock (0.000 us)
-4787 [000] 202.652825: lock_acquired: &rq->lock (0.000 us)
-4787 [000] 202.652829: lock_acquired: &rq->lock (0.000 us)
bash-4816 [000] 202.652833: lock_acquired: &sb->s_type->i_mutex_key (16.005 us)As shown above, the "lock acquired" field is followed by the time
it has been waiting for the lock. Usually, a lock contended entry
is followed by a near lock_acquired entry with a non-zero time waited.Signed-off-by: Frederic Weisbecker
Acked-by: Peter Zijlstra
Cc: Steven Rostedt
LKML-Reference:
Signed-off-by: Ingo Molnar
07 Apr, 2009
1 commit
-
* 'locking-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip:
lockdep: add stack dumps to asserts
hrtimer: fix rq->lock inversion (again)
02 Apr, 2009
1 commit
-
Conflicts:
include/linux/slub_def.h
lib/Kconfig.debug
mm/slob.c
mm/slub.c
31 Mar, 2009
3 commits
-
Have a better idea about exactly which loc causes a lockdep
limit overflow. Often it's a bug or inefficiency in that
subsystem.Signed-off-by: Peter Zijlstra
LKML-Reference:
Signed-off-by: Ingo Molnar -
Conflicts:
lib/Kconfig.debug -
Heiko reported that we grab the graph lock with irqs enabled.
Fix this by providng the same wrapper as all other lockdep entry
functions have.Reported-by: Heiko Carstens
Signed-off-by: Peter Zijlstra
Cc: Nick Piggin
LKML-Reference:
Signed-off-by: Ingo Molnar
13 Mar, 2009
1 commit
05 Mar, 2009
4 commits
-
Impact: cleanup
The atomic debug modifiers are already defined in
kernel/lockdep_internals.h.Signed-off-by: David Rientjes
Acked-by: Peter Zijlstra
LKML-Reference:
Signed-off-by: Ingo Molnar -
Augment the traces with lock names when lockdep is available:
1) | down_read_trylock() {
1) | _spin_lock_irqsave() {
1) | /* lock_acquire: &sem->wait_lock */
1) 4.201 us | }
1) | _spin_unlock_irqrestore() {
1) | /* lock_release: &sem->wait_lock */
1) 3.523 us | }
1) | /* lock_acquire: try read &mm->mmap_sem */
1) + 13.386 us | }
1) 1.635 us | find_vma();
1) | handle_mm_fault() {
1) | __do_fault() {
1) | filemap_fault() {
1) | find_lock_page() {
1) | find_get_page() {
1) | /* lock_acquire: read rcu_read_lock */
1) | /* lock_release: rcu_read_lock */
1) 5.697 us | }
1) 8.158 us | }
1) + 11.079 us | }
1) | _spin_lock() {
1) | /* lock_acquire: __pte_lockptr(page) */
1) 3.949 us | }
1) 1.460 us | page_add_file_rmap();
1) | _spin_unlock() {
1) | /* lock_release: __pte_lockptr(page) */
1) 3.115 us | }
1) | unlock_page() {
1) 1.421 us | page_waitqueue();
1) 1.220 us | __wake_up_bit();
1) 6.519 us | }
1) + 34.328 us | }
1) + 37.452 us | }
1) | up_read() {
1) | /* lock_release: &mm->mmap_sem */
1) | _spin_lock_irqsave() {
1) | /* lock_acquire: &sem->wait_lock */
1) 3.865 us | }
1) | _spin_unlock_irqrestore() {
1) | /* lock_release: &sem->wait_lock */
1) 8.562 us | }
1) + 17.370 us | }Signed-off-by: Peter Zijlstra
Cc: Steven Rostedt
Cc: =?ISO-8859-1?Q?T=F6r=F6k?= Edwin
Cc: Jason Baron
Cc: Frederic Weisbecker
LKML-Reference:
Signed-off-by: Ingo Molnar -
Impact: clarify lockdep printk text
print_irq_inversion_bug() gets handed state strings of the form
"HARDIRQ", "SOFTIRQ", "RECLAIM_FS"
and appends "-irq-{un,}safe" to them, which is either redudant for *IRQ or
confusing in the RECLAIM_FS case.Signed-off-by: Peter Zijlstra
LKML-Reference:
Signed-off-by: Ingo Molnar -
In the recent mark_lock_irq() rework a bug snuck in that would report the
state of write locks causing irq inversion under a read lock as a read
lock.Fix this by masking the read bit of the state when validating write
dependencies.Reported-by: Andrew Morton
Signed-off-by: Peter Zijlstra
LKML-Reference:
Signed-off-by: Ingo Molnar
15 Feb, 2009
20 commits
-
The __GFP_FS annotations fail to build with CONFIG_LOCKDEP=y,
CONFIG_PROVE_LOCKING=n, ammend that.Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
Arnd pointed out we have the stringify macro magic already in-kernel.
Signed-off-by: Peter Zijlstra
CC: Arnd Bergmann
Signed-off-by: Ingo Molnar -
Remove the manual state iteration thingy.
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
Generic, states independent, get_user_chars().
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
there's too much repetition of code..
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
re-add some of the comments that got lost in the refactoring.
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
Now that we have nice numerical relations for the states, remove the macro
magics.Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
Now what its only two functions, they again look rather similar.
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
These two are also remakably similar
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
The _READ helpers show remarkable similarity, merge them.
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
Kill another argument
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
take away another parameter
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
In order to unify them, take some arguments away
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
split mark_lock_irq() into 4 simple helper functions
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
generate the usage strings
XXX capital invasion :-(
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
remove the state iteration
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
remove the explicit state iteration
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
s/HELD_OVER/ENABLED/g
so that its similar to the hard and soft-irq names.
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
s/\(LOCKF\?_ENABLED_[^ ]*\)S\(_READ\)\?\>/\1\2/g
So that the USED_IN and ENABLED have the same names.
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar -
Here is another version, with the incremental patch rolled up, and
added reclaim context annotation to kswapd, and allocation tracing
to slab allocators (which may only ever reach the page allocator
in rare cases, so it is good to put annotations here too).Haven't tested this version as such, but it should be getting closer
to merge worthy ;)--
After noticing some code in mm/filemap.c accidentally perform a __GFP_FS
allocation when it should not have been, I thought it might be a good idea to
try to catch this kind of thing with lockdep.I coded up a little idea that seems to work. Unfortunately the system has to
actually be in __GFP_FS page reclaim, then take the lock, before it will mark
it. But at least that might still be some orders of magnitude more common
(and more debuggable) than an actual deadlock condition, so we have some
improvement I hope (the concept is no less complete than discovery of a lock's
interrupt contexts).I guess we could even do the same thing with __GFP_IO (normal reclaim), and
even GFP_NOIO locks too... but filesystems will have the most locks and fiddly
code paths, so let's start there and see how it goes.It *seems* to work. I did a quick test.
=================================
[ INFO: inconsistent lock state ]
2.6.28-rc6-00007-ged31348-dirty #26
---------------------------------
inconsistent {in-reclaim-W} -> {ov-reclaim-W} usage.
modprobe/8526 [HC0[0]:SC0[0]:HE1:SE1] takes:
(testlock){--..}, at: [] brd_init+0x55/0x216 [brd]
{in-reclaim-W} state was registered at:
[] __lock_acquire+0x75b/0x1a60
[] lock_acquire+0x91/0xc0
[] mutex_lock_nested+0xb1/0x310
[] brd_init+0x2b/0x216 [brd]
[] _stext+0x3b/0x170
[] sys_init_module+0xaf/0x1e0
[] system_call_fastpath+0x16/0x1b
[] 0xffffffffffffffff
irq event stamp: 3929
hardirqs last enabled at (3929): [] mutex_lock_nested+0x285/0x310
hardirqs last disabled at (3928): [] mutex_lock_nested+0x59/0x310
softirqs last enabled at (3732): [] sk_filter+0x83/0xe0
softirqs last disabled at (3730): [] sk_filter+0x16/0xe0other info that might help us debug this:
1 lock held by modprobe/8526:
#0: (testlock){--..}, at: [] brd_init+0x55/0x216 [brd]stack backtrace:
Pid: 8526, comm: modprobe Not tainted 2.6.28-rc6-00007-ged31348-dirty #26
Call Trace:
[] print_usage_bug+0x193/0x1d0
[] mark_lock+0xaf0/0xca0
[] mark_held_locks+0x55/0xc0
[] ? brd_init+0x0/0x216 [brd]
[] trace_reclaim_fs+0x2a/0x60
[] __alloc_pages_internal+0x475/0x580
[] ? mutex_lock_nested+0x26e/0x310
[] ? brd_init+0x0/0x216 [brd]
[] brd_init+0x6a/0x216 [brd]
[] ? brd_init+0x0/0x216 [brd]
[] _stext+0x3b/0x170
[] ? mutex_unlock+0x9/0x10
[] ? __mutex_unlock_slowpath+0x10d/0x180
[] ? trace_hardirqs_on_caller+0x12c/0x190
[] sys_init_module+0xaf/0x1e0
[] system_call_fastpath+0x16/0x1bSigned-off-by: Nick Piggin
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar
31 Dec, 2008
1 commit
-
* 'core-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip: (63 commits)
stacktrace: provide save_stack_trace_tsk() weak alias
rcu: provide RCU options on non-preempt architectures too
printk: fix discarding message when recursion_bug
futex: clean up futex_(un)lock_pi fault handling
"Tree RCU": scalable classic RCU implementation
futex: rename field in futex_q to clarify single waiter semantics
x86/swiotlb: add default swiotlb_arch_range_needs_mapping
x86/swiotlb: add default physbus conversion
x86: unify pci iommu setup and allow swiotlb to compile for 32 bit
x86: add swiotlb allocation functions
swiotlb: consolidate swiotlb info message printing
swiotlb: support bouncing of HighMem pages
swiotlb: factor out copy to/from device
swiotlb: add arch hook to force mapping
swiotlb: allow architectures to override physbusphys conversions
swiotlb: add comment where we handle the overflow of a dma mask on 32 bit
rcu: fix rcutorture behavior during reboot
resources: skip sanity check of busy resources
swiotlb: move some definitions to header
swiotlb: allow architectures to override swiotlb pool allocation
...Fix up trivial conflicts in
arch/x86/kernel/Makefile
arch/x86/mm/init_32.c
include/linux/hardirq.h
as per Ingo's suggestions.
04 Dec, 2008
2 commits
-
Impact: introduce new lockdep API
Allow to change a held lock's class. Basically the same as the existing
code to change a subclass therefore reuse all that.The XFS code will be able to use this to annotate their inode locking.
Signed-off-by: Peter Zijlstra
Signed-off-by: Ingo Molnar
03 Dec, 2008
1 commit
-
Impact: fix for lockdep and ftrace
The raw_local_irq_save/restore confuses lockdep. This patch
converts them to the local_irq_save/restore variants.Signed-off-by: Steven Rostedt
Signed-off-by: Ingo Molnar
25 Nov, 2008
1 commit
-
Impact: fix build warning
this warning:
kernel/lockdep.c:584: warning: ‘print_lock_dependencies’ defined but not used
triggers because print_lock_dependencies() is only used if both
CONFIG_TRACE_IRQFLAGS and CONFIG_PROVE_LOCKING are enabled.But adding #ifdefs is not an option here - it would spread out to 4-5
other helper functions and uglify the file. So mark this function
as __used - it's static and the compiler can eliminate it just fine.Signed-off-by: Ingo Molnar