Re: [PATCH v5 1/3] hung_task: replace blocker_mutex with encoded blocker

[Lance Yang]

Hi Andrew,

On Tue, Apr 15, 2025 at 5:36 AM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, 14 Apr 2025 22:59:43 +0800 Lance Yang <ioworker0@xxxxxxxxx> wrote:
>
> > This patch replaces 'struct mutex *blocker_mutex' with 'unsigned long
> > blocker', as only one blocker is active at a time.
> >
> > The blocker filed can store both the lock addrees and the lock type, with
> > LSB used to encode the type as Masami suggested, making it easier to extend
> > the feature to cover other types of locks.
> >
> > Also, once the lock type is determined, we can directly extract the address
> > and cast it to a lock pointer ;)
> >
> > ...
> >
> >  static void debug_show_blocker(struct task_struct *task)
> >  {
> >       struct task_struct *g, *t;
> > -     unsigned long owner;
> > -     struct mutex *lock;
> > +     unsigned long owner, blocker;
> > 
> >       RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "No rcu lock held");
> > 
> > -     lock = READ_ONCE(task->blocker_mutex);
> > -     if (!lock)
> > +     blocker = READ_ONCE(task->blocker);
> > +     if (!blocker ||
> > +         hung_task_get_blocker_type(blocker) != BLOCKER_TYPE_MUTEX)
> >               return;
> > 
> > -     owner = mutex_get_owner(lock);
> > +     owner = mutex_get_owner(
> > +             (struct mutex *)hung_task_blocker_to_lock(blocker));
>
> typecast is unneeded?

Good catch! This typecast is redundant here. Since the #02[1] patch already
covers this code section, could you please fold the following change into
that patch?

diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index d2432df2b905..b30b9ab17694 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -111,12 +111,10 @@ static void debug_show_blocker(struct task_struct *task)
 
 	switch (blocker_type) {
 	case BLOCKER_TYPE_MUTEX:
-		owner = mutex_get_owner(
-			(struct mutex *)hung_task_blocker_to_lock(blocker));
+		owner = mutex_get_owner(hung_task_blocker_to_lock(blocker));
 		break;
 	case BLOCKER_TYPE_SEM:
-		owner = sem_last_holder(
-			(struct semaphore *)hung_task_blocker_to_lock(blocker));
+		owner = sem_last_holder(hung_task_blocker_to_lock(blocker));
 		break;
 	default:
 		WARN_ON_ONCE(1);
--

[1] https://lore.kernel.org/all/20250414145945.84916-3-ioworker0@xxxxxxxxx

Thanks,
Lance