* Peter Zijlstra <[EMAIL PROTECTED]> wrote:
> Add lock contention tracking to lockdep
>
> [EMAIL PROTECTED] ~]# cat /proc/lockdep_contentions | sort -rnk 2 | head
> dcache_lock: 3000 0 [618] [<ffffffff8033badd>] _atomic_dec_and_lock+0x39/0x58
> [17] [<ffffffff802d6dce>] sysfs_open_file+0x28/0x25a [160]
> [<ffffffff802a5b61>] d_instantiate+0x2a/0xad [52] [<ffffffff8029d926>]
> __link_path_walk+0x270/0xde9
> &inode->i_mutex: 235 0 [15] [<ffffffff8029b053>] pipe_read+0x74/0x39d [23]
> [<ffffffff8029a9f8>] pipe_wait+0x86/0x8d [105] [<ffffffff8029bc9a>]
> do_lookup+0x81/0x1ae [85] [<ffffffff802a0be7>] vfs_readdir+0x5c/0xa9
> &zone->lru_lock: 141 0 [11] [<ffffffff8027366b>] activate_page+0x37/0xd6 [75]
> [<ffffffff80273327>] release_pages+0xb2/0x16c [42] [<ffffffff80273443>]
> __pagevec_lru_add_active+0x62/0xe5 [12] [<ffffffff80273529>]
> __pagevec_lru_add+0x63/0xe2
looks really nice!
> 6 files changed, 266 insertions(+), 18 deletions(-)
wow, that's really simple!
in general i like this alot because it cleverly reuses all the lock API
wrappers and lock tracking infrastructure that lockdep already adds.
Acked-by: Ingo Molnar <[EMAIL PROTECTED]>
i'd suggest to add a CONFIG_DEBUG_LOCK_STAT Kconfig method to turn this
on without any of the other lock-dependency tracking costs that lockdep
has.
here are a few minor nits about the code:
> +struct lockdep_contention_point {
> + unsigned long ip;
> + atomic_t count;
> +};
i'd say rename this to "lock_contention_point" - this is not really a
lockdep thing. Lockdep offers the machinery to track lock usage, but
there's no "dependency" portion of this.
> + printk("%s/%d is trying to content lock (",
s/content/contend
> +static int lockdep_contentions_show(struct seq_file *m, void *v)
> +ssize_t lockdep_contentions_write(struct file *file, const char __user *buf,
s/lockdep/lock
> + for (i = 0; i < ARRAY_SIZE(class->contention_point);
> + i++)
> + class->contention_point[i].ip = 0;
(loop body needs curly braces because the whole construct is 3-line.)
> + entry = create_proc_entry("lockdep_contentions", S_IRUSR, NULL);
s/lockdep/lock
> @@ -20,7 +20,10 @@ void down_read(struct rw_semaphore *sem)
> might_sleep();
> rwsem_acquire_read(&sem->dep_map, 0, 0, _RET_IP_);
>
> - __down_read(sem);
> + if (!__down_read_trylock(sem)) {
> + lock_contended(&sem->dep_map, 1, _RET_IP_);
> + __down_read(sem);
> + }
heh, clever trick that avoids the touching of dozens of architectures
;-) I guess make this dependent on CONFIG_LOCK_STAT though, because gcc
cannot optimize this away even if lock_contended() is a NOP.
(same for the other trylock additions in rwsem.c and spinlock.c)
Ingo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
