The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH 1/1] hung_task: show the blocker task if the task is hung on semaphore
@ 2025-03-01  5:51 Lance Yang
  2025-03-03  6:29 ` Masami Hiramatsu
  0 siblings, 1 reply; 6+ messages in thread
From: Lance Yang @ 2025-03-01  5:51 UTC (permalink / raw)
  To: akpm, will, peterz, mingo
  Cc: longman, mhiramat, anna.schumaker, boqun.feng, joel.granados,
	kent.overstreet, leonylgao, linux-kernel, rostedt, senozhatsky,
	tfiga, Lance Yang, Mingzhe Yang

Inspired by mutex blocker tracking[1], this patch makes a trade-off to
balance the overhead and utility of the hung task detector.

Unlike mutexes, semaphores lack explicit ownership tracking, making it
challenging to identify the root cause of hangs. To address this, we
introduce a last_holder field to the semaphore structure, which is
updated when a task successfully calls down() and cleared during up().

The assumption is that if a task is blocked on a semaphore, the holders
must not have released it. While this does not guarantee that the last
holder is one of the current blockers, it likely provides a practical hint
for diagnosing semaphore-related stalls.

With this change, the hung task detector can now show blocker task's info
like below:

[Sat Mar  1 02:39:52 2025] INFO: task cat:1437 blocked for more than 122 seconds.
[Sat Mar  1 02:39:52 2025]       Tainted: G           OE      6.14.0-rc3+ #9
[Sat Mar  1 02:39:52 2025] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[Sat Mar  1 02:39:52 2025] task:cat             state:D stack:0     pid:1437  tgid:1437  ppid:1007   task_flags:0x400000 flags:0x00000004
[Sat Mar  1 02:39:52 2025] Call trace:
[Sat Mar  1 02:39:52 2025]  __switch_to+0x1ec/0x380 (T)
[Sat Mar  1 02:39:52 2025]  __schedule+0xc30/0x44f8
[Sat Mar  1 02:39:52 2025]  schedule+0xb8/0x3b0
[Sat Mar  1 02:39:52 2025]  schedule_timeout+0x1d0/0x208
[Sat Mar  1 02:39:52 2025]  __down_common+0x27c/0x600
[Sat Mar  1 02:39:52 2025]  __down+0x24/0x50
[Sat Mar  1 02:39:52 2025]  down+0xe0/0x140
[Sat Mar  1 02:39:52 2025]  read_dummy+0x3c/0xa0 [hung_task_sem]
[Sat Mar  1 02:39:52 2025]  full_proxy_read+0xfc/0x1d0
[Sat Mar  1 02:39:52 2025]  vfs_read+0x1a0/0x858
[Sat Mar  1 02:39:52 2025]  ksys_read+0x100/0x220
[Sat Mar  1 02:39:52 2025]  __arm64_sys_read+0x78/0xc8
[Sat Mar  1 02:39:52 2025]  invoke_syscall+0xd8/0x278
[Sat Mar  1 02:39:52 2025]  el0_svc_common.constprop.0+0xb8/0x298
[Sat Mar  1 02:39:52 2025]  do_el0_svc+0x4c/0x88
[Sat Mar  1 02:39:52 2025]  el0_svc+0x44/0x108
[Sat Mar  1 02:39:52 2025]  el0t_64_sync_handler+0x134/0x160
[Sat Mar  1 02:39:52 2025]  el0t_64_sync+0x1b8/0x1c0
[Sat Mar  1 02:39:52 2025] INFO: task cat:1437 blocked on a semaphore likely last held by task cat:1436
[Sat Mar  1 02:39:52 2025] task:cat             state:S stack:0     pid:1436  tgid:1436  ppid:1007   task_flags:0x400000 flags:0x00000004
[Sat Mar  1 02:39:52 2025] Call trace:
[Sat Mar  1 02:39:52 2025]  __switch_to+0x1ec/0x380 (T)
[Sat Mar  1 02:39:52 2025]  __schedule+0xc30/0x44f8
[Sat Mar  1 02:39:52 2025]  schedule+0xb8/0x3b0
[Sat Mar  1 02:39:52 2025]  schedule_timeout+0xf4/0x208
[Sat Mar  1 02:39:52 2025]  msleep_interruptible+0x70/0x130
[Sat Mar  1 02:39:52 2025]  read_dummy+0x48/0xa0 [hung_task_sem]
[Sat Mar  1 02:39:52 2025]  full_proxy_read+0xfc/0x1d0
[Sat Mar  1 02:39:52 2025]  vfs_read+0x1a0/0x858
[Sat Mar  1 02:39:52 2025]  ksys_read+0x100/0x220
[Sat Mar  1 02:39:52 2025]  __arm64_sys_read+0x78/0xc8
[Sat Mar  1 02:39:52 2025]  invoke_syscall+0xd8/0x278
[Sat Mar  1 02:39:52 2025]  el0_svc_common.constprop.0+0xb8/0x298
[Sat Mar  1 02:39:52 2025]  do_el0_svc+0x4c/0x88
[Sat Mar  1 02:39:52 2025]  el0_svc+0x44/0x108
[Sat Mar  1 02:39:52 2025]  el0t_64_sync_handler+0x134/0x160
[Sat Mar  1 02:39:52 2025]  el0t_64_sync+0x1b8/0x1c0

[1] https://lore.kernel.org/all/174046694331.2194069.15472952050240807469.stgit@mhiramat.tok.corp.google.com

Signed-off-by: Mingzhe Yang <mingzhe.yang@ly.com>
Signed-off-by: Lance Yang <ioworker0@gmail.com>
---
 include/linux/sched.h      |  1 +
 include/linux/semaphore.h  | 15 ++++++++++-
 kernel/hung_task.c         | 52 ++++++++++++++++++++++++++-----------
 kernel/locking/semaphore.c | 53 ++++++++++++++++++++++++++++++++++----
 4 files changed, 100 insertions(+), 21 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 0cebdd736d44..5dfdca879ac4 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1219,6 +1219,7 @@ struct task_struct {
 
 #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
 	struct mutex			*blocker_mutex;
+	struct semaphore		*blocker_sem;
 #endif
 
 #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
diff --git a/include/linux/semaphore.h b/include/linux/semaphore.h
index 04655faadc2d..ca8240a5dbfc 100644
--- a/include/linux/semaphore.h
+++ b/include/linux/semaphore.h
@@ -16,13 +16,25 @@ struct semaphore {
 	raw_spinlock_t		lock;
 	unsigned int		count;
 	struct list_head	wait_list;
+
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+	atomic_long_t last_holder;
+#endif
 };
 
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+#define __LAST_HOLDER_SEMAPHORE_INITIALIZER				\
+	, .last_holder = ATOMIC_LONG_INIT(0)
+#else
+#define __LAST_HOLDER_SEMAPHORE_INITIALIZER
+#endif
+
 #define __SEMAPHORE_INITIALIZER(name, n)				\
 {									\
 	.lock		= __RAW_SPIN_LOCK_UNLOCKED((name).lock),	\
 	.count		= n,						\
-	.wait_list	= LIST_HEAD_INIT((name).wait_list),		\
+	.wait_list	= LIST_HEAD_INIT((name).wait_list)		\
+	__LAST_HOLDER_SEMAPHORE_INITIALIZER				\
 }
 
 /*
@@ -47,5 +59,6 @@ extern int __must_check down_killable(struct semaphore *sem);
 extern int __must_check down_trylock(struct semaphore *sem);
 extern int __must_check down_timeout(struct semaphore *sem, long jiffies);
 extern void up(struct semaphore *sem);
+extern unsigned long sem_last_holder(struct semaphore *sem);
 
 #endif /* __LINUX_SEMAPHORE_H */
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index ccd7217fcec1..332f555a97a9 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -98,30 +98,52 @@ static struct notifier_block panic_block = {
 static void debug_show_blocker(struct task_struct *task)
 {
 	struct task_struct *g, *t;
-	unsigned long owner;
-	struct mutex *lock;
+	unsigned long owner, holder;
+	struct semaphore *sem_lock;
+	struct mutex *mutex_lock;
 
 	RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "No rcu lock held");
 
-	lock = READ_ONCE(task->blocker_mutex);
-	if (!lock)
-		return;
+	mutex_lock = READ_ONCE(task->blocker_mutex);
+	if (mutex_lock) {
+		owner = mutex_get_owner(mutex_lock);
+		if (unlikely(!owner)) {
+			pr_err("INFO: task %s:%d is blocked on a mutex, but the owner is not found.\n",
+			       task->comm, task->pid);
+			goto blocker_sem;
+		}
 
-	owner = mutex_get_owner(lock);
-	if (unlikely(!owner)) {
-		pr_err("INFO: task %s:%d is blocked on a mutex, but the owner is not found.\n",
-			task->comm, task->pid);
+		/* Ensure the owner information is correct. */
+		for_each_process_thread(g, t) {
+			if ((unsigned long)t == owner) {
+				pr_err("INFO: task %s:%d is blocked on a mutex likely owned by task %s:%d.\n",
+				       task->comm, task->pid, t->comm, t->pid);
+				sched_show_task(t);
+				return;
+			}
+		}
 		return;
 	}
 
-	/* Ensure the owner information is correct. */
-	for_each_process_thread(g, t) {
-		if ((unsigned long)t == owner) {
-			pr_err("INFO: task %s:%d is blocked on a mutex likely owned by task %s:%d.\n",
-				task->comm, task->pid, t->comm, t->pid);
-			sched_show_task(t);
+blocker_sem:
+	sem_lock = READ_ONCE(task->blocker_sem);
+	if (sem_lock) {
+		holder = sem_last_holder(sem_lock);
+		if (unlikely(!holder)) {
+			pr_err("INFO: task %s:%d is blocked on a semaphore, but the last holder is not found.\n",
+			       task->comm, task->pid);
 			return;
 		}
+
+		for_each_process_thread(g, t) {
+			if ((unsigned long)t == holder) {
+				pr_err("INFO: task %s:%d blocked on a semaphore likely last held by task %s:%d\n",
+				       task->comm, task->pid, t->comm, t->pid);
+				sched_show_task(t);
+				return;
+			}
+		}
+		return;
 	}
 }
 #else
diff --git a/kernel/locking/semaphore.c b/kernel/locking/semaphore.c
index 34bfae72f295..5a684c0a3087 100644
--- a/kernel/locking/semaphore.c
+++ b/kernel/locking/semaphore.c
@@ -39,6 +39,7 @@ static noinline int __down_interruptible(struct semaphore *sem);
 static noinline int __down_killable(struct semaphore *sem);
 static noinline int __down_timeout(struct semaphore *sem, long timeout);
 static noinline void __up(struct semaphore *sem);
+static inline void __sem_acquire(struct semaphore *sem);
 
 /**
  * down - acquire the semaphore
@@ -58,7 +59,7 @@ void __sched down(struct semaphore *sem)
 	might_sleep();
 	raw_spin_lock_irqsave(&sem->lock, flags);
 	if (likely(sem->count > 0))
-		sem->count--;
+		__sem_acquire(sem);
 	else
 		__down(sem);
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
@@ -82,7 +83,7 @@ int __sched down_interruptible(struct semaphore *sem)
 	might_sleep();
 	raw_spin_lock_irqsave(&sem->lock, flags);
 	if (likely(sem->count > 0))
-		sem->count--;
+		__sem_acquire(sem);
 	else
 		result = __down_interruptible(sem);
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
@@ -109,7 +110,7 @@ int __sched down_killable(struct semaphore *sem)
 	might_sleep();
 	raw_spin_lock_irqsave(&sem->lock, flags);
 	if (likely(sem->count > 0))
-		sem->count--;
+		__sem_acquire(sem);
 	else
 		result = __down_killable(sem);
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
@@ -139,7 +140,7 @@ int __sched down_trylock(struct semaphore *sem)
 	raw_spin_lock_irqsave(&sem->lock, flags);
 	count = sem->count - 1;
 	if (likely(count >= 0))
-		sem->count = count;
+		__sem_acquire(sem);
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
 
 	return (count < 0);
@@ -164,7 +165,7 @@ int __sched down_timeout(struct semaphore *sem, long timeout)
 	might_sleep();
 	raw_spin_lock_irqsave(&sem->lock, flags);
 	if (likely(sem->count > 0))
-		sem->count--;
+		__sem_acquire(sem);
 	else
 		result = __down_timeout(sem, timeout);
 	raw_spin_unlock_irqrestore(&sem->lock, flags);
@@ -242,10 +243,18 @@ static inline int __sched __down_common(struct semaphore *sem, long state,
 {
 	int ret;
 
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+	WRITE_ONCE(current->blocker_sem, sem);
+#endif
+
 	trace_contention_begin(sem, 0);
 	ret = ___down_common(sem, state, timeout);
 	trace_contention_end(sem, ret);
 
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+	WRITE_ONCE(current->blocker_sem, NULL);
+#endif
+
 	return ret;
 }
 
@@ -274,6 +283,40 @@ static noinline void __sched __up(struct semaphore *sem)
 	struct semaphore_waiter *waiter = list_first_entry(&sem->wait_list,
 						struct semaphore_waiter, list);
 	list_del(&waiter->list);
+
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+	atomic_long_cmpxchg_release(&sem->last_holder, (unsigned long)current,
+				    0UL);
+#endif
+
 	waiter->up = true;
 	wake_up_process(waiter->task);
 }
+
+static inline struct task_struct *__holder_task(unsigned long holder)
+{
+	return (struct task_struct *)holder;
+}
+
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+/* Do not use the return value as a pointer directly. */
+unsigned long sem_last_holder(struct semaphore *sem)
+{
+	unsigned long holder = atomic_long_read(&sem->last_holder);
+
+	return (unsigned long)__holder_task(holder);
+}
+#else
+unsigned long sem_last_holder(struct semaphore *sem)
+{
+	return 0;
+}
+#endif
+
+static inline void __sem_acquire(struct semaphore *sem)
+{
+	sem->count--;
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+	atomic_long_set(&sem->last_holder, (unsigned long)current);
+#endif
+}
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] hung_task: show the blocker task if the task is hung on semaphore
  2025-03-01  5:51 [PATCH 1/1] hung_task: show the blocker task if the task is hung on semaphore Lance Yang
@ 2025-03-03  6:29 ` Masami Hiramatsu
  2025-03-03 10:52   ` Lance Yang
  2025-03-06  6:05   ` Lance Yang
  0 siblings, 2 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2025-03-03  6:29 UTC (permalink / raw)
  To: Lance Yang
  Cc: akpm, will, peterz, mingo, longman, mhiramat, anna.schumaker,
	boqun.feng, joel.granados, kent.overstreet, leonylgao,
	linux-kernel, rostedt, senozhatsky, tfiga, Mingzhe Yang

