Skip to content

Commit 0cce06b

Browse files
author
Peter Zijlstra
committed
debugobjects,locking: Annotate debug_object_fill_pool() wait type violation
There is an explicit wait-type violation in debug_object_fill_pool() for PREEMPT_RT=n kernels which allows them to more easily fill the object pool and reduce the chance of allocation failures. Lockdep's wait-type checks are designed to check the PREEMPT_RT locking rules even for PREEMPT_RT=n kernels and object to this, so create a lockdep annotation to allow this to stand. Specifically, create a 'lock' type that overrides the inner wait-type while it is held -- allowing one to temporarily raise it, such that the violation is hidden. Reported-by: Vlastimil Babka <[email protected]> Reported-by: Qi Zheng <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Tested-by: Qi Zheng <[email protected]> Link: https://lkml.kernel.org/r/[email protected]
1 parent 0af462f commit 0cce06b

File tree

4 files changed

+49
-9
lines changed

4 files changed

+49
-9
lines changed

include/linux/lockdep.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,16 @@ extern void lock_unpin_lock(struct lockdep_map *lock, struct pin_cookie);
339339
#define lockdep_repin_lock(l,c) lock_repin_lock(&(l)->dep_map, (c))
340340
#define lockdep_unpin_lock(l,c) lock_unpin_lock(&(l)->dep_map, (c))
341341

342+
/*
343+
* Must use lock_map_aquire_try() with override maps to avoid
344+
* lockdep thinking they participate in the block chain.
345+
*/
346+
#define DEFINE_WAIT_OVERRIDE_MAP(_name, _wait_type) \
347+
struct lockdep_map _name = { \
348+
.name = #_name "-wait-type-override", \
349+
.wait_type_inner = _wait_type, \
350+
.lock_type = LD_LOCK_WAIT_OVERRIDE, }
351+
342352
#else /* !CONFIG_LOCKDEP */
343353

344354
static inline void lockdep_init_task(struct task_struct *task)
@@ -427,6 +437,9 @@ extern int lockdep_is_held(const void *);
427437
#define lockdep_repin_lock(l, c) do { (void)(l); (void)(c); } while (0)
428438
#define lockdep_unpin_lock(l, c) do { (void)(l); (void)(c); } while (0)
429439

440+
#define DEFINE_WAIT_OVERRIDE_MAP(_name, _wait_type) \
441+
struct lockdep_map __maybe_unused _name = {}
442+
430443
#endif /* !LOCKDEP */
431444

432445
enum xhlock_context_t {
@@ -551,6 +564,7 @@ do { \
551564
#define rwsem_release(l, i) lock_release(l, i)
552565

553566
#define lock_map_acquire(l) lock_acquire_exclusive(l, 0, 0, NULL, _THIS_IP_)
567+
#define lock_map_acquire_try(l) lock_acquire_exclusive(l, 0, 1, NULL, _THIS_IP_)
554568
#define lock_map_acquire_read(l) lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_)
555569
#define lock_map_acquire_tryread(l) lock_acquire_shared_recursive(l, 0, 1, NULL, _THIS_IP_)
556570
#define lock_map_release(l) lock_release(l, _THIS_IP_)

include/linux/lockdep_types.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ enum lockdep_wait_type {
3333
enum lockdep_lock_type {
3434
LD_LOCK_NORMAL = 0, /* normal, catch all */
3535
LD_LOCK_PERCPU, /* percpu */
36+
LD_LOCK_WAIT_OVERRIDE, /* annotation */
3637
LD_LOCK_MAX,
3738
};
3839

kernel/locking/lockdep.c

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2253,6 +2253,9 @@ static inline bool usage_match(struct lock_list *entry, void *mask)
22532253

22542254
static inline bool usage_skip(struct lock_list *entry, void *mask)
22552255
{
2256+
if (entry->class->lock_type == LD_LOCK_NORMAL)
2257+
return false;
2258+
22562259
/*
22572260
* Skip local_lock() for irq inversion detection.
22582261
*
@@ -2279,14 +2282,16 @@ static inline bool usage_skip(struct lock_list *entry, void *mask)
22792282
* As a result, we will skip local_lock(), when we search for irq
22802283
* inversion bugs.
22812284
*/
2282-
if (entry->class->lock_type == LD_LOCK_PERCPU) {
2283-
if (DEBUG_LOCKS_WARN_ON(entry->class->wait_type_inner < LD_WAIT_CONFIG))
2284-
return false;
2285+
if (entry->class->lock_type == LD_LOCK_PERCPU &&
2286+
DEBUG_LOCKS_WARN_ON(entry->class->wait_type_inner < LD_WAIT_CONFIG))
2287+
return false;
22852288

2286-
return true;
2287-
}
2289+
/*
2290+
* Skip WAIT_OVERRIDE for irq inversion detection -- it's not actually
2291+
* a lock and only used to override the wait_type.
2292+
*/
22882293

2289-
return false;
2294+
return true;
22902295
}
22912296

22922297
/*
@@ -4752,7 +4757,8 @@ static int check_wait_context(struct task_struct *curr, struct held_lock *next)
47524757

47534758
for (; depth < curr->lockdep_depth; depth++) {
47544759
struct held_lock *prev = curr->held_locks + depth;
4755-
u8 prev_inner = hlock_class(prev)->wait_type_inner;
4760+
struct lock_class *class = hlock_class(prev);
4761+
u8 prev_inner = class->wait_type_inner;
47564762

47574763
if (prev_inner) {
47584764
/*
@@ -4762,6 +4768,14 @@ static int check_wait_context(struct task_struct *curr, struct held_lock *next)
47624768
* Also due to trylocks.
47634769
*/
47644770
curr_inner = min(curr_inner, prev_inner);
4771+
4772+
/*
4773+
* Allow override for annotations -- this is typically
4774+
* only valid/needed for code that only exists when
4775+
* CONFIG_PREEMPT_RT=n.
4776+
*/
4777+
if (unlikely(class->lock_type == LD_LOCK_WAIT_OVERRIDE))
4778+
curr_inner = prev_inner;
47654779
}
47664780
}
47674781

lib/debugobjects.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -591,10 +591,21 @@ static void debug_objects_fill_pool(void)
591591
{
592592
/*
593593
* On RT enabled kernels the pool refill must happen in preemptible
594-
* context:
594+
* context -- for !RT kernels we rely on the fact that spinlock_t and
595+
* raw_spinlock_t are basically the same type and this lock-type
596+
* inversion works just fine.
595597
*/
596-
if (!IS_ENABLED(CONFIG_PREEMPT_RT) || preemptible())
598+
if (!IS_ENABLED(CONFIG_PREEMPT_RT) || preemptible()) {
599+
/*
600+
* Annotate away the spinlock_t inside raw_spinlock_t warning
601+
* by temporarily raising the wait-type to WAIT_SLEEP, matching
602+
* the preemptible() condition above.
603+
*/
604+
static DEFINE_WAIT_OVERRIDE_MAP(fill_pool_map, LD_WAIT_SLEEP);
605+
lock_map_acquire_try(&fill_pool_map);
597606
fill_pool();
607+
lock_map_release(&fill_pool_map);
608+
}
598609
}
599610

600611
static void

0 commit comments

Comments
 (0)