12 Oct, 2007

2 commits

  • Provide a check to validate that we do not hold any locks when switching
    back to user-space.

    Signed-off-by: Peter Zijlstra
    Signed-off-by: Ingo Molnar

    Peter Zijlstra
     
  • It is possible for the current->curr_chain_key to become inconsistent with the
    current index if the chain fails to validate. The end result is that future
    lock_acquire() operations may inadvertently fail to find a hit in the cache
    resulting in a new node being added to the graph for every acquire.

    Signed-off-by: Gregory Haskins
    Signed-off-by: Peter Zijlstra
    Signed-off-by: Ingo Molnar

    Gregory Haskins
     

20 Jul, 2007

7 commits

  • When I started adding support for lockdep to 64-bit powerpc, I got a
    lockdep_init_error and with this patch was able to pinpoint why and where
    to put lockdep_init(). Let's support this generally for others adding
    lockdep support to their architecture.

    Signed-off-by: Johannes Berg
    Cc: Ingo Molnar
    Cc: Peter Zijlstra
    Cc: Arjan van de Ven
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Johannes Berg
     
  • __acquire
    |
    lock _____
    | \
    | __contended
    | |
    | wait
    | _______/
    |/
    |
    __acquired
    |
    __release
    |
    unlock

    We measure acquisition and contention bouncing.

    This is done by recording a cpu stamp in each lock instance.

    Contention bouncing requires the cpu stamp to be set on acquisition. Hence we
    move __acquired into the generic path.

    __acquired is then used to measure acquisition bouncing by comparing the
    current cpu with the old stamp before replacing it.

    __contended is used to measure contention bouncing (only useful for preemptable
    locks)

    [akpm@linux-foundation.org: cleanups]
    Signed-off-by: Peter Zijlstra
    Acked-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Peter Zijlstra
     
  • - update the copyright notices
    - use the default hash function
    - fix a thinko in a BUILD_BUG_ON
    - add a WARN_ON to spot inconsitent naming
    - fix a termination issue in /proc/lock_stat

    [akpm@linux-foundation.org: cleanups]
    Signed-off-by: Peter Zijlstra
    Acked-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Peter Zijlstra
     
  • Present all this fancy new lock statistics information:

    *warning, _wide_ output ahead*

    (output edited for purpose of brevity)

    # cat /proc/lock_stat
    lock_stat version 0.1
    -----------------------------------------------------------------------------------------------------------------------------------------------------------------
    class name contentions waittime-min waittime-max waittime-total acquisitions holdtime-min holdtime-max holdtime-total
    -----------------------------------------------------------------------------------------------------------------------------------------------------------------

    &inode->i_mutex: 14458 6.57 398832.75 2469412.23 6768876 0.34 11398383.65 339410830.89
    ---------------
    &inode->i_mutex 4486 [] pipe_wait+0x86/0x8d
    &inode->i_mutex 0 [] pipe_write_fasync+0x29/0x5d
    &inode->i_mutex 0 [] pipe_read+0x74/0x3a5
    &inode->i_mutex 0 [] do_lookup+0x81/0x1ae

    .................................................................................................................................................................

    &inode->i_data.tree_lock-W: 491 0.27 62.47 493.89 2477833 0.39 468.89 1146584.25
    &inode->i_data.tree_lock-R: 65 0.44 4.27 48.78 26288792 0.36 184.62 10197458.24
    --------------------------
    &inode->i_data.tree_lock 46 [] __do_page_cache_readahead+0x69/0x24f
    &inode->i_data.tree_lock 31 [] add_to_page_cache+0x31/0xba
    &inode->i_data.tree_lock 0 [] __do_page_cache_readahead+0xc2/0x24f
    &inode->i_data.tree_lock 0 [] find_get_page+0x1a/0x58

    .................................................................................................................................................................

    proc_inum_idr.lock: 0 0.00 0.00 0.00 36 0.00 65.60 148.26
    proc_subdir_lock: 0 0.00 0.00 0.00 3049859 0.00 106.81 1563212.42
    shrinker_rwsem-W: 0 0.00 0.00 0.00 5 0.00 1.73 3.68
    shrinker_rwsem-R: 0 0.00 0.00 0.00 633 2.57 246.57 10909.76

    'contentions' and 'acquisitions' are the number of such events measured (since
    the last reset). The waittime- and holdtime- (min, max, total) numbers are
    presented in microseconds.

    If there are any contention points, the lock class is presented in the block
    format (as i_mutex and tree_lock above), otherwise a single line of output is
    presented.

    The output is sorted on absolute number of contentions (read + write), this
    should get the worst offenders presented first, so that:

    # grep : /proc/lock_stat | head

    will quickly show who's bad.

    The stats can be reset using:

    # echo 0 > /proc/lock_stat

    [bunk@stusta.de: make 2 functions static]
    [akpm@linux-foundation.org: fix printk warning]
    Signed-off-by: Peter Zijlstra
    Acked-by: Ingo Molnar
    Acked-by: Jason Baron
    Signed-off-by: Adrian Bunk
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Peter Zijlstra
     
  • Introduce the core lock statistics code.

    Lock statistics provides lock wait-time and hold-time (as well as the count
    of corresponding contention and acquisitions events). Also, the first few
    call-sites that encounter contention are tracked.

    Lock wait-time is the time spent waiting on the lock. This provides insight
    into the locking scheme, that is, a heavily contended lock is indicative of
    a too coarse locking scheme.

    Lock hold-time is the duration the lock was held, this provides a reference for
    the wait-time numbers, so they can be put into perspective.

    1)
    lock
    2)
    ... do stuff ..
    unlock
    3)

    The time between 1 and 2 is the wait-time. The time between 2 and 3 is the
    hold-time.

    The lockdep held-lock tracking code is reused, because it already collects locks
    into meaningful groups (classes), and because it is an existing infrastructure
    for lock instrumentation.

    Currently lockdep tracks lock acquisition with two hooks:

    lock()
    lock_acquire()
    _lock()

    ... code protected by lock ...

    unlock()
    lock_release()
    _unlock()

    We need to extend this with two more hooks, in order to measure contention.

    lock_contended() - used to measure contention events
    lock_acquired() - completion of the contention

    These are then placed the following way:

    lock()
    lock_acquire()
    if (!_try_lock())
    lock_contended()
    _lock()
    lock_acquired()

    ... do locked stuff ...

    unlock()
    lock_release()
    _unlock()

    (Note: the try_lock() 'trick' is used to avoid instrumenting all platform
    dependent lock primitive implementations.)

    It is also possible to toggle the two lockdep features at runtime using:

    /proc/sys/kernel/prove_locking
    /proc/sys/kernel/lock_stat

    (esp. turning off the O(n^2) prove_locking functionaliy can help)

    [akpm@linux-foundation.org: build fixes]
    [akpm@linux-foundation.org: nuke unneeded ifdefs]
    Signed-off-by: Peter Zijlstra
    Acked-by: Ingo Molnar
    Acked-by: Jason Baron
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Peter Zijlstra
     
  • Move code around to get fewer but larger #ifdef sections. Break some
    in-function #ifdefs out into their own functions.

    Signed-off-by: Peter Zijlstra
    Acked-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Peter Zijlstra
     
  • Ensure that all of the lock dependency tracking code is under
    CONFIG_PROVE_LOCKING. This allows us to use the held lock tracking code for
    other purposes.

    Signed-off-by: Peter Zijlstra
    Acked-by: Ingo Molnar
    Acked-by: Jason Baron
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Peter Zijlstra
     