On Sat,  1 Mar 2025 13:51:02 +0800
Lance Yang <ioworker0@gmail.com> wrote:

> Inspired by mutex blocker tracking[1], this patch makes a trade-off to
> balance the overhead and utility of the hung task detector.

Thanks for adding new one! 

> 
> Unlike mutexes, semaphores lack explicit ownership tracking, making it
> challenging to identify the root cause of hangs. To address this, we
> introduce a last_holder field to the semaphore structure, which is
> updated when a task successfully calls down() and cleared during up().
> 
> The assumption is that if a task is blocked on a semaphore, the holders
> must not have released it. While this does not guarantee that the last
> holder is one of the current blockers, it likely provides a practical hint
> for diagnosing semaphore-related stalls.

Yeah, if we can have something like the owner_list, we can find the
longest time blocker (owner) but it takes more {memory, performance}
overheads.

> 
> With this change, the hung task detector can now show blocker task's info
> like below:
> 
> [Sat Mar  1 02:39:52 2025] INFO: task cat:1437 blocked for more than 122 seconds.
> [Sat Mar  1 02:39:52 2025]       Tainted: G           OE      6.14.0-rc3+ #9
> [Sat Mar  1 02:39:52 2025] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [Sat Mar  1 02:39:52 2025] task:cat             state:D stack:0     pid:1437  tgid:1437  ppid:1007   task_flags:0x400000 flags:0x00000004
> [Sat Mar  1 02:39:52 2025] Call trace:
> [Sat Mar  1 02:39:52 2025]  __switch_to+0x1ec/0x380 (T)
> [Sat Mar  1 02:39:52 2025]  __schedule+0xc30/0x44f8
> [Sat Mar  1 02:39:52 2025]  schedule+0xb8/0x3b0
> [Sat Mar  1 02:39:52 2025]  schedule_timeout+0x1d0/0x208
> [Sat Mar  1 02:39:52 2025]  __down_common+0x27c/0x600
> [Sat Mar  1 02:39:52 2025]  __down+0x24/0x50
> [Sat Mar  1 02:39:52 2025]  down+0xe0/0x140
> [Sat Mar  1 02:39:52 2025]  read_dummy+0x3c/0xa0 [hung_task_sem]
> [Sat Mar  1 02:39:52 2025]  full_proxy_read+0xfc/0x1d0
> [Sat Mar  1 02:39:52 2025]  vfs_read+0x1a0/0x858
> [Sat Mar  1 02:39:52 2025]  ksys_read+0x100/0x220
> [Sat Mar  1 02:39:52 2025]  __arm64_sys_read+0x78/0xc8
> [Sat Mar  1 02:39:52 2025]  invoke_syscall+0xd8/0x278
> [Sat Mar  1 02:39:52 2025]  el0_svc_common.constprop.0+0xb8/0x298
> [Sat Mar  1 02:39:52 2025]  do_el0_svc+0x4c/0x88
> [Sat Mar  1 02:39:52 2025]  el0_svc+0x44/0x108
> [Sat Mar  1 02:39:52 2025]  el0t_64_sync_handler+0x134/0x160
> [Sat Mar  1 02:39:52 2025]  el0t_64_sync+0x1b8/0x1c0
> [Sat Mar  1 02:39:52 2025] INFO: task cat:1437 blocked on a semaphore likely last held by task cat:1436
> [Sat Mar  1 02:39:52 2025] task:cat             state:S stack:0     pid:1436  tgid:1436  ppid:1007   task_flags:0x400000 flags:0x00000004
> [Sat Mar  1 02:39:52 2025] Call trace:
> [Sat Mar  1 02:39:52 2025]  __switch_to+0x1ec/0x380 (T)
> [Sat Mar  1 02:39:52 2025]  __schedule+0xc30/0x44f8
> [Sat Mar  1 02:39:52 2025]  schedule+0xb8/0x3b0
> [Sat Mar  1 02:39:52 2025]  schedule_timeout+0xf4/0x208
> [Sat Mar  1 02:39:52 2025]  msleep_interruptible+0x70/0x130
> [Sat Mar  1 02:39:52 2025]  read_dummy+0x48/0xa0 [hung_task_sem]
> [Sat Mar  1 02:39:52 2025]  full_proxy_read+0xfc/0x1d0
> [Sat Mar  1 02:39:52 2025]  vfs_read+0x1a0/0x858
> [Sat Mar  1 02:39:52 2025]  ksys_read+0x100/0x220
> [Sat Mar  1 02:39:52 2025]  __arm64_sys_read+0x78/0xc8
> [Sat Mar  1 02:39:52 2025]  invoke_syscall+0xd8/0x278
> [Sat Mar  1 02:39:52 2025]  el0_svc_common.constprop.0+0xb8/0x298
> [Sat Mar  1 02:39:52 2025]  do_el0_svc+0x4c/0x88
> [Sat Mar  1 02:39:52 2025]  el0_svc+0x44/0x108
> [Sat Mar  1 02:39:52 2025]  el0t_64_sync_handler+0x134/0x160
> [Sat Mar  1 02:39:52 2025]  el0t_64_sync+0x1b8/0x1c0
> 
> [1] https://lore.kernel.org/all/174046694331.2194069.15472952050240807469.stgit@mhiramat.tok.corp.google.com
> 
> Signed-off-by: Mingzhe Yang <mingzhe.yang@ly.com>
> Signed-off-by: Lance Yang <ioworker0@gmail.com>
> ---
>  include/linux/sched.h      |  1 +
>  include/linux/semaphore.h  | 15 ++++++++++-
>  kernel/hung_task.c         | 52 ++++++++++++++++++++++++++-----------
>  kernel/locking/semaphore.c | 53 ++++++++++++++++++++++++++++++++++----
>  4 files changed, 100 insertions(+), 21 deletions(-)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 0cebdd736d44..5dfdca879ac4 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1219,6 +1219,7 @@ struct task_struct {
>  
>  #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
>  	struct mutex			*blocker_mutex;
> +	struct semaphore		*blocker_sem;

Can we make this a union because only one blocker is active at a time?
Waiman has been suggested the update way;

https://lore.kernel.org/all/9f9150b4-1cf5-4380-b431-419f70775a7d@redhat.com/

If we can use MSB or LSB of those pointers, it is the smallest memory
footprint.

>  #endif
>  
>  #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
> diff --git a/include/linux/semaphore.h b/include/linux/semaphore.h
> index 04655faadc2d..ca8240a5dbfc 100644
> --- a/include/linux/semaphore.h
> +++ b/include/linux/semaphore.h
> @@ -16,13 +16,25 @@ struct semaphore {
>  	raw_spinlock_t		lock;
>  	unsigned int		count;
>  	struct list_head	wait_list;
> +
> +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> +	atomic_long_t last_holder;
> +#endif

Would we need to make this atomic? I think mutex needs to use it directly
as a pointer, but debug_show_blocker() searches the task, so we can
use an unsigned long + WRITE_ONCE()/READ_ONCE().

>  };
>  
> +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> +#define __LAST_HOLDER_SEMAPHORE_INITIALIZER				\
> +	, .last_holder = ATOMIC_LONG_INIT(0)
> +#else
> +#define __LAST_HOLDER_SEMAPHORE_INITIALIZER
> +#endif
> +
>  #define __SEMAPHORE_INITIALIZER(name, n)				\
>  {									\
>  	.lock		= __RAW_SPIN_LOCK_UNLOCKED((name).lock),	\
>  	.count		= n,						\
> -	.wait_list	= LIST_HEAD_INIT((name).wait_list),		\
> +	.wait_list	= LIST_HEAD_INIT((name).wait_list)		\
> +	__LAST_HOLDER_SEMAPHORE_INITIALIZER				\
>  }
>  
>  /*
> @@ -47,5 +59,6 @@ extern int __must_check down_killable(struct semaphore *sem);
>  extern int __must_check down_trylock(struct semaphore *sem);
>  extern int __must_check down_timeout(struct semaphore *sem, long jiffies);
>  extern void up(struct semaphore *sem);
> +extern unsigned long sem_last_holder(struct semaphore *sem);
>  
>  #endif /* __LINUX_SEMAPHORE_H */
> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index ccd7217fcec1..332f555a97a9 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -98,30 +98,52 @@ static struct notifier_block panic_block = {
>  static void debug_show_blocker(struct task_struct *task)
>  {
>  	struct task_struct *g, *t;
> -	unsigned long owner;
> -	struct mutex *lock;
> +	unsigned long owner, holder;
> +	struct semaphore *sem_lock;
> +	struct mutex *mutex_lock;
>  
>  	RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "No rcu lock held");
>  
> -	lock = READ_ONCE(task->blocker_mutex);
> -	if (!lock)
> -		return;
> +	mutex_lock = READ_ONCE(task->blocker_mutex);
> +	if (mutex_lock) {
> +		owner = mutex_get_owner(mutex_lock);
> +		if (unlikely(!owner)) {
> +			pr_err("INFO: task %s:%d is blocked on a mutex, but the owner is not found.\n",
> +			       task->comm, task->pid);
> +			goto blocker_sem;
> +		}
>  
> -	owner = mutex_get_owner(lock);
> -	if (unlikely(!owner)) {
> -		pr_err("INFO: task %s:%d is blocked on a mutex, but the owner is not found.\n",
> -			task->comm, task->pid);
> +		/* Ensure the owner information is correct. */
> +		for_each_process_thread(g, t) {
> +			if ((unsigned long)t == owner) {
> +				pr_err("INFO: task %s:%d is blocked on a mutex likely owned by task %s:%d.\n",
> +				       task->comm, task->pid, t->comm, t->pid);
> +				sched_show_task(t);
> +				return;
> +			}
> +		}
>  		return;
>  	}
>  
> -	/* Ensure the owner information is correct. */
> -	for_each_process_thread(g, t) {
> -		if ((unsigned long)t == owner) {
> -			pr_err("INFO: task %s:%d is blocked on a mutex likely owned by task %s:%d.\n",
> -				task->comm, task->pid, t->comm, t->pid);
> -			sched_show_task(t);
> +blocker_sem:
> +	sem_lock = READ_ONCE(task->blocker_sem);
> +	if (sem_lock) {
> +		holder = sem_last_holder(sem_lock);
> +		if (unlikely(!holder)) {
> +			pr_err("INFO: task %s:%d is blocked on a semaphore, but the last holder is not found.\n",
> +			       task->comm, task->pid);
>  			return;
>  		}
> +
> +		for_each_process_thread(g, t) {
> +			if ((unsigned long)t == holder) {
> +				pr_err("INFO: task %s:%d blocked on a semaphore likely last held by task %s:%d\n",
> +				       task->comm, task->pid, t->comm, t->pid);
> +				sched_show_task(t);
> +				return;
> +			}
> +		}
> +		return;
>  	}
>  }
>  #else
> diff --git a/kernel/locking/semaphore.c b/kernel/locking/semaphore.c
> index 34bfae72f295..5a684c0a3087 100644
> --- a/kernel/locking/semaphore.c
> +++ b/kernel/locking/semaphore.c
> @@ -39,6 +39,7 @@ static noinline int __down_interruptible(struct semaphore *sem);
>  static noinline int __down_killable(struct semaphore *sem);
>  static noinline int __down_timeout(struct semaphore *sem, long timeout);
>  static noinline void __up(struct semaphore *sem);
> +static inline void __sem_acquire(struct semaphore *sem);
>  
>  /**
>   * down - acquire the semaphore
> @@ -58,7 +59,7 @@ void __sched down(struct semaphore *sem)
>  	might_sleep();
>  	raw_spin_lock_irqsave(&sem->lock, flags);
>  	if (likely(sem->count > 0))
> -		sem->count--;
> +		__sem_acquire(sem);
>  	else
>  		__down(sem);
>  	raw_spin_unlock_irqrestore(&sem->lock, flags);
> @@ -82,7 +83,7 @@ int __sched down_interruptible(struct semaphore *sem)
>  	might_sleep();
>  	raw_spin_lock_irqsave(&sem->lock, flags);
>  	if (likely(sem->count > 0))
> -		sem->count--;
> +		__sem_acquire(sem);
>  	else
>  		result = __down_interruptible(sem);
>  	raw_spin_unlock_irqrestore(&sem->lock, flags);
> @@ -109,7 +110,7 @@ int __sched down_killable(struct semaphore *sem)
>  	might_sleep();
>  	raw_spin_lock_irqsave(&sem->lock, flags);
>  	if (likely(sem->count > 0))
> -		sem->count--;
> +		__sem_acquire(sem);
>  	else
>  		result = __down_killable(sem);
>  	raw_spin_unlock_irqrestore(&sem->lock, flags);
> @@ -139,7 +140,7 @@ int __sched down_trylock(struct semaphore *sem)
>  	raw_spin_lock_irqsave(&sem->lock, flags);
>  	count = sem->count - 1;
>  	if (likely(count >= 0))
> -		sem->count = count;
> +		__sem_acquire(sem);
>  	raw_spin_unlock_irqrestore(&sem->lock, flags);
>  
>  	return (count < 0);
> @@ -164,7 +165,7 @@ int __sched down_timeout(struct semaphore *sem, long timeout)
>  	might_sleep();
>  	raw_spin_lock_irqsave(&sem->lock, flags);
>  	if (likely(sem->count > 0))
> -		sem->count--;
> +		__sem_acquire(sem);
>  	else
>  		result = __down_timeout(sem, timeout);
>  	raw_spin_unlock_irqrestore(&sem->lock, flags);
> @@ -242,10 +243,18 @@ static inline int __sched __down_common(struct semaphore *sem, long state,
>  {
>  	int ret;
>  
> +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> +	WRITE_ONCE(current->blocker_sem, sem);
> +#endif
> +
>  	trace_contention_begin(sem, 0);
>  	ret = ___down_common(sem, state, timeout);
>  	trace_contention_end(sem, ret);
>  
> +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> +	WRITE_ONCE(current->blocker_sem, NULL);
> +#endif
> +
>  	return ret;
>  }
>  
> @@ -274,6 +283,40 @@ static noinline void __sched __up(struct semaphore *sem)
>  	struct semaphore_waiter *waiter = list_first_entry(&sem->wait_list,
>  						struct semaphore_waiter, list);
>  	list_del(&waiter->list);
> +
> +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> +	atomic_long_cmpxchg_release(&sem->last_holder, (unsigned long)current,
> +				    0UL);

Ah, you don't need atomic here because __up() is called with sem->lock locked.
Also, if wait_list is empty, the 'up()' does only sem->count++. In this case,
the owner is not cleared.

Thank you,

> +#endif
> +
>  	waiter->up = true;
>  	wake_up_process(waiter->task);
>  }
> +
> +static inline struct task_struct *__holder_task(unsigned long holder)
> +{
> +	return (struct task_struct *)holder;
> +}
> +
> +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> +/* Do not use the return value as a pointer directly. */
> +unsigned long sem_last_holder(struct semaphore *sem)
> +{
> +	unsigned long holder = atomic_long_read(&sem->last_holder);
> +
> +	return (unsigned long)__holder_task(holder);
> +}
> +#else
> +unsigned long sem_last_holder(struct semaphore *sem)
> +{
> +	return 0;
> +}
> +#endif
> +
> +static inline void __sem_acquire(struct semaphore *sem)
> +{
> +	sem->count--;
> +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> +	atomic_long_set(&sem->last_holder, (unsigned long)current);
> +#endif
> +}
> -- 
> 2.45.2
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] hung_task: show the blocker task if the task is hung on semaphore
  2025-03-03  6:29 ` Masami Hiramatsu
@ 2025-03-03 10:52   ` Lance Yang
  2025-03-06  6:05   ` Lance Yang
  1 sibling, 0 replies; 6+ messages in thread
From: Lance Yang @ 2025-03-03 10:52 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: akpm, will, peterz, mingo, longman, anna.schumaker, boqun.feng,
	joel.granados, kent.overstreet, leonylgao, linux-kernel, rostedt,
	senozhatsky, tfiga, Mingzhe Yang

Hi Masami,

Thanks a lot for taking time to review!

On Mon, Mar 3, 2025 at 2:29 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Sat,  1 Mar 2025 13:51:02 +0800
> Lance Yang <ioworker0@gmail.com> wrote:
>
> > Inspired by mutex blocker tracking[1], this patch makes a trade-off to
> > balance the overhead and utility of the hung task detector.
>
> Thanks for adding new one!
>
> >
> > Unlike mutexes, semaphores lack explicit ownership tracking, making it
> > challenging to identify the root cause of hangs. To address this, we
> > introduce a last_holder field to the semaphore structure, which is
> > updated when a task successfully calls down() and cleared during up().
> >
> > The assumption is that if a task is blocked on a semaphore, the holders
> > must not have released it. While this does not guarantee that the last
> > holder is one of the current blockers, it likely provides a practical hint
> > for diagnosing semaphore-related stalls.
>
> Yeah, if we can have something like the owner_list, we can find the
> longest time blocker (owner) but it takes more {memory, performance}
> overheads.

You're right. Let's keep it simple and light for now without more
 {memory, performance} overheads ;p

>
> >
> > With this change, the hung task detector can now show blocker task's info
> > like below:
> >
> > [Sat Mar  1 02:39:52 2025] INFO: task cat:1437 blocked for more than 122 seconds.
> > [Sat Mar  1 02:39:52 2025]       Tainted: G           OE      6.14.0-rc3+ #9
> > [Sat Mar  1 02:39:52 2025] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> > [Sat Mar  1 02:39:52 2025] task:cat             state:D stack:0     pid:1437  tgid:1437  ppid:1007   task_flags:0x400000 flags:0x00000004
> > [Sat Mar  1 02:39:52 2025] Call trace:
> > [Sat Mar  1 02:39:52 2025]  __switch_to+0x1ec/0x380 (T)
> > [Sat Mar  1 02:39:52 2025]  __schedule+0xc30/0x44f8
> > [Sat Mar  1 02:39:52 2025]  schedule+0xb8/0x3b0
> > [Sat Mar  1 02:39:52 2025]  schedule_timeout+0x1d0/0x208
> > [Sat Mar  1 02:39:52 2025]  __down_common+0x27c/0x600
> > [Sat Mar  1 02:39:52 2025]  __down+0x24/0x50
> > [Sat Mar  1 02:39:52 2025]  down+0xe0/0x140
> > [Sat Mar  1 02:39:52 2025]  read_dummy+0x3c/0xa0 [hung_task_sem]
> > [Sat Mar  1 02:39:52 2025]  full_proxy_read+0xfc/0x1d0
> > [Sat Mar  1 02:39:52 2025]  vfs_read+0x1a0/0x858
> > [Sat Mar  1 02:39:52 2025]  ksys_read+0x100/0x220
> > [Sat Mar  1 02:39:52 2025]  __arm64_sys_read+0x78/0xc8
> > [Sat Mar  1 02:39:52 2025]  invoke_syscall+0xd8/0x278
> > [Sat Mar  1 02:39:52 2025]  el0_svc_common.constprop.0+0xb8/0x298
> > [Sat Mar  1 02:39:52 2025]  do_el0_svc+0x4c/0x88
> > [Sat Mar  1 02:39:52 2025]  el0_svc+0x44/0x108
> > [Sat Mar  1 02:39:52 2025]  el0t_64_sync_handler+0x134/0x160
> > [Sat Mar  1 02:39:52 2025]  el0t_64_sync+0x1b8/0x1c0
> > [Sat Mar  1 02:39:52 2025] INFO: task cat:1437 blocked on a semaphore likely last held by task cat:1436
> > [Sat Mar  1 02:39:52 2025] task:cat             state:S stack:0     pid:1436  tgid:1436  ppid:1007   task_flags:0x400000 flags:0x00000004
> > [Sat Mar  1 02:39:52 2025] Call trace:
> > [Sat Mar  1 02:39:52 2025]  __switch_to+0x1ec/0x380 (T)
> > [Sat Mar  1 02:39:52 2025]  __schedule+0xc30/0x44f8
> > [Sat Mar  1 02:39:52 2025]  schedule+0xb8/0x3b0
> > [Sat Mar  1 02:39:52 2025]  schedule_timeout+0xf4/0x208
> > [Sat Mar  1 02:39:52 2025]  msleep_interruptible+0x70/0x130
> > [Sat Mar  1 02:39:52 2025]  read_dummy+0x48/0xa0 [hung_task_sem]
> > [Sat Mar  1 02:39:52 2025]  full_proxy_read+0xfc/0x1d0
> > [Sat Mar  1 02:39:52 2025]  vfs_read+0x1a0/0x858
> > [Sat Mar  1 02:39:52 2025]  ksys_read+0x100/0x220
> > [Sat Mar  1 02:39:52 2025]  __arm64_sys_read+0x78/0xc8
> > [Sat Mar  1 02:39:52 2025]  invoke_syscall+0xd8/0x278
> > [Sat Mar  1 02:39:52 2025]  el0_svc_common.constprop.0+0xb8/0x298
> > [Sat Mar  1 02:39:52 2025]  do_el0_svc+0x4c/0x88
> > [Sat Mar  1 02:39:52 2025]  el0_svc+0x44/0x108
> > [Sat Mar  1 02:39:52 2025]  el0t_64_sync_handler+0x134/0x160
> > [Sat Mar  1 02:39:52 2025]  el0t_64_sync+0x1b8/0x1c0
> >
> > [1] https://lore.kernel.org/all/174046694331.2194069.15472952050240807469.stgit@mhiramat.tok.corp.google.com
> >
> > Signed-off-by: Mingzhe Yang <mingzhe.yang@ly.com>
> > Signed-off-by: Lance Yang <ioworker0@gmail.com>
> > ---
> >  include/linux/sched.h      |  1 +
> >  include/linux/semaphore.h  | 15 ++++++++++-
> >  kernel/hung_task.c         | 52 ++++++++++++++++++++++++++-----------
> >  kernel/locking/semaphore.c | 53 ++++++++++++++++++++++++++++++++++----
> >  4 files changed, 100 insertions(+), 21 deletions(-)
> >
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 0cebdd736d44..5dfdca879ac4 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -1219,6 +1219,7 @@ struct task_struct {
> >
> >  #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> >       struct mutex                    *blocker_mutex;
> > +     struct semaphore                *blocker_sem;
>
> Can we make this a union because only one blocker is active at a time?
> Waiman has been suggested the update way;
>
> https://lore.kernel.org/all/9f9150b4-1cf5-4380-b431-419f70775a7d@redhat.com/
>
> If we can use MSB or LSB of those pointers, it is the smallest memory
> footprint.

Nice! Thanks for sharing the link and the idea. I will look into it and see
if we can make it work ;)

>
> >  #endif
> >
> >  #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
> > diff --git a/include/linux/semaphore.h b/include/linux/semaphore.h
> > index 04655faadc2d..ca8240a5dbfc 100644
> > --- a/include/linux/semaphore.h
> > +++ b/include/linux/semaphore.h
> > @@ -16,13 +16,25 @@ struct semaphore {
> >       raw_spinlock_t          lock;
> >       unsigned int            count;
> >       struct list_head        wait_list;
> > +
> > +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > +     atomic_long_t last_holder;
> > +#endif
>
> Would we need to make this atomic? I think mutex needs to use it directly
> as a pointer, but debug_show_blocker() searches the task, so we can
> use an unsigned long + WRITE_ONCE()/READ_ONCE().

Good spot! Using 'an unsigned long + WRITE_ONCE()/READ_ONCE()' is
the way to go, so let me give it a try ;)

>
> >  };
> >
> > +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > +#define __LAST_HOLDER_SEMAPHORE_INITIALIZER                          \
> > +     , .last_holder = ATOMIC_LONG_INIT(0)
> > +#else
> > +#define __LAST_HOLDER_SEMAPHORE_INITIALIZER
> > +#endif
> > +
> >  #define __SEMAPHORE_INITIALIZER(name, n)                             \
> >  {                                                                    \
> >       .lock           = __RAW_SPIN_LOCK_UNLOCKED((name).lock),        \
> >       .count          = n,                                            \
> > -     .wait_list      = LIST_HEAD_INIT((name).wait_list),             \
> > +     .wait_list      = LIST_HEAD_INIT((name).wait_list)              \
> > +     __LAST_HOLDER_SEMAPHORE_INITIALIZER                             \
> >  }
> >
> >  /*
> > @@ -47,5 +59,6 @@ extern int __must_check down_killable(struct semaphore *sem);
> >  extern int __must_check down_trylock(struct semaphore *sem);
> >  extern int __must_check down_timeout(struct semaphore *sem, long jiffies);
> >  extern void up(struct semaphore *sem);
> > +extern unsigned long sem_last_holder(struct semaphore *sem);
> >
> >  #endif /* __LINUX_SEMAPHORE_H */
> > diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> > index ccd7217fcec1..332f555a97a9 100644
> > --- a/kernel/hung_task.c
> > +++ b/kernel/hung_task.c
> > @@ -98,30 +98,52 @@ static struct notifier_block panic_block = {
> >  static void debug_show_blocker(struct task_struct *task)
> >  {
> >       struct task_struct *g, *t;
> > -     unsigned long owner;
> > -     struct mutex *lock;
> > +     unsigned long owner, holder;
> > +     struct semaphore *sem_lock;
> > +     struct mutex *mutex_lock;
> >
> >       RCU_LOCKDEP_WARN(!rcu_read_lock_held(), "No rcu lock held");
> >
> > -     lock = READ_ONCE(task->blocker_mutex);
> > -     if (!lock)
> > -             return;
> > +     mutex_lock = READ_ONCE(task->blocker_mutex);
> > +     if (mutex_lock) {
> > +             owner = mutex_get_owner(mutex_lock);
> > +             if (unlikely(!owner)) {
> > +                     pr_err("INFO: task %s:%d is blocked on a mutex, but the owner is not found.\n",
> > +                            task->comm, task->pid);
> > +                     goto blocker_sem;
> > +             }
> >
> > -     owner = mutex_get_owner(lock);
> > -     if (unlikely(!owner)) {
> > -             pr_err("INFO: task %s:%d is blocked on a mutex, but the owner is not found.\n",
> > -                     task->comm, task->pid);
> > +             /* Ensure the owner information is correct. */
> > +             for_each_process_thread(g, t) {
> > +                     if ((unsigned long)t == owner) {
> > +                             pr_err("INFO: task %s:%d is blocked on a mutex likely owned by task %s:%d.\n",
> > +                                    task->comm, task->pid, t->comm, t->pid);
> > +                             sched_show_task(t);
> > +                             return;
> > +                     }
> > +             }
> >               return;
> >       }
> >
> > -     /* Ensure the owner information is correct. */
> > -     for_each_process_thread(g, t) {
> > -             if ((unsigned long)t == owner) {
> > -                     pr_err("INFO: task %s:%d is blocked on a mutex likely owned by task %s:%d.\n",
> > -                             task->comm, task->pid, t->comm, t->pid);
> > -                     sched_show_task(t);
> > +blocker_sem:
> > +     sem_lock = READ_ONCE(task->blocker_sem);
> > +     if (sem_lock) {
> > +             holder = sem_last_holder(sem_lock);
> > +             if (unlikely(!holder)) {
> > +                     pr_err("INFO: task %s:%d is blocked on a semaphore, but the last holder is not found.\n",
> > +                            task->comm, task->pid);
> >                       return;
> >               }
> > +
> > +             for_each_process_thread(g, t) {
> > +                     if ((unsigned long)t == holder) {
> > +                             pr_err("INFO: task %s:%d blocked on a semaphore likely last held by task %s:%d\n",
> > +                                    task->comm, task->pid, t->comm, t->pid);
> > +                             sched_show_task(t);
> > +                             return;
> > +                     }
> > +             }
> > +             return;
> >       }
> >  }
> >  #else
> > diff --git a/kernel/locking/semaphore.c b/kernel/locking/semaphore.c
> > index 34bfae72f295..5a684c0a3087 100644
> > --- a/kernel/locking/semaphore.c
> > +++ b/kernel/locking/semaphore.c
> > @@ -39,6 +39,7 @@ static noinline int __down_interruptible(struct semaphore *sem);
> >  static noinline int __down_killable(struct semaphore *sem);
> >  static noinline int __down_timeout(struct semaphore *sem, long timeout);
> >  static noinline void __up(struct semaphore *sem);
> > +static inline void __sem_acquire(struct semaphore *sem);
> >
> >  /**
> >   * down - acquire the semaphore
> > @@ -58,7 +59,7 @@ void __sched down(struct semaphore *sem)
> >       might_sleep();
> >       raw_spin_lock_irqsave(&sem->lock, flags);
> >       if (likely(sem->count > 0))
> > -             sem->count--;
> > +             __sem_acquire(sem);
> >       else
> >               __down(sem);
> >       raw_spin_unlock_irqrestore(&sem->lock, flags);
> > @@ -82,7 +83,7 @@ int __sched down_interruptible(struct semaphore *sem)
> >       might_sleep();
> >       raw_spin_lock_irqsave(&sem->lock, flags);
> >       if (likely(sem->count > 0))
> > -             sem->count--;
> > +             __sem_acquire(sem);
> >       else
> >               result = __down_interruptible(sem);
> >       raw_spin_unlock_irqrestore(&sem->lock, flags);
> > @@ -109,7 +110,7 @@ int __sched down_killable(struct semaphore *sem)
> >       might_sleep();
> >       raw_spin_lock_irqsave(&sem->lock, flags);
> >       if (likely(sem->count > 0))
> > -             sem->count--;
> > +             __sem_acquire(sem);
> >       else
> >               result = __down_killable(sem);
> >       raw_spin_unlock_irqrestore(&sem->lock, flags);
> > @@ -139,7 +140,7 @@ int __sched down_trylock(struct semaphore *sem)
> >       raw_spin_lock_irqsave(&sem->lock, flags);
> >       count = sem->count - 1;
> >       if (likely(count >= 0))
> > -             sem->count = count;
> > +             __sem_acquire(sem);
> >       raw_spin_unlock_irqrestore(&sem->lock, flags);
> >
> >       return (count < 0);
> > @@ -164,7 +165,7 @@ int __sched down_timeout(struct semaphore *sem, long timeout)
> >       might_sleep();
> >       raw_spin_lock_irqsave(&sem->lock, flags);
> >       if (likely(sem->count > 0))
> > -             sem->count--;
> > +             __sem_acquire(sem);
> >       else
> >               result = __down_timeout(sem, timeout);
> >       raw_spin_unlock_irqrestore(&sem->lock, flags);
> > @@ -242,10 +243,18 @@ static inline int __sched __down_common(struct semaphore *sem, long state,
> >  {
> >       int ret;
> >
> > +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > +     WRITE_ONCE(current->blocker_sem, sem);
> > +#endif
> > +
> >       trace_contention_begin(sem, 0);
> >       ret = ___down_common(sem, state, timeout);
> >       trace_contention_end(sem, ret);
> >
> > +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > +     WRITE_ONCE(current->blocker_sem, NULL);
> > +#endif
> > +
> >       return ret;
> >  }
> >
> > @@ -274,6 +283,40 @@ static noinline void __sched __up(struct semaphore *sem)
> >       struct semaphore_waiter *waiter = list_first_entry(&sem->wait_list,
> >                                               struct semaphore_waiter, list);
> >       list_del(&waiter->list);
> > +
> > +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > +     atomic_long_cmpxchg_release(&sem->last_holder, (unsigned long)current,
> > +                                 0UL);
>
> Ah, you don't need atomic here because __up() is called with sem->lock locked.

Indeed.

> Also, if wait_list is empty, the 'up()' does only sem->count++. In this case,
> the owner is not cleared.

Good catch! I totally missed that and will fix it in the next version.

Thanks a lot again for your time!
Lance


>
> Thank you,
>
> > +#endif
> > +
> >       waiter->up = true;
> >       wake_up_process(waiter->task);
> >  }
> > +
> > +static inline struct task_struct *__holder_task(unsigned long holder)
> > +{
> > +     return (struct task_struct *)holder;
> > +}
> > +
> > +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > +/* Do not use the return value as a pointer directly. */
> > +unsigned long sem_last_holder(struct semaphore *sem)
> > +{
> > +     unsigned long holder = atomic_long_read(&sem->last_holder);
> > +
> > +     return (unsigned long)__holder_task(holder);
> > +}
> > +#else
> > +unsigned long sem_last_holder(struct semaphore *sem)
> > +{
> > +     return 0;
> > +}
> > +#endif
> > +
> > +static inline void __sem_acquire(struct semaphore *sem)
> > +{
> > +     sem->count--;
> > +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > +     atomic_long_set(&sem->last_holder, (unsigned long)current);
> > +#endif
> > +}
> > --
> > 2.45.2
> >
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] hung_task: show the blocker task if the task is hung on semaphore
  2025-03-03  6:29 ` Masami Hiramatsu
  2025-03-03 10:52   ` Lance Yang
@ 2025-03-06  6:05   ` Lance Yang
  2025-03-11  4:19     ` Masami Hiramatsu
  1 sibling, 1 reply; 6+ messages in thread
From: Lance Yang @ 2025-03-06  6:05 UTC (permalink / raw)
  To: mhiramat
  Cc: akpm, anna.schumaker, boqun.feng, ioworker0, joel.granados,
	kent.overstreet, leonylgao, linux-kernel, longman, mingo,
	mingzhe.yang, peterz, rostedt, senozhatsky, tfiga, will

Hi Masami,

Could you give me a quick feedback before sending out the v2?

It seems unnecessary to make 'blocker' a union. I replaced 'struct mutex
*blocker_mutex' with 'unsigned long blocker', as only one blocker is active
at a time, IIUC ;)

The blocker filed can store both the lock addrees and the lock type, with
LSB used to encode the type as you 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 ;)

---
 include/linux/hung_task.h | 82 +++++++++++++++++++++++++++++++++++++++
 include/linux/sched.h     |  2 +-
 kernel/hung_task.c        | 15 ++++---
 kernel/locking/mutex.c    |  8 +++-
 4 files changed, 99 insertions(+), 8 deletions(-)
 create mode 100644 include/linux/hung_task.h

diff --git a/include/linux/hung_task.h b/include/linux/hung_task.h
new file mode 100644
index 000000000000..569d2e579f36
--- /dev/null
+++ b/include/linux/hung_task.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Detect Hung Task: detecting tasks stuck in D state
+ *
+ * Copyright 2025 Lance Yang <ioworker0@gmail.com>
+ */
+#ifndef __LINUX_HUNG_TASK_H
+#define __LINUX_HUNG_TASK_H
+
+#include <linux/bug.h>
+#include <linux/sched.h>
+#include <linux/compiler.h>
+
+/*
+ * @blocker: Combines lock address and blocking type.
+ *
+ * Since lock pointers are at least 4-byte aligned(32-bit) or 8-byte
+ * aligned(64-bit). This leaves the 2 least bits (LSBs) of the pointer
+ * always zero. So we can use these bits to encode the specific blocking
+ * type.
+ *
+ * Type encoding:
+ * 00 - Blocked on mutex        (BLOCKER_TYPE_MUTEX)
+ * 01 - Blocked on semaphore    (BLOCKER_TYPE_SEM)
+ * 10 - Blocked on rw-mutex     (BLOCKER_TYPE_RWMUTEX)
+ * 11 - Blocked on rw-semaphore (BLOCKER_TYPE_RWSEM)
+ */
+#define BLOCKER_TYPE_MUTEX      0x00UL
+#define BLOCKER_TYPE_SEM        0x01UL
+#define BLOCKER_TYPE_RWMUTEX    0x02UL
+#define BLOCKER_TYPE_RWSEM      0x03UL
+
+#define BLOCKER_TYPE_MASK       0x03UL
+
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+static inline void hung_task_set_blocker(void *lock, unsigned long type)
+{
+	unsigned long lock_ptr = (unsigned long)lock;
+
+	WARN_ON_ONCE(!lock_ptr);
+	WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK);
+	WARN_ON_ONCE(READ_ONCE(current->blocker));
+
+	WRITE_ONCE(current->blocker, lock_ptr | type);
+}
+
+static inline void hung_task_clear_blocker(void)
+{
+	WARN_ON_ONCE(!READ_ONCE(current->blocker));
+
+	WRITE_ONCE(current->blocker, 0UL);
+}
+
+static inline bool hung_task_blocker_is_type(unsigned long blocker,
+					  unsigned long type)
+{
+	WARN_ON_ONCE(!blocker);
+
+	return (blocker & BLOCKER_TYPE_MASK) == type;
+}
+
+static inline void *hung_task_blocker_to_lock(unsigned long blocker)
+{
+	WARN_ON_ONCE(!blocker);
+
+	return (void *)(blocker & ~BLOCKER_TYPE_MASK);
+}
+#else
+#define hung_task_set_blocker(lock, type)       do {} while (0)
+#define hung_task_clear_blocker()               do {} while (0)
+static inline bool hung_task_blocker_is_type(unsigned long blocker,
+					     unsigned long type)
+{
+	return false;
+}
+static inline void *hung_task_blocker_to_lock(unsigned long blocker)
+{
+	return NULL;
+}
+#endif
+
+#endif /* __LINUX_HUNG_TASK_H */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 1419d94c8e87..f27060dac499 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1218,7 +1218,7 @@ struct task_struct {
 #endif
 
 #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
-	struct mutex			*blocker_mutex;
+	unsigned long			blocker;
 #endif
 
 #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index ccd7217fcec1..f7fa832261c8 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -25,6 +25,10 @@
 
 #include <trace/events/sched.h>
 
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+#include <linux/hung_task.h>
+#endif
+
 /*
  * The number of tasks checked:
  */
@@ -98,16 +102,17 @@ static struct notifier_block panic_block = {
 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_blocker_is_type(blocker, BLOCKER_TYPE_MUTEX))
 		return;
 
-	owner = mutex_get_owner(lock);
+	owner = mutex_get_owner(
+		(struct mutex *)hung_task_blocker_to_lock(blocker));
+
 	if (unlikely(!owner)) {
 		pr_err("INFO: task %s:%d is blocked on a mutex, but the owner is not found.\n",
 			task->comm, task->pid);
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 6a543c204a14..642d6398e0dd 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -42,6 +42,10 @@
 # define MUTEX_WARN_ON(cond)
 #endif
 
+#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
+#include <linux/hung_task.h>
+#endif
+
 void
 __mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key)
 {
@@ -189,7 +193,7 @@ __mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
 		   struct list_head *list)
 {
 #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
-	WRITE_ONCE(current->blocker_mutex, lock);
+	hung_task_set_blocker(lock, BLOCKER_TYPE_MUTEX);
 #endif
 	debug_mutex_add_waiter(lock, waiter, current);
 
@@ -207,7 +211,7 @@ __mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter)
 
 	debug_mutex_remove_waiter(lock, waiter, current);
 #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
-	WRITE_ONCE(current->blocker_mutex, NULL);
+	hung_task_clear_blocker();
 #endif
 }
 
-- 

Thanks,
Lance

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] hung_task: show the blocker task if the task is hung on semaphore
  2025-03-06  6:05   ` Lance Yang
@ 2025-03-11  4:19     ` Masami Hiramatsu
  2025-03-11  4:48       ` Lance Yang
  0 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2025-03-11  4:19 UTC (permalink / raw)
  To: Lance Yang
  Cc: akpm, anna.schumaker, boqun.feng, joel.granados, kent.overstreet,
	leonylgao, linux-kernel, longman, mingo, mingzhe.yang, peterz,
	rostedt, senozhatsky, tfiga, will

On Thu,  6 Mar 2025 14:05:42 +0800
Lance Yang <ioworker0@gmail.com> wrote:

> Hi Masami,
> 
> Could you give me a quick feedback before sending out the v2?
> 
> It seems unnecessary to make 'blocker' a union. I replaced 'struct mutex
> *blocker_mutex' with 'unsigned long blocker', as only one blocker is active
> at a time, IIUC ;)
> 
> The blocker filed can store both the lock addrees and the lock type, with
> LSB used to encode the type as you 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 ;)

Hi Lance,

Sorry I missed to reply.

> 
> ---
>  include/linux/hung_task.h | 82 +++++++++++++++++++++++++++++++++++++++
>  include/linux/sched.h     |  2 +-
>  kernel/hung_task.c        | 15 ++++---
>  kernel/locking/mutex.c    |  8 +++-
>  4 files changed, 99 insertions(+), 8 deletions(-)
>  create mode 100644 include/linux/hung_task.h
> 
> diff --git a/include/linux/hung_task.h b/include/linux/hung_task.h
> new file mode 100644
> index 000000000000..569d2e579f36
> --- /dev/null
> +++ b/include/linux/hung_task.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Detect Hung Task: detecting tasks stuck in D state
> + *
> + * Copyright 2025 Lance Yang <ioworker0@gmail.com>
> + */
> +#ifndef __LINUX_HUNG_TASK_H
> +#define __LINUX_HUNG_TASK_H
> +
> +#include <linux/bug.h>
> +#include <linux/sched.h>
> +#include <linux/compiler.h>
> +
> +/*
> + * @blocker: Combines lock address and blocking type.
> + *
> + * Since lock pointers are at least 4-byte aligned(32-bit) or 8-byte
> + * aligned(64-bit). This leaves the 2 least bits (LSBs) of the pointer
> + * always zero. So we can use these bits to encode the specific blocking
> + * type.
> + *
> + * Type encoding:
> + * 00 - Blocked on mutex        (BLOCKER_TYPE_MUTEX)
> + * 01 - Blocked on semaphore    (BLOCKER_TYPE_SEM)
> + * 10 - Blocked on rw-mutex     (BLOCKER_TYPE_RWMUTEX)

                      ^ RT-Mutex?

> + * 11 - Blocked on rw-semaphore (BLOCKER_TYPE_RWSEM)
> + */
> +#define BLOCKER_TYPE_MUTEX      0x00UL
> +#define BLOCKER_TYPE_SEM        0x01UL
> +#define BLOCKER_TYPE_RWMUTEX    0x02UL

Ditto.

> +#define BLOCKER_TYPE_RWSEM      0x03UL
> +
> +#define BLOCKER_TYPE_MASK       0x03UL
> +
> +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> +static inline void hung_task_set_blocker(void *lock, unsigned long type)
> +{
> +	unsigned long lock_ptr = (unsigned long)lock;
> +
> +	WARN_ON_ONCE(!lock_ptr);
> +	WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK);

If it founds the lock_ptr & BLOCKER_TYPE_MASK is set, it should return
without writing anything. (WARN_ON_ONCE() is OK)

> +	WARN_ON_ONCE(READ_ONCE(current->blocker));
> +
> +	WRITE_ONCE(current->blocker, lock_ptr | type);
> +}
> +
> +static inline void hung_task_clear_blocker(void)
> +{
> +	WARN_ON_ONCE(!READ_ONCE(current->blocker));
> +
> +	WRITE_ONCE(current->blocker, 0UL);
> +}
> +
> +static inline bool hung_task_blocker_is_type(unsigned long blocker,
> +					  unsigned long type)
> +{
> +	WARN_ON_ONCE(!blocker);
> +
> +	return (blocker & BLOCKER_TYPE_MASK) == type;
> +}
> +
> +static inline void *hung_task_blocker_to_lock(unsigned long blocker)
> +{
> +	WARN_ON_ONCE(!blocker);
> +
> +	return (void *)(blocker & ~BLOCKER_TYPE_MASK);
> +}
> +#else
> +#define hung_task_set_blocker(lock, type)       do {} while (0)
> +#define hung_task_clear_blocker()               do {} while (0)
> +static inline bool hung_task_blocker_is_type(unsigned long blocker,
> +					     unsigned long type)
> +{
> +	return false;
> +}
> +static inline void *hung_task_blocker_to_lock(unsigned long blocker)
> +{
> +	return NULL;
> +}
> +#endif
> +
> +#endif /* __LINUX_HUNG_TASK_H */
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 1419d94c8e87..f27060dac499 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1218,7 +1218,7 @@ struct task_struct {
>  #endif
>  
>  #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> -	struct mutex			*blocker_mutex;
> +	unsigned long			blocker;
>  #endif
>  
>  #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index ccd7217fcec1..f7fa832261c8 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -25,6 +25,10 @@
>  
>  #include <trace/events/sched.h>
>  
> +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> +#include <linux/hung_task.h>
> +#endif
> +
>  /*
>   * The number of tasks checked:
>   */
> @@ -98,16 +102,17 @@ static struct notifier_block panic_block = {
>  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_blocker_is_type(blocker, BLOCKER_TYPE_MUTEX))
>  		return;
>  
> -	owner = mutex_get_owner(lock);
> +	owner = mutex_get_owner(
> +		(struct mutex *)hung_task_blocker_to_lock(blocker));

Yeah, others look good to me.

Thank you!


> +
>  	if (unlikely(!owner)) {
>  		pr_err("INFO: task %s:%d is blocked on a mutex, but the owner is not found.\n",
>  			task->comm, task->pid);
> diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> index 6a543c204a14..642d6398e0dd 100644
> --- a/kernel/locking/mutex.c
> +++ b/kernel/locking/mutex.c
> @@ -42,6 +42,10 @@
>  # define MUTEX_WARN_ON(cond)
>  #endif
>  
> +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> +#include <linux/hung_task.h>
> +#endif
> +
>  void
>  __mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key)
>  {
> @@ -189,7 +193,7 @@ __mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
>  		   struct list_head *list)
>  {
>  #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> -	WRITE_ONCE(current->blocker_mutex, lock);
> +	hung_task_set_blocker(lock, BLOCKER_TYPE_MUTEX);
>  #endif
>  	debug_mutex_add_waiter(lock, waiter, current);
>  
> @@ -207,7 +211,7 @@ __mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter)
>  
>  	debug_mutex_remove_waiter(lock, waiter, current);
>  #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> -	WRITE_ONCE(current->blocker_mutex, NULL);
> +	hung_task_clear_blocker();
>  #endif
>  }
>  
> -- 
> 
> Thanks,
> Lance


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] hung_task: show the blocker task if the task is hung on semaphore
  2025-03-11  4:19     ` Masami Hiramatsu
@ 2025-03-11  4:48       ` Lance Yang
  0 siblings, 0 replies; 6+ messages in thread
