lkml.org 
[lkml]   [2020]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[RFC v7 01/19] locking: More accurate annotations for read_lock()
    Date
    On the archs using QUEUED_RWLOCKS, read_lock() is not always a recursive
    read lock, actually it's only recursive if in_interrupt() is true. So
    change the annotation accordingly to catch more deadlocks.

    Note we used to treat read_lock() as pure recursive read locks in
    lib/locking-seftest.c, and this is useful, especially for the lockdep
    development selftest, so we keep this via a variable to force switching
    lock annotation for read_lock().

    Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
    ---
    include/linux/lockdep.h | 23 ++++++++++++++++++++++-
    kernel/locking/lockdep.c | 14 ++++++++++++++
    lib/locking-selftest.c | 11 +++++++++++
    3 files changed, 47 insertions(+), 1 deletion(-)

    diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
    index 8fce5c98a4b0..6b7cb390f19f 100644
    --- a/include/linux/lockdep.h
    +++ b/include/linux/lockdep.h
    @@ -640,6 +640,20 @@ static inline void print_irqtrace_events(struct task_struct *curr)
    }
    #endif

    +/* Variable used to make lockdep treat read_lock() as recursive in selftests */
    +#ifdef CONFIG_DEBUG_LOCKING_API_SELFTESTS
    +extern unsigned int force_read_lock_recursive;
    +#else /* CONFIG_DEBUG_LOCKING_API_SELFTESTS */
    +#define force_read_lock_recursive 0
    +#endif /* CONFIG_DEBUG_LOCKING_API_SELFTESTS */
    +
    +#ifdef CONFIG_LOCKDEP
    +extern bool read_lock_is_recursive(void);
    +#else /* CONFIG_LOCKDEP */
    +/* If !LOCKDEP, the value is meaningless */
    +#define read_lock_is_recursive() 0
    +#endif
    +
    /*
    * For trivial one-depth nesting of a lock-class, the following
    * global define can be used. (Subsystems with multiple levels
    @@ -661,7 +675,14 @@ static inline void print_irqtrace_events(struct task_struct *curr)
    #define spin_release(l, i) lock_release(l, i)

    #define rwlock_acquire(l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)
    -#define rwlock_acquire_read(l, s, t, i) lock_acquire_shared_recursive(l, s, t, NULL, i)
    +#define rwlock_acquire_read(l, s, t, i) \
    +do { \
    + if (read_lock_is_recursive()) \
    + lock_acquire_shared_recursive(l, s, t, NULL, i); \
    + else \
    + lock_acquire_shared(l, s, t, NULL, i); \
    +} while (0)
    +
    #define rwlock_release(l, i) lock_release(l, i)

    #define seqcount_acquire(l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)
    diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
    index 29a8de4c50b9..fbcbb6350ce7 100644
    --- a/kernel/locking/lockdep.c
    +++ b/kernel/locking/lockdep.c
    @@ -4921,6 +4921,20 @@ static bool lockdep_nmi(void)
    return true;
    }

    +/*
    + * read_lock() is recursive if:
    + * 1. We force lockdep think this way in selftests or
    + * 2. The implementation is not queued read/write lock or
    + * 3. The locker is at an in_interrupt() context.
    + */
    +bool read_lock_is_recursive(void)
    +{
    + return force_read_lock_recursive ||
    + !IS_ENABLED(CONFIG_QUEUED_RWLOCKS) ||
    + in_interrupt();
    +}
    +EXPORT_SYMBOL_GPL(read_lock_is_recursive);
    +
    /*
    * We are not always called with irqs disabled - do that here,
    * and also avoid lockdep recursion:
    diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c
    index 14f44f59e733..caadc4dd3368 100644
    --- a/lib/locking-selftest.c
    +++ b/lib/locking-selftest.c
    @@ -28,6 +28,7 @@
    * Change this to 1 if you want to see the failure printouts:
    */
    static unsigned int debug_locks_verbose;
    +unsigned int force_read_lock_recursive;

    static DEFINE_WD_CLASS(ww_lockdep);

    @@ -1978,6 +1979,11 @@ void locking_selftest(void)
    return;
    }

    + /*
    + * treats read_lock() as recursive read locks for testing purpose
    + */
    + force_read_lock_recursive = 1;
    +
    /*
    * Run the testsuite:
    */
    @@ -2073,6 +2079,11 @@ void locking_selftest(void)

    ww_tests();

    + force_read_lock_recursive = 0;
    + /*
    + * queued_read_lock() specific test cases can be put here
    + */
    +
    if (unexpected_testcase_failures) {
    printk("-----------------------------------------------------------------\n");
    debug_locks = 0;
    --
    2.28.0
    \
     
     \ /
      Last update: 2020-08-07 09:43    [W:4.071 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site