debugobjects: Various backports
Subsystems (4)
Thomas Gleixner
maintainercover debugobjects: Various backports
This series contains the necessary backports to apply the recent important debugobject fixes to the linux-6.1.y series.
I leave 5.15/5.10 for people who are into kernel necrophilia as they need much more care and effort.
Thanks,
tglx
Thomas Gleixner
maintainerPATCH 1/5 debugobjects,locking: Annotate debug_object_fill_pool() wait type violation
From: Peter Zijlstra <[email protected]>
commit 0cce06ba859a515bd06224085d3addb870608b6d upstream.
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.
--- include/linux/lockdep.h | 14 ++++++++++++++ include/linux/lockdep_types.h | 1 + kernel/locking/lockdep.c | 28 +++++++++++++++++++++------- lib/debugobjects.c | 15 +++++++++++++-- 4 files changed, 49 insertions(+), 9 deletions(-)
include/linux/lockdep.h +14 -0
@@ -339,6 +339,16 @@ extern void lock_unpin_lock(struct lockd#define lockdep_repin_lock (l,c) lock_repin_lock(&(l)->dep_map, (c))#define lockdep_unpin_lock (l,c) lock_unpin_lock(&(l)->dep_map, (c)) +/* +* Must use lock_map_aquire_try() with override maps to avoid +* lockdep thinking they participate in the block chain. +*/ +#define DEFINE_WAIT_OVERRIDE_MAP (_name, _wait_type) \ + struct lockdep_map _name = { \ + .name = #_name "-wait-type-override", \ + .wait_type_inner = _wait_type, \ + .lock_type = LD_LOCK_WAIT_OVERRIDE, } +#else /* !CONFIG_LOCKDEP */ static inline void lockdep_init_task (struct task_struct *task)
@@ -427,6 +437,9 @@ extern int lockdep_is_held(const void *)#define lockdep_repin_lock (l, c) do { (void)(l); (void)(c); } while (0)#define lockdep_unpin_lock (l, c) do { (void)(l); (void)(c); } while (0) +#define DEFINE_WAIT_OVERRIDE_MAP (_name, _wait_type) \ + struct lockdep_map __maybe_unused _name = {} +#endif /* !LOCKDEP */ enum xhlock_context_t {
@@ -552,6 +565,7 @@ do { \#define rwsem_release (l, i) lock_release(l, i)#define lock_map_acquire (l) lock_acquire_exclusive(l, 0, 0, NULL, _THIS_IP_) +#define lock_map_acquire_try (l) lock_acquire_exclusive(l, 0, 1, NULL, _THIS_IP_)#define lock_map_acquire_read (l) lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_)#define lock_map_acquire_tryread (l) lock_acquire_shared_recursive(l, 0, 1, NULL, _THIS_IP_)#define lock_map_release (l) lock_release(l, _THIS_IP_)
include/linux/lockdep_types.h +1 -0
@@ -33,6 +33,7 @@ enum lockdep_wait_type {enum lockdep_lock_type {LD_LOCK_NORMAL =0 ,/* normal, catch all */ LD_LOCK_PERCPU ,/* percpu */ +LD_LOCK_WAIT_OVERRIDE ,/* annotation */ LD_LOCK_MAX , };
kernel/locking/lockdep.c +21 -7
@@ -2245,6 +2245,9 @@ static inline bool usage_match(struct lostatic inline bool usage_skip (struct lock_list *entry,void *mask) { +if (entry->class->lock_type ==LD_LOCK_NORMAL ) +return false; +/* * Skip local_lock() for irq inversion detection. *
@@ -2271,14 +2274,16 @@ static inline bool usage_skip(struct loc* As a result, we will skip local_lock(), when we search for irq * inversion bugs. */ -if (entry->class->lock_type ==LD_LOCK_PERCPU ) { -if (DEBUG_LOCKS_WARN_ON (entry->class->wait_type_inner <LD_WAIT_CONFIG )) -return false; +if (entry->class->lock_type ==LD_LOCK_PERCPU && +DEBUG_LOCKS_WARN_ON (entry->class->wait_type_inner <LD_WAIT_CONFIG )) +return false; -return true; - } +/* +* Skip WAIT_OVERRIDE for irq inversion detection -- it's not actually +* a lock and only used to override the wait_type. +*/ -return false; +return true; }/*
@@ -4745,7 +4750,8 @@ static int check_wait_context(struct tasfor (; depth < curr->lockdep_depth; depth++) {struct held_lock *prev = curr->held_locks + depth; -u8 prev_inner =hlock_class (prev)->wait_type_inner; +struct lock_class *class =hlock_class (prev); +u8 prev_inner = class->wait_type_inner;if (prev_inner) {/*
@@ -4755,6 +4761,14 @@ static int check_wait_context(struct tas* Also due to trylocks. */ curr_inner =min (curr_inner, prev_inner); + +/* +* Allow override for annotations -- this is typically +* only valid/needed for code that only exists when +* CONFIG_PREEMPT_RT=n. +*/ +if (unlikely (class->lock_type ==LD_LOCK_WAIT_OVERRIDE )) + curr_inner = prev_inner; } }
lib/debugobjects.c +13 -2
@@ -601,10 +601,21 @@ static void debug_objects_fill_pool(void {/* * On RT enabled kernels the pool refill must happen in preemptible -* context: +* context -- for !RT kernels we rely on the fact that spinlock_t and +* raw_spinlock_t are basically the same type and this lock-type +* inversion works just fine. */ -if (!IS_ENABLED (CONFIG_PREEMPT_RT ) ||preemptible ()) +if (!IS_ENABLED (CONFIG_PREEMPT_RT ) ||preemptible ()) { +/* +* Annotate away the spinlock_t inside raw_spinlock_t warning +* by temporarily raising the wait-type to WAIT_SLEEP, matching +* the preemptible() condition above. +*/ + static DEFINE_WAIT_OVERRIDE_MAP(fill_pool_map, LD_WAIT_SLEEP); +lock_map_acquire_try (&fill_pool_map);fill_pool (); +lock_map_release (&fill_pool_map); + } } static void
Thomas Gleixner
maintainerPATCH 2/5 debugobjects: Allow to refill the pool before SYSTEM_SCHEDULING
In reply to Thomas GleixnerFrom: Sebastian Andrzej Siewior <[email protected]>
commit 06e0ae988f6e3499785c407429953ade19c1096b upstream.
The pool of free objects is refilled on several occasions such as object initialisation. On PREEMPT_RT refilling is limited to preemptible sections due to sleeping locks used by the memory allocator. The system boots with disabled interrupts so the pool can not be refilled.
If too many objects are initialized and the pool gets empty then debugobjects disables itself.
Refiling can also happen early in the boot with disabled interrupts as long as the scheduler is not operational. If the scheduler can not preempt a task then a sleeping lock can not be contended.
Allow to additionally refill the pool if the scheduler is not operational.
--- lib/debugobjects.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
---
lib/debugobjects.c +1 -1
@@ -605,7 +605,7 @@ static void debug_objects_fill_pool(void* raw_spinlock_t are basically the same type and this lock-type * inversion works just fine. */ -if (!IS_ENABLED (CONFIG_PREEMPT_RT ) ||preemptible ()) { +if (!IS_ENABLED (CONFIG_PREEMPT_RT ) ||preemptible () || system_state <SYSTEM_SCHEDULING ) {/* * Annotate away the spinlock_t inside raw_spinlock_t warning * by temporarily raising the wait-type to WAIT_SLEEP, matching
Thomas Gleixner
maintainerPATCH 3/5 debugobjects: Use LD_WAIT_CONFIG instead of LD_WAIT_SLEEP
In reply to Thomas GleixnerFrom: Sebastian Andrzej Siewior <[email protected]>
commit 37de2dbc318ee10577c1c2704de5a803e75e55a2 upstream.
fill_pool_map is used to suppress nesting violations caused by acquiring a spinlock_t (from within the memory allocator) while holding a raw_spinlock_t. The used annotation is wrong.
LD_WAIT_SLEEP is for always sleeping lock types such as mutex_t. LD_WAIT_CONFIG is for lock type which are sleeping while spinning on PREEMPT_RT such as spinlock_t.
Use LD_WAIT_CONFIG as override.
--- lib/debugobjects.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
---
lib/debugobjects.c +2 -2
@@ -608,10 +608,10 @@ static void debug_objects_fill_pool(voidif (!IS_ENABLED (CONFIG_PREEMPT_RT ) ||preemptible () || system_state <SYSTEM_SCHEDULING ) {/* * Annotate away the spinlock_t inside raw_spinlock_t warning -* by temporarily raising the wait-type to WAIT_SLEEP, matching +* by temporarily raising the wait-type to LD_WAIT_CONFIG, matching * the preemptible() condition above. */ - static DEFINE_WAIT_OVERRIDE_MAP(fill_pool_map, LD_WAIT_SLEEP); + static DEFINE_WAIT_OVERRIDE_MAP(fill_pool_map, LD_WAIT_CONFIG);lock_map_acquire_try (&fill_pool_map);fill_pool ();lock_map_release (&fill_pool_map);
Thomas Gleixner
maintainerPATCH 4/5 debugobjects: Do not fill_pool() if pi_blocked_on
In reply to Thomas GleixnerFrom: Helen Koike <[email protected]>
commit 5f41161059fd0f1bbf18c90f3180e38cc45a14eb upstream.
On RT enabled kernels, fill_pool() ends up calling rtlock_lock(), which asserts if current::pi_blocked_on is set, because a task can obviously only block on one lock as otherwise the priority inheritenace chain gets corrupted.
Prevent this by expanding the conditional to take current::pi_blocked_on into account.
--- lib/debugobjects.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
---
lib/debugobjects.c +14 -4
@@ -596,15 +596,25 @@ static struct debug_obj *lookup_object_oreturn NULL ; } +static inline bool debug_objects_is_pi_blocked_on (void ) +{ +#ifdef CONFIG_RT_MUTEXES +return current->pi_blocked_on !=NULL ; +#else +return false; +#endif +} +static void debug_objects_fill_pool (void ) {/* * On RT enabled kernels the pool refill must happen in preemptible -* context -- for !RT kernels we rely on the fact that spinlock_t and -* raw_spinlock_t are basically the same type and this lock-type -* inversion works just fine. +* context and not enqueued on an rt_mutex -- for !RT kernels we rely +* on the fact that spinlock_t and raw_spinlock_t are basically the +* same type and this lock-type inversion works just fine. */ -if (!IS_ENABLED (CONFIG_PREEMPT_RT ) ||preemptible () || system_state <SYSTEM_SCHEDULING ) { +if (!IS_ENABLED (CONFIG_PREEMPT_RT ) || system_state <SYSTEM_SCHEDULING || + (preemptible () && !debug_objects_is_pi_blocked_on ())) {/* * Annotate away the spinlock_t inside raw_spinlock_t warning * by temporarily raising the wait-type to LD_WAIT_CONFIG, matching
Thomas Gleixner
maintainerPATCH 5/5 debugobjects: Dont call fill_pool() in early boot hardirq context
In reply to Thomas GleixnerFrom: Waiman Long <[email protected]>
commit 0d046ae106255cba5eb83b23f78ee93f3620247d upstream.
When booting a debug PREEMPT_RT kernel on an ARM64 system, a "inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage" lockdep warning message was reported to the console.
During early boot, interrupts are enabled before the scheduler is enabled. In this window (before SYSTEM_SCHEDULING is set) interrupts can fire and in the hard interrupt context handler attempt to fill the pool
This can lead to a deadlock when the interrupt occurred when the interrupt hits a region which holds a lock that is required to be taken in the allocation path.
Add a new can_fill_pool() helper and reorder the exception rule and forbid this scenario by excluding allocations from hard interrupt context.
--- lib/debugobjects.c | 44 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-)
---
lib/debugobjects.c +36 -8
@@ -606,20 +606,48 @@ static inline bool debug_objects_is_pi_b#endif } -static void debug_objects_fill_pool (void ) +static inline bool can_fill_pool (void ) {/* -* On RT enabled kernels the pool refill must happen in preemptible -* context and not enqueued on an rt_mutex -- for !RT kernels we rely -* on the fact that spinlock_t and raw_spinlock_t are basically the -* same type and this lock-type inversion works just fine. +* On !RT enabled kernels there are no restrictions and spinlock_t and +* raw_spinlock_t are the same types. +*/ +if (!IS_ENABLED (CONFIG_PREEMPT_RT )) +return true; + +/* +* On RT enabled kernels, the task must not be blocked on a lock as +* that could corrupt the PI state when blocking on a lock in the +* allocation path. +*/ +if (debug_objects_is_pi_blocked_on ()) +return false; + +/* +* On RT enabled kernels the pool refill should happen in preemptible +* context. */ -if (!IS_ENABLED (CONFIG_PREEMPT_RT ) || system_state <SYSTEM_SCHEDULING || - (preemptible () && !debug_objects_is_pi_blocked_on ())) { +if (preemptible ()) +return true; + +/* +* Though during system boot before scheduling is set up, preemption is +* disabled and the pool can get exhausted. Before scheduling is active +* a task cannot be blocked on a sleeping lock, but it might hold a lock +* and if interrupted then hard interrupt context might run into a lock +* inversion. So exclude hard interrupt context from allocations before +* scheduling is active. +*/ +return system_state <SYSTEM_SCHEDULING && !in_hardirq (); +} + +static void debug_objects_fill_pool (void ) +{ +if (can_fill_pool ()) {/* * Annotate away the spinlock_t inside raw_spinlock_t warning * by temporarily raising the wait-type to LD_WAIT_CONFIG, matching -* the preemptible() condition above. +* the preemptible() condition in can_fill_pool(). */ static DEFINE_WAIT_OVERRIDE_MAP(fill_pool_map, LD_WAIT_CONFIG);lock_map_acquire_try (&fill_pool_map);
Sasha Levin
maintainerQuoted 1 line · Thomas Gleixner
[patch v6.1.y 0/5] debugobjects: Various backports
All five queued for 6.1, thanks!
Signature · 2 lines
Thanks, Sasha