From: Lance Yang @ 2025-03-11  4:48 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: akpm, anna.schumaker, boqun.feng, joel.granados, kent.overstreet,
	leonylgao, linux-kernel, longman, mingo, mingzhe.yang, peterz,
	rostedt, senozhatsky, tfiga, will

On Tue, Mar 11, 2025 at 12:20 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Thu,  6 Mar 2025 14:05:42 +0800
> Lance Yang <ioworker0@gmail.com> wrote:
>
> > Hi Masami,
> >
> > Could you give me a quick feedback before sending out the v2?
> >
> > It seems unnecessary to make 'blocker' a union. I replaced 'struct mutex
> > *blocker_mutex' with 'unsigned long blocker', as only one blocker is active
> > at a time, IIUC ;)
> >
> > The blocker filed can store both the lock addrees and the lock type, with
> > LSB used to encode the type as you 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 ;)
>
> Hi Lance,
>
> Sorry I missed to reply.

No worries at all ;p

>
> >
> > ---
> >  include/linux/hung_task.h | 82 +++++++++++++++++++++++++++++++++++++++
> >  include/linux/sched.h     |  2 +-
> >  kernel/hung_task.c        | 15 ++++---
> >  kernel/locking/mutex.c    |  8 +++-
> >  4 files changed, 99 insertions(+), 8 deletions(-)
> >  create mode 100644 include/linux/hung_task.h
> >
> > diff --git a/include/linux/hung_task.h b/include/linux/hung_task.h
> > new file mode 100644
> > index 000000000000..569d2e579f36
> > --- /dev/null
> > +++ b/include/linux/hung_task.h
> > @@ -0,0 +1,82 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Detect Hung Task: detecting tasks stuck in D state
> > + *
> > + * Copyright 2025 Lance Yang <ioworker0@gmail.com>
> > + */
> > +#ifndef __LINUX_HUNG_TASK_H
> > +#define __LINUX_HUNG_TASK_H
> > +
> > +#include <linux/bug.h>
> > +#include <linux/sched.h>
> > +#include <linux/compiler.h>
> > +
> > +/*
> > + * @blocker: Combines lock address and blocking type.
> > + *
> > + * Since lock pointers are at least 4-byte aligned(32-bit) or 8-byte
> > + * aligned(64-bit). This leaves the 2 least bits (LSBs) of the pointer
> > + * always zero. So we can use these bits to encode the specific blocking
> > + * type.
> > + *
> > + * Type encoding:
> > + * 00 - Blocked on mutex        (BLOCKER_TYPE_MUTEX)
> > + * 01 - Blocked on semaphore    (BLOCKER_TYPE_SEM)
> > + * 10 - Blocked on rw-mutex     (BLOCKER_TYPE_RWMUTEX)
>
>                       ^ RT-Mutex?

Good catch! It's a typo, sorry ;(

>
> > + * 11 - Blocked on rw-semaphore (BLOCKER_TYPE_RWSEM)
> > + */
> > +#define BLOCKER_TYPE_MUTEX      0x00UL
> > +#define BLOCKER_TYPE_SEM        0x01UL
> > +#define BLOCKER_TYPE_RWMUTEX    0x02UL
>
> Ditto.