18 Jul, 2007

1 commit

  • KSYM_NAME_LEN is peculiar in that it does not include the space for the
    trailing '\0', forcing all users to use KSYM_NAME_LEN + 1 when allocating
    buffer. This is nonsense and error-prone. Moreover, when the caller
    forgets that it's very likely to subtly bite back by corrupting the stack
    because the last position of the buffer is always cleared to zero.

    This patch increments KSYM_NAME_LEN by one and updates code accordingly.

    * off-by-one bug in asm-powerpc/kprobes.h::kprobe_lookup_name() macro
    is fixed.

    * Where MODULE_NAME_LEN and KSYM_NAME_LEN were used together,
    MODULE_NAME_LEN was treated as if it didn't include space for the
    trailing '\0'. Fix it.

    Signed-off-by: Tejun Heo
    Acked-by: Paulo Marques
    Cc: Benjamin Herrenschmidt
    Cc: Paul Mackerras
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Tejun Heo
     

09 May, 2007

4 commits


23 Mar, 2007

1 commit

  • lockdep's data shouldn't be used when debug_locks == 0 because it's not
    updated after this, so it's more misleading than helpful.

    PS: probably lockdep's current-> fields should be reset after it turns
    debug_locks off: so, after printing a bug report, but before return from
    exported functions, but there are really a lot of these possibilities (e.g.
    after DEBUG_LOCKS_WARN_ON), so, something could be missed. (Of course
    direct use of this fields isn't recommended either.)

    Reported-by: Folkert van Heusden
    Inspired-by: Oleg Nesterov
    Signed-off-by: Jarek Poplawski
    Acked-by: Peter Zijlstra
    Cc: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Jarek Poplawski
     

02 Mar, 2007

1 commit

  • lockdep_init() is marked __init but used in several places
    outside __init code. This causes following warnings:
    $ scripts/mod/modpost kernel/lockdep.o
    WARNING: kernel/built-in.o - Section mismatch: reference to .init.text:lockdep_init from .text.lockdep_init_map after 'lockdep_init_map' (at offset 0x105)
    WARNING: kernel/built-in.o - Section mismatch: reference to .init.text:lockdep_init from .text.lockdep_reset_lock after 'lockdep_reset_lock' (at offset 0x35)
    WARNING: kernel/built-in.o - Section mismatch: reference to .init.text:lockdep_init from .text.__lock_acquire after '__lock_acquire' (at offset 0xb2)

    The warnings are less obviously due to heavy inlining by gcc - this is not
    altered.

    Fix the section mismatch warnings by removing the __init marking, which
    seems obviously wrong.

    Signed-off-by: Sam Ravnborg
    Acked-by: Ingo Molnar
    Cc:
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Sam Ravnborg
     

21 Feb, 2007

1 commit


12 Feb, 2007

2 commits

  • Generate locking graph information into /proc/lockdep, for lock hierarchy
    documentation and visualization purposes.

    sample output:

    c089fd5c OPS: 138 FD: 14 BD: 1 --..: &tty->termios_mutex
    -> [c07a3430] tty_ldisc_lock
    -> [c07a37f0] &port_lock_key
    -> [c07afdc0] &rq->rq_lock_key#2

    The lock classes listed are all the first-hop lock dependencies that
    lockdep has seen so far.

    Signed-off-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Jason Baron
     
  • - returns after DEBUG_LOCKS_WARN_ON added in 3 places

    - debug_locks checking after lookup_chain_cache() added in
    __lock_acquire()

    - locking for testing and changing global variable max_lockdep_depth
    added in __lock_acquire()

    From: Ingo Molnar

    My __acquire_lock() cleanup introduced a locking bug: on SMP systems we'd
    release a non-owned graph lock. Fix this by moving the graph unlock back,
    and by leaving the max_lockdep_depth variable update possibly racy. (we
    dont care, it's just statistics)

    Also add some minimal debugging code to graph_unlock()/graph_lock(),
    which caught this locking bug.

    Signed-off-by: Jarek Poplawski
    Signed-off-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Jarek Poplawski
     

31 Dec, 2006

1 commit

  • kernel/lockdep.c: In function `lookup_chain_cache':
    kernel/lockdep.c:1339: warning: long long unsigned int format, u64 arg (arg 2)
    kernel/lockdep.c:1344: warning: long long unsigned int format, u64 arg (arg 2)

    Cc: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andrew Morton
     

14 Dec, 2006

7 commits

  • Jarek Poplawski noticed that lockdep global state could be accessed in a
    racy way if one CPU did a lockdep assert (shutting lockdep down), while the
    other CPU would try to do something that changes its global state.

    This patch fixes those races and cleans up lockdep's internal locking by
    adding a graph_lock()/graph_unlock()/debug_locks_off_graph_unlock helpers.

    (Also note that as we all know the Linux kernel is, by definition, bug-free
    and perfect, so this code never triggers, so these fixes are highly
    theoretical. I wrote this patch for aesthetic reasons alone.)

    [akpm@osdl.org: build fix]
    [jarkao2@o2.pl: build fix's refix]
    Signed-off-by: Ingo Molnar
    Signed-off-by: Jarek Poplawski
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     
  • When we print an assert due to scheduling-in-atomic bugs, and if lockdep
    is enabled, then the IRQ tracing information of lockdep can be printed
    to pinpoint the code location that disabled interrupts. This saved me
    quite a bit of debugging time in cases where the backtrace did not
    identify the irq-disabling site well enough.

    Signed-off-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     
  • CONFIG_DEBUG_LOCKDEP is unacceptably slow because it does not utilize
    the chain-hash. Turn the chain-hash back on in this case too.

    Signed-off-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     
  • Cleanup: the VERY_VERBOSE define was unnecessarily dependent on #ifdef VERBOSE
    - while the VERBOSE switch is 0 or 1 (always defined).

    Signed-off-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     
  • Clear all the chains during lockdep_reset(). This fixes some locking-selftest
    false positives i saw on -rt. (never saw those on mainline though, but it
    could happen.)

    Signed-off-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     
  • Make verbose lockdep messages (off by default) more informative by printing
    out the hash chain key. (this patch was what helped me catch the earlier
    lockdep hash-collision bug)

    Signed-off-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     
  • Fix typo in the class_filter() function. (filtering is not used by default so
    this only affects lockdep-internal debugging cases)

    Signed-off-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     

08 Dec, 2006

6 commits

  • * 'for-linus' of git://one.firstfloor.org/home/andi/git/linux-2.6: (156 commits)
    [PATCH] x86-64: Export smp_call_function_single
    [PATCH] i386: Clean up smp_tune_scheduling()
    [PATCH] unwinder: move .eh_frame to RODATA
    [PATCH] unwinder: fully support linker generated .eh_frame_hdr section
    [PATCH] x86-64: don't use set_irq_regs()
    [PATCH] x86-64: check vector in setup_ioapic_dest to verify if need setup_IO_APIC_irq
    [PATCH] x86-64: Make ix86 default to HIGHMEM4G instead of NOHIGHMEM
    [PATCH] i386: replace kmalloc+memset with kzalloc
    [PATCH] x86-64: remove remaining pc98 code
    [PATCH] x86-64: remove unused variable
    [PATCH] x86-64: Fix constraints in atomic_add_return()
    [PATCH] x86-64: fix asm constraints in i386 atomic_add_return
    [PATCH] x86-64: Correct documentation for bzImage protocol v2.05
    [PATCH] x86-64: replace kmalloc+memset with kzalloc in MTRR code
    [PATCH] x86-64: Fix numaq build error
    [PATCH] x86-64: include/asm-x86_64/cpufeature.h isn't a userspace header
    [PATCH] unwinder: Add debugging output to the Dwarf2 unwinder
    [PATCH] x86-64: Clarify error message in GART code
    [PATCH] x86-64: Fix interrupt race in idle callback (3rd try)
    [PATCH] x86-64: Remove unwind stack pointer alignment forcing again
    ...

    Fixed conflict in include/linux/uaccess.h manually

    Signed-off-by: Linus Torvalds

    Linus Torvalds
     
  • The hash_lock must only ever be taken with irqs disabled. This happens in
    all the important places, except one codepath: register_lock_class(). The
    race should trigger rarely because register_lock_class() is quite rare and
    single-threaded (happens during init most of the time).

    The fix is to disable irqs.

    ( bug found live in -rt: there preemption is alot more agressive and
    preempting with the hash-lock held caused a lockup.)

    Signed-off-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     
  • Remove the no longer used lockdep_internal().

    Signed-off-by: Adrian Bunk
    Acked-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Adrian Bunk
     
  • - numeric string size replaced with constant in print_lock_name and
    print_lockdep_cache,

    - return on null pointer in print_lock_dependencies,

    - one more lockdep return with 0 with unlocking fix in mark_lock.

    Signed-off-by: Jarek Poplawski
    Acked-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Jarek Poplawski
     
  • Here are mainly some lockdep returns with 0 with unlocking fixes.

    Signed-off-by: Jarek Poplawski
    Signed-off-by: Andrew Morton
    Signed-off-by: Ingo Molnar
    Signed-off-by: Linus Torvalds

    Jarek Poplawski
     
  • Stick NFS sockets in their own class to avoid some lockdep warnings. NFS
    sockets are never exposed to user-space, and will hence not trigger certain
    code paths that would otherwise pose deadlock scenarios.

    [akpm@osdl.org: cleanups]
    Signed-off-by: Peter Zijlstra
    Signed-off-by: Steven Dickson
    Acked-by: Ingo Molnar
    Cc: Trond Myklebust
    Acked-by: Neil Brown
    Cc: "David S. Miller"
    Signed-off-by: Andrew Morton
    [ Fixed patch corruption by quilt, pointed out by Peter Zijlstra ]
    Signed-off-by: Linus Torvalds

    Peter Zijlstra
     

07 Dec, 2006

1 commit


18 Nov, 2006

1 commit

  • lockdep got confused by certain locks in modules:

    INFO: trying to register non-static key.
    the code is fine but needs lockdep annotation.
    turning off the locking correctness validator.

    Call Trace:
    [] dump_trace+0xaa/0x3f2
    [] show_trace+0x3a/0x60
    [] dump_stack+0x15/0x17
    [] __lock_acquire+0x724/0x9bb
    [] lock_acquire+0x4d/0x67
    [] rt_spin_lock+0x3d/0x41
    [] :ip_conntrack:__ip_ct_refresh_acct+0x131/0x174
    [] :ip_conntrack:udp_packet+0xbf/0xcf
    [] :ip_conntrack:ip_conntrack_in+0x394/0x4a7
    [] nf_iterate+0x41/0x7f
    [] nf_hook_slow+0x64/0xd5
    [] ip_rcv+0x24e/0x506
    [...]

    Steven Rostedt found the bug: static_obj() check did not take
    PERCPU_ENOUGH_ROOM into account, so in-module DEFINE_PER_CPU-area locks
    were triggering this message.

    Signed-off-by: Ingo Molnar
    Signed-off-by: Steven Rostedt
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     

17 Oct, 2006

2 commits

  • * git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input:
    Input: fm801-gp - handle errors from pci_enable_device()
    Input: gameport core - handle errors returned by device_bind_driver()
    Input: serio core - handle errors returned by device_bind_driver()
    Lockdep: fix compile error in drivers/input/serio/serio.c
    Input: serio - add lockdep annotations
    Lockdep: add lockdep_set_class_and_subclass() and lockdep_set_subclass()
    Input: atkbd - supress "too many keys" error message
    Input: i8042 - supress ACK/NAKs when blinking during panic
    Input: add missing exports to fix modular build

    Linus Torvalds
     
  • In general, lockdep warnings are intended to be non-fatal, so I have put in
    various practical limits on internal data structure failure modes. We haven't
    had a /single/ lockdep-internal crash ever since lockdep went upstream [the
    unwinder crashes are outside of lockdep], and that's largely due to the good
    internal checks it does.

    Recursion within the dependency graph is currently limited to 20, that's
    probably not enough on some many-CPU boxes - this patch doubles it to 40. I
    have written the lockdep functions to have as small stackframes as possible,
    so 40 should be OK too. (The practical recursion limit should be somewhere
    between 100 and 200 entries. If we hit that then I'll change the algorithm to
    be iteration-based. Graph walking logic is so easy to program via recursion,
    so i'd like to keep recursion as long as possible.)

    Signed-off-by: Ingo Molnar
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Ingo Molnar
     

12 Oct, 2006

1 commit


11 Oct, 2006

1 commit


02 Oct, 2006

1 commit

  • In some places, particularly drivers and __init code, the init utsns is the
    appropriate one to use. This patch replaces those with a the init_utsname
    helper.

    Changes: Removed several uses of init_utsname(). Hope I picked all the
    right ones in net/ipv4/ipconfig.c. These are now changed to
    utsname() (the per-process namespace utsname) in the previous
    patch (2/7)

    [akpm@osdl.org: CIFS fix]
    Signed-off-by: Serge E. Hallyn
    Cc: Kirill Korotaev
    Cc: "Eric W. Biederman"
    Cc: Herbert Poetzl
    Cc: Andrey Savochkin
    Cc: Serge Hallyn
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Serge E. Hallyn