[v5 PATCH 1/2] hung_task: Introduce helper for hung task warning

Aaron Tomlin posted 2 patches 1 month, 1 week ago
There is a newer version of this series
[v5 PATCH 1/2] hung_task: Introduce helper for hung task warning
Posted by Aaron Tomlin 1 month, 1 week ago
Consolidate the multi-line console output block for reporting a hung
task into a new helper function, hung_task_diagnostics(). This improves
readability in the main check_hung_task() loop and makes the diagnostic
output structure easier to maintain and update in the future.

Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 kernel/hung_task.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index d2254c91450b..00c3296fd692 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -223,6 +223,28 @@ static inline void debug_show_blocker(struct task_struct *task, unsigned long ti
 }
 #endif
 
+/**
+ * hung_task_diagnostics - Print structured diagnostic info for a hung task.
+ * @t: Pointer to the detected hung task.
+ *
+ * This function consolidates the printing of core diagnostic information
+ * for a task found to be blocked.
+ */
+static inline void hung_task_diagnostics(struct task_struct *t)
+{
+	unsigned long blocked_secs = (jiffies - t->last_switch_time) / HZ;
+
+	pr_err("INFO: task %s:%d blocked for more than %ld seconds.\n",
+	       t->comm, t->pid, blocked_secs);
+	pr_err("      %s %s %.*s\n",
+	       print_tainted(), init_utsname()->release,
+	       (int)strcspn(init_utsname()->version, " "),
+	       init_utsname()->version);
+	if (t->flags & PF_POSTCOREDUMP)
+		pr_err("      Blocked by coredump.\n");
+	pr_err("\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\" disables this message.\n");
+}
+
 static void check_hung_task(struct task_struct *t, unsigned long timeout,
 		unsigned long prev_detect_count)
 {
@@ -252,16 +274,7 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout,
 	if (sysctl_hung_task_warnings || hung_task_call_panic) {
 		if (sysctl_hung_task_warnings > 0)
 			sysctl_hung_task_warnings--;
-		pr_err("INFO: task %s:%d blocked for more than %ld seconds.\n",
-		       t->comm, t->pid, (jiffies - t->last_switch_time) / HZ);
-		pr_err("      %s %s %.*s\n",
-			print_tainted(), init_utsname()->release,
-			(int)strcspn(init_utsname()->version, " "),
-			init_utsname()->version);
-		if (t->flags & PF_POSTCOREDUMP)
-			pr_err("      Blocked by coredump.\n");
-		pr_err("\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\""
-			" disables this message.\n");
+		hung_task_diagnostics(t);
 		sched_show_task(t);
 		debug_show_blocker(t, timeout);
 
-- 
2.51.0
Re: [v5 PATCH 1/2] hung_task: Introduce helper for hung task warning
Posted by Lance Yang 1 month, 1 week ago

On 2025/12/31 08:41, Aaron Tomlin wrote:
> Consolidate the multi-line console output block for reporting a hung
> task into a new helper function, hung_task_diagnostics(). This improves
> readability in the main check_hung_task() loop and makes the diagnostic
> output structure easier to maintain and update in the future.
> 
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
>   kernel/hung_task.c | 33 +++++++++++++++++++++++----------
>   1 file changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index d2254c91450b..00c3296fd692 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -223,6 +223,28 @@ static inline void debug_show_blocker(struct task_struct *task, unsigned long ti
>   }
>   #endif
>   
> +/**
> + * hung_task_diagnostics - Print structured diagnostic info for a hung task.
> + * @t: Pointer to the detected hung task.
> + *
> + * This function consolidates the printing of core diagnostic information
> + * for a task found to be blocked.
> + */
> +static inline void hung_task_diagnostics(struct task_struct *t)
> +{
> +	unsigned long blocked_secs = (jiffies - t->last_switch_time) / HZ;
> +
> +	pr_err("INFO: task %s:%d blocked for more than %ld seconds.\n",
> +	       t->comm, t->pid, blocked_secs);
> +	pr_err("      %s %s %.*s\n",
> +	       print_tainted(), init_utsname()->release,
> +	       (int)strcspn(init_utsname()->version, " "),
> +	       init_utsname()->version);
> +	if (t->flags & PF_POSTCOREDUMP)
> +		pr_err("      Blocked by coredump.\n");
> +	pr_err("\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\" disables this message.\n");
> +}
> +
>   static void check_hung_task(struct task_struct *t, unsigned long timeout,
>   		unsigned long prev_detect_count)
>   {
> @@ -252,16 +274,7 @@ static void check_hung_task(struct task_struct *t, unsigned long timeout,
>   	if (sysctl_hung_task_warnings || hung_task_call_panic) {
>   		if (sysctl_hung_task_warnings > 0)
>   			sysctl_hung_task_warnings--;
> -		pr_err("INFO: task %s:%d blocked for more than %ld seconds.\n",
> -		       t->comm, t->pid, (jiffies - t->last_switch_time) / HZ);
> -		pr_err("      %s %s %.*s\n",
> -			print_tainted(), init_utsname()->release,
> -			(int)strcspn(init_utsname()->version, " "),
> -			init_utsname()->version);
> -		if (t->flags & PF_POSTCOREDUMP)
> -			pr_err("      Blocked by coredump.\n");
> -		pr_err("\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\""
> -			" disables this message.\n");
> +		hung_task_diagnostics(t);

I am wondering whether we should leave that code as-is to
avoid unnecessary churn ...

That code was not particularly complex or duplicated :)

>   		sched_show_task(t);
>   		debug_show_blocker(t, timeout);
>
Re: [v5 PATCH 1/2] hung_task: Introduce helper for hung task warning
Posted by Aaron Tomlin 1 month, 1 week ago
On Thu, Jan 01, 2026 at 05:49:59PM +0800, Lance Yang wrote:
> I am wondering whether we should leave that code as-is to
> avoid unnecessary churn ...
> 
> That code was not particularly complex or duplicated :)

Hi Lance,

While I agree the current logic is simple, separating the verbose reporting
from the detection loop significantly improves the readability of
check_hung_task(). This refactoring introduces no runtime overhead
(static inline) while providing a cleaner, encapsulated structure for any
future diagnostic enhancements.


Kind regards,
-- 
Aaron Tomlin
Re: [v5 PATCH 1/2] hung_task: Introduce helper for hung task warning
Posted by Lance Yang 1 month, 1 week ago

On 2026/1/2 03:28, Aaron Tomlin wrote:
> On Thu, Jan 01, 2026 at 05:49:59PM +0800, Lance Yang wrote:
>> I am wondering whether we should leave that code as-is to
>> avoid unnecessary churn ...
>>
>> That code was not particularly complex or duplicated :)
> 
> Hi Lance,
> 
> While I agree the current logic is simple, separating the verbose reporting
> from the detection loop significantly improves the readability of
> check_hung_task(). This refactoring introduces no runtime overhead
> (static inline) while providing a cleaner, encapsulated structure for any
> future diagnostic enhancements.

For a single-use diagnostic block, I still think this refactoring
does not add much practical value ...

Let's leave the code alone - it's probably not worth the churn :)
Re: [v5 PATCH 1/2] hung_task: Introduce helper for hung task warning
Posted by Aaron Tomlin 1 month ago
On Fri, Jan 02, 2026 at 11:40:24AM +0800, Lance Yang wrote:
> For a single-use diagnostic block, I still think this refactoring
> does not add much practical value ...
> 
> Let's leave the code alone - it's probably not worth the churn :)

Hi Lance,

Fair point. While I remain of the view that the encapsulation offers a bit
more clarity, I certainly appreciate your point regarding unnecessary churn
for a single-use block.

I shall drop this patch from the series in the next iteration to keep
things concise.


Best regards,
-- 
Aaron Tomlin