All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Bradley Morgan <include@grrlz.net>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Douglas Anderson <dianders@chromium.org>,
	Mayank Rungta <mrungta@google.com>, Tejun Heo <tj@kernel.org>,
	Feng Tang <feng.tang@linux.alibaba.com>,
	ZhenguoYao <yaozhenguo1@gmail.com>,
	Shengming Hu <hu.shengming@zte.com.cn>,
	Joel Granados <joel.granados@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] watchdog: avoid extra sys_info dumps for all_bt
Date: Tue, 23 Jun 2026 14:37:00 +0200	[thread overview]
Message-ID: <ajp97LUT8rsWie74@pathway.suse.cz> (raw)
In-Reply-To: <20260620220140.15479-1-include@grrlz.net>

On Sat 2026-06-20 22:01:40, Bradley Morgan wrote:
> The watchdog handles SYS_INFO_ALL_BT itself. When that is the only
> watchdog specific bit, sys_info(0) falls back to kernel_sys_info.

Great catch!

> Skip sys_info() for that case.
> 
> Fixes: a9af76a78760 ("watchdog: add sys_info sysctls to dump sys info on system lockup")
> Signed-off-by: Bradley Morgan <include@grrlz.net>

> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -54,6 +54,16 @@ static int __read_mostly watchdog_hardlockup_available;
>  struct cpumask watchdog_cpumask __read_mostly;
>  unsigned long *watchdog_cpumask_bits = cpumask_bits(&watchdog_cpumask);
>  

Similar problem exists also in kernel/panic.c. It would make
sense to put this variant into lib/sys_info.c and use a generic name,
e.g. sys_info_without_all_bt().

> +static void watchdog_sys_info(unsigned long si_mask)
> +{
> +	unsigned long dump_mask = si_mask & ~SYS_INFO_ALL_BT;
> +

This looks like an optimization. The real reason is not obvious.
It would deserve a comment, something like:

	/*
	 * Do not call sys_info() when the caller context required
	 * only backtraces from all CPUs. Otherwise, sys_info()
	 * would fall back to the generic "kernel_si_mask".
	 */
> +	if (si_mask && !dump_mask)
> +		return;
> +
> +	sys_info(dump_mask);
> +}

Alternative solution would be to remove the custom
trigger_all*_cpu_backtrace() calls from both watchdog and panic
code and rely on sys_info() API. But it is not easy, especially
in panic().

Otherwise, the changes make sense.

Best Regards,
Petr

      parent reply	other threads:[~2026-06-23 12:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-20 22:01 [PATCH 1/1] watchdog: avoid extra sys_info dumps for all_bt Bradley Morgan
2026-06-22  3:23 ` Feng Tang
2026-06-22  6:30   ` Bradley Morgan
2026-06-22  7:37     ` Feng Tang
2026-06-22 14:51       ` Bradley Morgan
2026-06-23 12:37 ` Petr Mladek [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajp97LUT8rsWie74@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=dianders@chromium.org \
    --cc=feng.tang@linux.alibaba.com \
    --cc=hu.shengming@zte.com.cn \
    --cc=include@grrlz.net \
    --cc=joel.granados@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mrungta@google.com \
    --cc=tj@kernel.org \
    --cc=yaozhenguo1@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.