Skip to content

Commit

Permalink
list_bl: fixup bogus lockdep warning
Browse files Browse the repository at this point in the history
At first glance, the use of 'static inline' seems appropriate for
INIT_HLIST_BL_HEAD().

However, when a 'static inline' function invocation is inlined by gcc,
all callers share any static local data declared within that inline
function.

This presents a problem for how lockdep classes are setup.  raw_spinlocks, for
example, when CONFIG_DEBUG_SPINLOCK,

	# define raw_spin_lock_init(lock)				\
	do {								\
		static struct lock_class_key __key;			\
									\
		__raw_spin_lock_init((lock), #lock, &__key);		\
	} while (0)

When this macro is expanded into a 'static inline' caller, like
INIT_HLIST_BL_HEAD():

	static inline INIT_HLIST_BL_HEAD(struct hlist_bl_head *h)
	{
		h->first = NULL;
		raw_spin_lock_init(&h->lock);
	}

...the static local lock_class_key object is made a function static.

For compilation units which initialize invoke INIT_HLIST_BL_HEAD() more
than once, then, all of the invocations share this same static local
object.

This can lead to some very confusing lockdep splats (example below).
Solve this problem by forcing the INIT_HLIST_BL_HEAD() to be a macro,
which prevents the lockdep class object sharing.

 =============================================
 [ INFO: possible recursive locking detected ]
 4.4.4-rt11 raspberrypi#4 Not tainted
 ---------------------------------------------
 kswapd0/59 is trying to acquire lock:
  (&h->lock#2){+.+.-.}, at: mb_cache_shrink_scan

 but task is already holding lock:
  (&h->lock#2){+.+.-.}, at:  mb_cache_shrink_scan

 other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(&h->lock#2);
   lock(&h->lock#2);

  *** DEADLOCK ***

  May be due to missing lock nesting notation

 2 locks held by kswapd0/59:
  #0:  (shrinker_rwsem){+.+...}, at: rt_down_read_trylock
  #1:  (&h->lock#2){+.+.-.}, at: mb_cache_shrink_scan

Reported-by: Luis Claudio R. Goncalves <lclaudio@uudg.org>
Tested-by: Luis Claudio R. Goncalves <lclaudio@uudg.org>
Signed-off-by: Josh Cartwright <joshc@ni.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
  • Loading branch information
Josh Cartwright authored and Tiejun Chen committed Feb 24, 2018
1 parent 6e3870d commit 1ef66b1
Showing 1 changed file with 7 additions and 5 deletions.
12 changes: 7 additions & 5 deletions include/linux/list_bl.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,15 @@ struct hlist_bl_node {
struct hlist_bl_node *next, **pprev;
};

static inline void INIT_HLIST_BL_HEAD(struct hlist_bl_head *h)
{
h->first = NULL;
#ifdef CONFIG_PREEMPT_RT_BASE
raw_spin_lock_init(&h->lock);
#define INIT_HLIST_BL_HEAD(h) \
do { \
(h)->first = NULL; \
raw_spin_lock_init(&(h)->lock); \
} while (0)
#else
#define INIT_HLIST_BL_HEAD(h) (h)->first = NULL
#endif
}

static inline void INIT_HLIST_BL_NODE(struct hlist_bl_node *h)
{
Expand Down

0 comments on commit 1ef66b1

Please sign in to comment.