Re: [PATCH 1/2] hung_task: Consolidate hung task warning into an atomic log block

[Lance Yang]

On 2025/12/9 12:12, Aaron Tomlin wrote:
> Consolidate the multi-line console output in check_hung_task() into a new
> helper function, hung_task_diagnostics().
>
> This patch ensures the entire diagnostic block (task info, kernel
> version, and sysctl advice) is logged to the ring buffer via a single
> pr_err() call. This is critical in a concurrent environment to prevent
> message lines from interleaving with other CPU activity, thus
> maintaining contextual integrity of the warning message.
>
> Signed-off-by: Aaron Tomlin <atomlin@xxxxxxxxxxx>
> ---
>   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..d5109a0994c5 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: The struct task_struct of the detected hung task.
> + *
> + * This function consolidates the printing of core diagnostic information
> + * for a task found to be blocked. This approach ensures atomic logging
> + * of the multi-line message block, preventing interleaving by other
> + * console activity, thus maintaining contextual clarity.
> + */
> +static 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"
> +	       "      %s %s %.*s\n"
> +	       "%s\n"

"%s\n" forces an unconditional newline, causing a spurious blank line when
the flag isn't set, right?

> +	       "\"echo 0 > /proc/sys/kernel/hung_task_timeout_secs\""
> +	       " disables this message.\n",
> +	       t->comm, t->pid, blocked_secs,
> +	       print_tainted(), init_utsname()->release,
> +	       (int)strcspn(init_utsname()->version, " "),
> +	       init_utsname()->version,
> +	       (t->flags & PF_POSTCOREDUMP) ?
> +	       "      Blocked by coredump." : "");
> +}
> +
>   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);

Since there's only one caller, why not just open-code it ...

TBH, jamming everything into a single pr_err() call doesn't look any
cleaner to me than the original code :(

>   		sched_show_task(t);
>   		debug_show_blocker(t, timeout);
>   

Cheers,
Lance