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 | 37 +++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index d2254c91450b..5902573200c0 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -223,6 +223,32 @@ 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;
+	const char *coredump_msg = "      Blocked by coredump.";
+	const char *disable_msg =
+		"\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\""
+		" disables this message.";
+
+	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("%s\n", coredump_msg);
+	pr_err("%s\n", disable_msg);
+}
+
 static void check_hung_task(struct task_struct *t, unsigned long timeout,
 		unsigned long prev_detect_count)
 {
@@ -252,16 +278,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

On Mon 2025-12-15 22:00:35, 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 | 37 +++++++++++++++++++++++++++----------
>  1 file changed, 27 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index d2254c91450b..5902573200c0 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -223,6 +223,32 @@ 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;

It makes sense to make the computation separately.

> +	const char *coredump_msg = "      Blocked by coredump.";

Honestly, I do not see any advantage in storing the string into a
variable and passing it via %s. It does not help with readability
because the reader has to lookup the variable definition. And it
is less effective code (not a big deal but...).

Instead, I would add "\n". It will allow to show the message
immediately on consoles. Without the trailing "\n", the message
is not finalized because it might still get extended by pr_cont().

> +	const char *disable_msg =
> +		"\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\""
> +		" disables this message.";

Same here. I would avoid the %s and add the trailing '\n'.

Also the message should be on a single line. It makes it easier
to find it via "git grep". ./scripts/checkpatch.pl even complain
about it:

WARNING: quoted string split across lines
#37: FILE: kernel/hung_task.c:239:
+		"\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\""
+		" disables this message.";


> +	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("%s\n", coredump_msg);
> +	pr_err("%s\n", disable_msg);
> +}

Otherwise, I like the change.

Best Regards,
Petr

On Wed, Dec 17, 2025 at 10:39:07AM +0100, Petr Mladek wrote:
> It makes sense to make the computation separately.

Hi Petr,

Thank you for the detailed review and the constructive feedback.

> > +	const char *coredump_msg = "      Blocked by coredump.";
> 
> Honestly, I do not see any advantage in storing the string into a
> variable and passing it via %s. It does not help with readability
> because the reader has to lookup the variable definition. And it
> is less effective code (not a big deal but...).
> 
> Instead, I would add "\n". It will allow to show the message
> immediately on consoles. Without the trailing "\n", the message
> is not finalized because it might still get extended by pr_cont().
> 
> > +	const char *disable_msg =
> > +		"\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\""
> > +		" disables this message.";
> 
> Same here. I would avoid the %s and add the trailing '\n'.
> 
> Also the message should be on a single line. It makes it easier
> to find it via "git grep". ./scripts/checkpatch.pl even complain
> about it:
> 
> WARNING: quoted string split across lines
> #37: FILE: kernel/hung_task.c:239:
> +		"\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\""
> +		" disables this message.";
> 
> 
> > +	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("%s\n", coredump_msg);
> > +	pr_err("%s\n", disable_msg);
> > +}
> 
> Otherwise, I like the change.

I entirely agree with your observations. Upon reflection, using local
variables for these strings was indeed an unnecessary abstraction that
hindered rather than helped readability.


Kind regards,
-- 
Aaron Tomlin