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

From: Zhan Xusheng <[email protected]>=0D

Hi Ruipeng,=0D
=0D
Nice extension.  I traced the set/clear pairing and it looks sound: the=0D
blocker is set/cleared alongside every task->pi_blocked_on assignment (all=
=0D
three pi_blocked_on =3D NULL sites clear it), so there is no stale blocker=
=0D
left behind.  Gating the mutex/rwsem cases in debug_show_blocker() under=0D
!CONFIG_PREEMPT_RT also matches that their setters (and mutex_get_owner()/=
=0D
rwsem_owner()) only exist on !RT, while BLOCKER_TYPE_SEM is correctly kept=
=0D
since semaphores stay counting on RT.  A few comments:=0D
=0D
> +extern void debug_trace_blocker(struct task_struct *task,=0D
> +	       unsigned long timeout, bool iter);=0D
[...]=0D
> +static inline void debug_trace_blocker(struct task_struct *task,=0D
> +	      unsigned long timeout, bool iter)=0D
> +{=0D
> +}=0D
=0D
debug_trace_blocker() seems to be only declared (and stubbed) here - I=0D
can't find any definition or caller in the series or in-tree.  Is it a=0D
leftover from a larger WIP that should be dropped?=0D
=0D
> +#ifdef CONFIG_64BIT=0D
> +#define BLOCKER_TYPE_RTMUTEX		0x04UL=0D
> +#define BLOCKER_TYPE_MASK		0x07UL=0D
> +#else=0D
> #define BLOCKER_TYPE_MASK		0x03UL=0D
> +#endif=0D
=0D
Since the 3rd bit is only available on 64-bit, rtmutex blocker tracking is=
=0D
effectively 64-bit only (on 32-bit the two low bits are already taken by=0D
mutex/sem/rwsem-r/w).  Could the changelog state that explicitly?  Also,=0D
on PREEMPT_RT spinlock_t/rwlock_t/mutex/rwsem are all rtmutex-based, so=0D
this effectively reports every rtmutex-based sleeping-lock wait, not only=0D
rt_mutex_lock() - a word about that would keep the "blocked on a rtmutex"=0D
line from being surprising for what was a mutex/spinlock at the source=0D
level.=0D
=0D
> +#if defined(CONFIG_64BIT)=0D
> +	hung_task_set_blocker(task, lock, BLOCKER_TYPE_RTMUTEX);=0D
> +#endif=0D
=0D
(and the three matching clear sites)=0D
=0D
Would it be cleaner to hide the CONFIG_64BIT-ness inside the hung_task=0D
helpers - e.g. a no-op rtmutex set/clear on 32-bit - so rtmutex.c doesn't=0D
need the scattered #if at each call site?=0D
=0D
Thanks,=0D
Zhan Xusheng=0D