Yes. It should have been called BLOCKER_TYPE_RTMUTEX ;)

>
> > +#define BLOCKER_TYPE_RWSEM      0x03UL
> > +
> > +#define BLOCKER_TYPE_MASK       0x03UL
> > +
> > +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > +static inline void hung_task_set_blocker(void *lock, unsigned long type)
> > +{
> > +     unsigned long lock_ptr = (unsigned long)lock;
> > +
> > +     WARN_ON_ONCE(!lock_ptr);
> > +     WARN_ON_ONCE(lock_ptr & BLOCKER_TYPE_MASK);
>
> If it founds the lock_ptr & BLOCKER_TYPE_MASK is set, it should return
> without writing anything. (WARN_ON_ONCE() is OK)

Yeah, it makes sense to me. Will adjust as you suggested.

>
> > +     WARN_ON_ONCE(READ_ONCE(current->blocker));
> > +
> > +     WRITE_ONCE(current->blocker, lock_ptr | type);
> > +}
> > +
> > +static inline void hung_task_clear_blocker(void)
> > +{
> > +     WARN_ON_ONCE(!READ_ONCE(current->blocker));
> > +
> > +     WRITE_ONCE(current->blocker, 0UL);
> > +}
> > +
> > +static inline bool hung_task_blocker_is_type(unsigned long blocker,
> > +                                       unsigned long type)
> > +{
> > +     WARN_ON_ONCE(!blocker);
> > +
> > +     return (blocker & BLOCKER_TYPE_MASK) == type;
> > +}
> > +
> > +static inline void *hung_task_blocker_to_lock(unsigned long blocker)
> > +{
> > +     WARN_ON_ONCE(!blocker);
> > +
> > +     return (void *)(blocker & ~BLOCKER_TYPE_MASK);
> > +}
> > +#else
> > +#define hung_task_set_blocker(lock, type)       do {} while (0)
> > +#define hung_task_clear_blocker()               do {} while (0)
> > +static inline bool hung_task_blocker_is_type(unsigned long blocker,
> > +                                          unsigned long type)
> > +{
> > +     return false;
> > +}
> > +static inline void *hung_task_blocker_to_lock(unsigned long blocker)
> > +{
> > +     return NULL;
> > +}
> > +#endif
> > +
> > +#endif /* __LINUX_HUNG_TASK_H */
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 1419d94c8e87..f27060dac499 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -1218,7 +1218,7 @@ struct task_struct {
> >  #endif
> >
> >  #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > -     struct mutex                    *blocker_mutex;
> > +     unsigned long                   blocker;
> >  #endif
> >
> >  #ifdef CONFIG_DEBUG_ATOMIC_SLEEP
> > diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> > index ccd7217fcec1..f7fa832261c8 100644
> > --- a/kernel/hung_task.c
> > +++ b/kernel/hung_task.c
> > @@ -25,6 +25,10 @@
> >
> >  #include <trace/events/sched.h>
> >
> > +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > +#include <linux/hung_task.h>
> > +#endif
> > +
> >  /*
> >   * The number of tasks checked:
> >   */
> > @@ -98,16 +102,17 @@ static struct notifier_block panic_block = {
> >  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_blocker_is_type(blocker, BLOCKER_TYPE_MUTEX))
> >               return;
> >
> > -     owner = mutex_get_owner(lock);
> > +     owner = mutex_get_owner(
> > +             (struct mutex *)hung_task_blocker_to_lock(blocker));
>
> Yeah, others look good to me.

Thanks a lot for taking time to review!
Lance

>
> Thank you!
>
>
> > +
> >       if (unlikely(!owner)) {
> >               pr_err("INFO: task %s:%d is blocked on a mutex, but the owner is not found.\n",
> >                       task->comm, task->pid);
> > diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> > index 6a543c204a14..642d6398e0dd 100644
> > --- a/kernel/locking/mutex.c
> > +++ b/kernel/locking/mutex.c
> > @@ -42,6 +42,10 @@
> >  # define MUTEX_WARN_ON(cond)
> >  #endif
> >
> > +#ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > +#include <linux/hung_task.h>
> > +#endif
> > +
> >  void
> >  __mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key)
> >  {
> > @@ -189,7 +193,7 @@ __mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
> >                  struct list_head *list)
> >  {
> >  #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > -     WRITE_ONCE(current->blocker_mutex, lock);
> > +     hung_task_set_blocker(lock, BLOCKER_TYPE_MUTEX);
> >  #endif
> >       debug_mutex_add_waiter(lock, waiter, current);
> >
> > @@ -207,7 +211,7 @@ __mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter)
> >
> >       debug_mutex_remove_waiter(lock, waiter, current);
> >  #ifdef CONFIG_DETECT_HUNG_TASK_BLOCKER
> > -     WRITE_ONCE(current->blocker_mutex, NULL);
> > +     hung_task_clear_blocker();
> >  #endif
> >  }
> >
> > --
> >
> > Thanks,
> > Lance
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-03-11  4:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-01  5:51 [PATCH 1/1] hung_task: show the blocker task if the task is hung on semaphore Lance Yang
2025-03-03  6:29 ` Masami Hiramatsu
2025-03-03 10:52   ` Lance Yang
2025-03-06  6:05   ` Lance Yang
2025-03-11  4:19     ` Masami Hiramatsu
2025-03-11  4:48       ` Lance Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox