linux-kernel - Re: [PATCH v5 1/3] hung_task: replace blocker

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250415034422.95089-1-ioworker0@gmail.com>
Date: Tue, 15 Apr 2025 11:44:21 +0800
From: Lance Yang <ioworker0@...il.com>
To: akpm@...ux-foundation.org
Cc: amaindex@...look.com,
	anna.schumaker@...cle.com,
	boqun.feng@...il.com,
	ioworker0@...il.com,
	joel.granados@...nel.org,
	jstultz@...gle.com,
	kent.overstreet@...ux.dev,
	leonylgao@...cent.com,
	linux-kernel@...r.kernel.org,
	longman@...hat.com,
	mhiramat@...nel.org,
	mingo@...hat.com,
	mingzhe.yang@...com,
	peterz@...radead.org,
	rostedt@...dmis.org,
	senozhatsky@...omium.org,
	tfiga@...omium.org,
	will@...nel.org
Subject: Re: [PATCH v5 1/3] hung_task: replace blocker_mutex with encoded blocker

Hi Andrew,

On Tue, Apr 15, 2025 at 5:36 AM Andrew Morton <akpm@...ux-foundation.org> wrote:
>
> On Mon, 14 Apr 2025 22:59:43 +0800 Lance Yang <ioworker0@...il.com> 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@gmail.com

Thanks,
Lance