Re: [PATCH 1/2] hung_task: show the blocker task if the task is hung on rtmutex

Re: [PATCH 1/2] hung_task: show the blocker task if the task is hung on rtmutex

[Zhan Xusheng]

From: Zhan Xusheng <zhanxusheng1024@gmail.com>

Hi Ruipeng,

Nice extension.  I traced the set/clear pairing and it looks sound: the
blocker is set/cleared alongside every task->pi_blocked_on assignment (all
three pi_blocked_on = NULL sites clear it), so there is no stale blocker
left behind.  Gating the mutex/rwsem cases in debug_show_blocker() under
!CONFIG_PREEMPT_RT also matches that their setters (and mutex_get_owner()/
rwsem_owner()) only exist on !RT, while BLOCKER_TYPE_SEM is correctly kept
since semaphores stay counting on RT.  A few comments:

> +extern void debug_trace_blocker(struct task_struct *task,
> +	       unsigned long timeout, bool iter);
[...]
> +static inline void debug_trace_blocker(struct task_struct *task,
> +	      unsigned long timeout, bool iter)
> +{
> +}

debug_trace_blocker() seems to be only declared (and stubbed) here - I
can't find any definition or caller in the series or in-tree.  Is it a
leftover from a larger WIP that should be dropped?

> +#ifdef CONFIG_64BIT
> +#define BLOCKER_TYPE_RTMUTEX		0x04UL
> +#define BLOCKER_TYPE_MASK		0x07UL
> +#else
> #define BLOCKER_TYPE_MASK		0x03UL
> +#endif

Since the 3rd bit is only available on 64-bit, rtmutex blocker tracking is
effectively 64-bit only (on 32-bit the two low bits are already taken by
mutex/sem/rwsem-r/w).  Could the changelog state that explicitly?  Also,
on PREEMPT_RT spinlock_t/rwlock_t/mutex/rwsem are all rtmutex-based, so
this effectively reports every rtmutex-based sleeping-lock wait, not only
rt_mutex_lock() - a word about that would keep the "blocked on a rtmutex"
line from being surprising for what was a mutex/spinlock at the source
level.

> +#if defined(CONFIG_64BIT)
> +	hung_task_set_blocker(task, lock, BLOCKER_TYPE_RTMUTEX);
> +#endif

(and the three matching clear sites)

Would it be cleaner to hide the CONFIG_64BIT-ness inside the hung_task
helpers - e.g. a no-op rtmutex set/clear on 32-bit - so rtmutex.c doesn't
need the scattered #if at each call site?

Thanks,
Zhan Xusheng