Commit e9181886 authored by Boqun Feng's avatar Boqun Feng Committed by Peter Zijlstra
Browse files

locking: More accurate annotations for read_lock() 



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: default avatarBoqun Feng <boqun.feng@gmail.com>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20200807074238.1632519-2-boqun.feng@gmail.com
parent 92b4e9f1

include/linux/lockdep.h

+22 −1
Original line number Diff line number Diff line
@@ -469,6 +469,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
@@ -490,7 +504,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)

kernel/locking/lockdep.c

+14 −0
Original line number Diff line number Diff line
@@ -4967,6 +4967,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:

lib/locking-selftest.c

+11 −0
Original line number Diff line number Diff line
@@ -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;

CRA Git | Maintained and supported by SUSTech CRA and CCSE