linux-kernel - Re: [PATCHv12 4/4] watchdog/softlockup: report the most frequent interrupts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6109a3e3-ca88-4a4d-86c5-c4eb0d7f6f9c@linux.alibaba.com>
Date: Mon, 25 Mar 2024 17:47:58 +0800
From: Bitao Hu <yaoma@...ux.alibaba.com>
To: Thomas Gleixner <tglx@...utronix.de>, dianders@...omium.org,
 liusong@...ux.alibaba.com, akpm@...ux-foundation.org, pmladek@...e.com,
 kernelfans@...il.com, deller@....de, npiggin@...il.com,
 tsbogend@...ha.franken.de, James.Bottomley@...senPartnership.com,
 jan.kiszka@...mens.com
Cc: linux-kernel@...r.kernel.org, linux-mips@...r.kernel.org,
 linux-parisc@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
 yaoma@...ux.alibaba.com
Subject: Re: [PATCHv12 4/4] watchdog/softlockup: report the most frequent
 interrupts

Hi, Thomas

On 2024/3/24 04:43, Thomas Gleixner wrote:
> On Wed, Mar 06 2024 at 20:52, Bitao Hu wrote:
>> +	if (__this_cpu_read(snapshot_taken)) {
>> +		for_each_active_irq(i) {
>> +			count = kstat_get_irq_since_snapshot(i);
>> +			tabulate_irq_count(irq_counts_sorted, i, count, NUM_HARDIRQ_REPORT);
>> +		}
>> +
>> +		/*
>> +		 * We do not want the "watchdog: " prefix on every line,
>> +		 * hence we use "printk" instead of "pr_crit".
>> +		 */
> 
> You are not providing any justification why the prefix is not
> wanted. Just saying 'We do not want' does not cut it and who is 'We'. I
> certainly not.
> 
> I really disagree because the prefixes are very useful for searching log
> files. So not having it makes it harder to filter out for no reason.
> 

Regarding the use of printk() instead of pr_crit(), I have had a
discussion with Liu Song and Douglas in PATCHv1:
https://lore.kernel.org/all/CAD=FV=WEEQeKX=ec3Gr-8CKs2K0MaWN3V0-0yOsuret0qcB_AA@mail.gmail.com/

Please allow me to elaborate on my reasoning. The purpose of the
report_cpu_status() function I implemented is similar to that of
print_modules(), show_regs(), and dump_stack(). These functions are
designed to assist in analyzing the causes of a soft lockup, rather
than to report that a soft lockup has occurred. Therefore, I think
that adding the "watchdog: " prefix to every line is redundant and
not concise. Besides, the existing pr_emerg() in the watchdog.c file
is already sufficient for searching useful information in the logs.
The information I added, along with the call tree and other data, is
located near the line with the "watchdog: " prefix.

Are the two reasons I've provided reasonable?

Best Regards,

	Bitao Hu