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@xxxxxxxxxxxxxx/ 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
