On Fri, Jun 7, 2024 at 7:59 PM Faiyaz Mohammed <quic_faiyazm@xxxxxxxxxxx> wrote:Agreed, It's not critical to print the process name which trigger reboot with pr_emerg(), so should it be OK if we add additional logging with reduced log level?
It is useful to add the PID and Comm information along with command info.
Currently, when system reboot kernel logs don not print PID and Comm:
reboot: Restarting system with command 'reboot,scheduled_reboot'
reboot: Restarting system with command 'RescueParty'
reboot: Restarting system with command 'bootloader'
reboot: Restarting system with command 'recovery'
reboot: Restarting system with command 'userrequested,recovery’
For Example after adding PID and Comm:
reboot: PID: 1 Comm: init Restarting system with command 'shell'
reboot: PID: 1 Comm: init Restarting system with command 'bootloader'
Printing out PID and COMM information might be useful for getting
which task is triggered system reboot. However, It's never a critical
information that deserves printed with pr_emerg() to whoever want the
system to be rebooted, unless the kernel is in a problematic
situation.
If reboot is called by user space via reboot system call, reboot isAgreed in kernel view user's space reboot is intended but if system is silently rebooting due to any user process(vendor specific user process or even a OS specific user deamon) meeting any error condition then a developer debugging the system need to know which user process is issuing reboot system call to debug further, so in humble opinion it is helpful for system debug perspective.
never a problematic situation because it's user's intend in the
kernel's view. Other kernel codes which invokes involuntary restart
such as temperature overheat (drivers/memory/emif.c:622), already
prints out the situation before invoking system_reboot(), hence, there
is no reason to print out who called system_reboot().
Again, system reboot is not kernel panic, oops nor bug. If your intend
is to debug the reboot handler's behavior more easily, just set a
breakpoint for kernel_restart() function with gdb.
--
Best Regards,
Dongmin Lee
https://ldmsys.net/