kernel/reboot: enhance dmesg logging for system restart

[PATCH] kernel/reboot: enhance dmesg logging for system restart

Posted by Faiyaz Mohammed 1 year, 8 months ago

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'

Signed-off-by: Faiyaz Mohammed <quic_faiyazm@quicinc.com>
---
 kernel/reboot.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/reboot.c b/kernel/reboot.c
index f05dbde2c93f..91a4a1428eb9 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -274,14 +274,17 @@ static void do_kernel_restart_prepare(void)
  */
 void kernel_restart(char *cmd)
 {
+	char comm[sizeof(current->comm)];
+
+	get_task_comm(comm, current);
 	kernel_restart_prepare(cmd);
 	do_kernel_restart_prepare();
 	migrate_to_reboot_cpu();
 	syscore_shutdown();
 	if (!cmd)
-		pr_emerg("Restarting system\n");
+		pr_emerg("PID: %d Comm: %s Restarting system\n", current->pid, comm);
 	else
-		pr_emerg("Restarting system with command '%s'\n", cmd);
+		pr_emerg("PID: %d Comm: %s Restarting system with command '%s'\n", current->pid, comm, cmd);
 	kmsg_dump(KMSG_DUMP_SHUTDOWN);
 	machine_restart(cmd);
 }
-- 
2.25.1

Re: [PATCH] kernel/reboot: enhance dmesg logging for system restart

Posted by 이동민 1 year, 8 months ago

On Fri, Jun 7, 2024 at 7:59 PM Faiyaz Mohammed <quic_faiyazm@quicinc.com> wrote:
>
> 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 is
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/

Re: [PATCH] kernel/reboot: enhance dmesg logging for system restart

Posted by Faiyaz Mohammed 1 year, 7 months ago

Sorry for the delayed response.

On 6/7/2024 7:35 PM, 이동민 wrote:
> On Fri, Jun 7, 2024 at 7:59 PM Faiyaz Mohammed <quic_faiyazm@quicinc.com> wrote:
>>
>> 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.
> 
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?

> If reboot is called by user space via reboot system call, reboot is
> 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().
> 
Agreed 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.

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

Re: [PATCH] kernel/reboot: enhance dmesg logging for system restart

Posted by Dongmin Lee 1 year, 7 months ago

On Wed, Jun 19, 2024 at 8:09 PM Faiyaz Mohammed
<quic_faiyazm@quicinc.com> wrote:
> Agreed 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.

Your situation is where system call tracing is required. Tracing
itself (via printing out kernel backtrace and/or comm and name) is
actually not what system calls should support regardless of debugging
level and/or kconfig option.

As I mentioned in earlier mail, you might use gdb debugger for the
purpose, or you might utilize SystemTap or eBPF to trace system calls
(including reboot call), actually these are for you to do system call
tracing rather than unnecessarily modifying system call's behavior.

You might want to refer to: https://lwn.net/Articles/852112/

Sincerely,
Dongmin Lee
https://ldmsys.net/