Re: [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason

2020-05-23 Thread Michael Ellerman 
Kees Cook  writes:
> To turn the KMSG_DUMP_* reasons into a more ordered list, collapse
> the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into
> KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully
> distinguish between them, so there's no need to, as discussed here:
> https://lore.kernel.org/lkml/ca+ck2bapv5u1ih5y9t5funtyximtfctdyxjcpuyjoyhnokr...@mail.gmail.com/
>
> Signed-off-by: Kees Cook 
> ---
>  arch/powerpc/kernel/nvram_64.c | 4 +---
>  fs/pstore/platform.c   | 8 ++--
>  include/linux/kmsg_dump.h  | 4 +---
>  kernel/reboot.c| 6 +++---
>  4 files changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> index fb4f61096613..0cd1c88bfc8b 100644
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -655,9 +655,7 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
>   int rc = -1;
>  
>   switch (reason) {
> - case KMSG_DUMP_RESTART:
> - case KMSG_DUMP_HALT:
> - case KMSG_DUMP_POWEROFF:
> + case KMSG_DUMP_SHUTDOWN:
>   /* These are almost always orderly shutdowns. */
>   return;
>   case KMSG_DUMP_OOPS:

Acked-by: Michael Ellerman  (powerpc)

cheers

Re: [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason

2020-05-22 Thread Petr Mladek 
On Fri 2020-05-15 11:44:29, Kees Cook wrote:
> To turn the KMSG_DUMP_* reasons into a more ordered list, collapse
> the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into
> KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully
> distinguish between them, so there's no need to, as discussed here:
> https://lore.kernel.org/lkml/ca+ck2bapv5u1ih5y9t5funtyximtfctdyxjcpuyjoyhnokr...@mail.gmail.com/
> 
> Signed-off-by: Kees Cook 

Looks good to me:

Reviewed-by: Petr Mladek 

Best Regards,
Petr

Re: [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason

2020-05-15 Thread Pavel Tatashin 
On Fri, May 15, 2020 at 2:44 PM Kees Cook  wrote:
>
> To turn the KMSG_DUMP_* reasons into a more ordered list, collapse
> the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into
> KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully
> distinguish between them, so there's no need to, as discussed here:
> https://lore.kernel.org/lkml/ca+ck2bapv5u1ih5y9t5funtyximtfctdyxjcpuyjoyhnokr...@mail.gmail.com/
>
> Signed-off-by: Kees Cook 

Maybe it makes sense to mention in the commit log that for all three
merged cases there is a pr_emerg() message logged right before the
kmsg_dump(), thus the reason is distinguishable from the dmesg log
itself.

Reviewed-by: Pavel Tatashin

[PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason

2020-05-15 Thread Kees Cook 
To turn the KMSG_DUMP_* reasons into a more ordered list, collapse
the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into
KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully
distinguish between them, so there's no need to, as discussed here:
https://lore.kernel.org/lkml/ca+ck2bapv5u1ih5y9t5funtyximtfctdyxjcpuyjoyhnokr...@mail.gmail.com/

Signed-off-by: Kees Cook 
---
 arch/powerpc/kernel/nvram_64.c | 4 +---
 fs/pstore/platform.c   | 8 ++--
 include/linux/kmsg_dump.h  | 4 +---
 kernel/reboot.c| 6 +++---
 4 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index fb4f61096613..0cd1c88bfc8b 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -655,9 +655,7 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
int rc = -1;
 
switch (reason) {
-   case KMSG_DUMP_RESTART:
-   case KMSG_DUMP_HALT:
-   case KMSG_DUMP_POWEROFF:
+   case KMSG_DUMP_SHUTDOWN:
/* These are almost always orderly shutdowns. */
return;
case KMSG_DUMP_OOPS:
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 072440457c08..90d74ebaa70a 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -144,12 +144,8 @@ static const char *get_reason_str(enum kmsg_dump_reason 
reason)
return "Oops";
case KMSG_DUMP_EMERG:
return "Emergency";
-   case KMSG_DUMP_RESTART:
-   return "Restart";
-   case KMSG_DUMP_HALT:
-   return "Halt";
-   case KMSG_DUMP_POWEROFF:
-   return "Poweroff";
+   case KMSG_DUMP_SHUTDOWN:
+   return "Shutdown";
default:
return "Unknown";
}
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 2e7a1e032c71..3f82b5cb2d82 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -25,9 +25,7 @@ enum kmsg_dump_reason {
KMSG_DUMP_PANIC,
KMSG_DUMP_OOPS,
KMSG_DUMP_EMERG,
-   KMSG_DUMP_RESTART,
-   KMSG_DUMP_HALT,
-   KMSG_DUMP_POWEROFF,
+   KMSG_DUMP_SHUTDOWN,
 };
 
 /**
diff --git a/kernel/reboot.c b/kernel/reboot.c
index c4d472b7f1b4..491f1347bf43 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -250,7 +250,7 @@ void kernel_restart(char *cmd)
pr_emerg("Restarting system\n");
else
pr_emerg("Restarting system with command '%s'\n", cmd);
-   kmsg_dump(KMSG_DUMP_RESTART);
+   kmsg_dump(KMSG_DUMP_SHUTDOWN);
machine_restart(cmd);
 }
 EXPORT_SYMBOL_GPL(kernel_restart);
@@ -274,7 +274,7 @@ void kernel_halt(void)
migrate_to_reboot_cpu();
syscore_shutdown();
pr_emerg("System halted\n");
-   kmsg_dump(KMSG_DUMP_HALT);
+   kmsg_dump(KMSG_DUMP_SHUTDOWN);
machine_halt();
 }
 EXPORT_SYMBOL_GPL(kernel_halt);
@@ -292,7 +292,7 @@ void kernel_power_off(void)
migrate_to_reboot_cpu();
syscore_shutdown();
pr_emerg("Power down\n");
-   kmsg_dump(KMSG_DUMP_POWEROFF);
+   kmsg_dump(KMSG_DUMP_SHUTDOWN);
machine_power_off();
 }
 EXPORT_SYMBOL_GPL(kernel_power_off);
-- 
2.20.1