[PATCHv12 4/4] watchdog/softlockup: report the most frequent interrupts
Bitao Hu
yaoma at linux.alibaba.com
Mon Mar 25 20:47:58 AEDT 2024
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
More information about the Linuxppc-dev
mailing list