Re: [PATCH 1/2] hung_task: show the blocker task if the task is hung on rtmutex
Ruipeng Qi <[email protected]> Mon, 3 Aug 2026 22:42:48 +0800
| Newsgroups | dev.linux.lists.linux-rt-devel |
|---|---|
| Message-ID | <[email protected]> |
On 8/1/2026 12:07 AM, Petr Mladek wrote: > On Thu 2026-07-30 10:29:08, ruipengqi wrote: >> From: Ruipeng Qi <[email protected]> >> >> Currently, debug_show_blocker() only tracks mutex, semaphore and rwsem >> lock types. Extend it to also cover rtmutex or rt_mutex-based >> implementations locks on PREEMPT_RT, so that when a task is hung >> on an rtmutex, the hung task detector can identify and report which >> task holds the lock. >> >> On 64-bit systems, lock pointers are 8-byte aligned, so their three >> least significant bits are always zero. Use these bits to encode the >> blocker type. >> >> Unlike semaphores, rtmutex has built-in owner tracking via lock->owner, >> so no additional bookkeeping is needed. The owner can be read with >> rt_mutex_owner(). >> >> --- a/kernel/hung_task.c >> +++ b/kernel/hung_task.c >> @@ -27,6 +27,7 @@ >> #include <linux/sys_info.h> >> >> #include <trace/events/sched.h> >> +#include <linux/rtmutex.h> >> >> /* >> * The number of tasks checked: >> @@ -149,12 +150,15 @@ static void debug_show_blocker(struct task_struct *task, unsigned long timeout) >> blocker_type = hung_task_get_blocker_type(blocker); >> >> switch (blocker_type) { >> +#ifndef CONFIG_PREEMPT_RT >> case BLOCKER_TYPE_MUTEX: >> owner = mutex_get_owner(hung_task_blocker_to_lock(blocker)); >> break; >> +#endif >> case BLOCKER_TYPE_SEM: >> owner = sem_last_holder(hung_task_blocker_to_lock(blocker)); >> break; >> +#ifndef CONFIG_PREEMPT_RT >> case BLOCKER_TYPE_RWSEM_READER: >> case BLOCKER_TYPE_RWSEM_WRITER: >> owner = (unsigned long)rwsem_owner( >> @@ -165,6 +169,12 @@ static void debug_show_blocker(struct task_struct *task, unsigned long timeout) >> hung_task_blocker_to_lock(blocker)) ? >> "reader" : "writer"; >> break; >> +#endif >> +#if defined(CONFIG_64BIT) && defined(CONFIG_RT_MUTEXES) >> + case BLOCKER_TYPE_RTMUTEX: >> + owner = (unsigned long)rt_mutex_owner(hung_task_blocker_to_lock(blocker)); >> + break; >> +#endif >> default: >> WARN_ON_ONCE(1); >> return; > On one hand, it is "elegant" to handle rtmutex the same way as > the other locks. > > On the other hand, this adds a lot of if-deffery and > does not work on 32-bit systems. > > Alternative solution would use the existing tracking, > something like: > > if (rt_mutext_blocking_task(task)) > owner = (unsigned long)rt_mutex_owner(rt_mutex_blocking_task(task)); > > , where rt_mutex_blocking_task() would be a racy public variant > of the existing task_blocked_on_lock(). Something like: > > static inline struct rt_mutex_base *rt_mutex_blocking_task(struct task_struct *p) > { > struct rt_mutex_waiter *waiter; > > waiter = data_race(READ_ONCE(p->pi_blocked_on) > > if (waiter) > return data_race(READ_ONCE(waiter->lock); > > return NULL; > } Thanks to Petr, Lance, and Xusheng for the feedback. One concern with the alternative approach is that 'waiter' is typically allocated on the stack (for instance, in __rt_mutex_slowlock_locked()). If the blocked task gets woken up and resumes execution, its stack frame is destroyed and the 'waiter' structure becomes invalid. Since the hung task detector reads p->pi_blocked_on asynchronously without holding the lock, dereferencing waiter->lock could lead to a UAF race condition if the waiter is destroyed concurrently. Therefore, the first solution (encoding the blocker type in the pointer bits) seems safer, as it avoids dereferencing a potentially stale stack pointer. I am currently preparing the V2 patch to address the issues you mentioned. v2 changes: - Reduce #ifdef-deffery. - Remove the unused function debug_trace_blocker. - Clean up and update relevant comments. Best regards, Ruipeng > Plus, it would require using WRITE_ONCE() when task->pi_blocked_on > is updated to make sure that the value is always consistent > (valid pointer or NULL). > > It would avoid the duplicate tracking in the rt_mutex code > and might be more acceptable for the rt_mutex maintainers. > > Best Regards, > Petr