| Message ID | 20220509233235.995021-8-dmitry.osipenko@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce power-off+restart call chain API | expand |
Hi Dmitry, On Tue, May 10, 2022 at 1:33 AM Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > Add kernel_can_power_off() helper that replaces open-coded checks of > the global pm_power_off variable. This is a necessary step towards > supporting chained power-off handlers. > > Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> Thanks for your patch, which is now commit 0e2110d2e910e44c ("kernel/reboot: Add kernel_can_power_off()") in pm/linux-next. This causes the "poweroff" command (Debian nfsroot) to no longer cleanly halt the system on arm32 systems, but fail with a panic instead: -reboot: System halted +reboot: Power down +Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000 +CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 5.18.0-rc7-shmobile-00007-g0e2110d2e910 #1274 +Hardware name: Generic R-Car Gen2 (Flattened Device Tree) + unwind_backtrace from show_stack+0x10/0x14 + show_stack from dump_stack_lvl+0x40/0x4c + dump_stack_lvl from panic+0xf4/0x330 + panic from do_exit+0x1c8/0x8e4 + do_exit from __do_sys_reboot+0x174/0x1fc + __do_sys_reboot from ret_fast_syscall+0x0/0x54 +Exception stack(0xf0815fa8 to 0xf0815ff0) +5fa0: 004e6954 00000000 fee1dead 28121969 4321fedc f0d94600 +5fc0: 004e6954 00000000 00000000 00000058 befa0c78 00000000 befa0c10 004e56f8 +5fe0: 00000058 befa0b6c b6ec8d45 b6e4a746 +---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000 ]--- On arm64, "poweroff" causes a clean "reboot: Power down" before/after. On both arm32 and arm64, the same handlers are registered: - SYS_OFF_MODE_POWER_OFF_PREPARE: legacy_pm_power_off_prepare - SYS_OFF_MODE_POWER_OFF: legacy_pm_power_off On both arm32 and arm64, legacy_pm_power_off_prepare() is called. On both arm32 and arm64, legacy_pm_power_off() does not seem to be called. On arm32, both pm_power_off_prepare and pm_power_off are NULL. On arm64, pm_power_off_prepare is NULL, and pm_power_off is psci_sys_poweroff. Do you have a clue? Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 5/24/22 16:14, Geert Uytterhoeven wrote: > Hi Dmitry, > > On Tue, May 10, 2022 at 1:33 AM Dmitry Osipenko > <dmitry.osipenko@collabora.com> wrote: >> Add kernel_can_power_off() helper that replaces open-coded checks of >> the global pm_power_off variable. This is a necessary step towards >> supporting chained power-off handlers. >> >> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > > Thanks for your patch, which is now commit 0e2110d2e910e44c > ("kernel/reboot: Add kernel_can_power_off()") in pm/linux-next. > > This causes the "poweroff" command (Debian nfsroot) to no longer > cleanly halt the system on arm32 systems, but fail with a panic > instead: > > -reboot: System halted > +reboot: Power down > +Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000 > +CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted > 5.18.0-rc7-shmobile-00007-g0e2110d2e910 #1274 > +Hardware name: Generic R-Car Gen2 (Flattened Device Tree) > + unwind_backtrace from show_stack+0x10/0x14 > + show_stack from dump_stack_lvl+0x40/0x4c > + dump_stack_lvl from panic+0xf4/0x330 > + panic from do_exit+0x1c8/0x8e4 > + do_exit from __do_sys_reboot+0x174/0x1fc > + __do_sys_reboot from ret_fast_syscall+0x0/0x54 > +Exception stack(0xf0815fa8 to 0xf0815ff0) > +5fa0: 004e6954 00000000 fee1dead 28121969 4321fedc f0d94600 > +5fc0: 004e6954 00000000 00000000 00000058 befa0c78 00000000 befa0c10 004e56f8 > +5fe0: 00000058 befa0b6c b6ec8d45 b6e4a746 > +---[ end Kernel panic - not syncing: Attempted to kill init! > exitcode=0x00000000 ]--- > > On arm64, "poweroff" causes a clean "reboot: Power down" before/after. > > On both arm32 and arm64, the same handlers are registered: > - SYS_OFF_MODE_POWER_OFF_PREPARE: legacy_pm_power_off_prepare > - SYS_OFF_MODE_POWER_OFF: legacy_pm_power_off > > On both arm32 and arm64, legacy_pm_power_off_prepare() is called. > On both arm32 and arm64, legacy_pm_power_off() does not seem to > be called. > > On arm32, both pm_power_off_prepare and pm_power_off are NULL. > On arm64, pm_power_off_prepare is NULL, and > pm_power_off is psci_sys_poweroff. > > Do you have a clue? > Thanks! Thank you, Geert! I see the problem, the kernel_can_power_off() checks whether power-off handler is registered, but it's always registered because legacy_pm_power_off is registered unconditionally. So it causes trouble for platforms that don't have power-off handler installed at all. All platforms that I tested have a power-off handler, so now wonder that I didn't notice this before. This change should fix the problem, please give it a try: --- 8< --- diff --git a/kernel/reboot.c b/kernel/reboot.c index 0bdc64ecf4f6..2d55b8bdb444 100644 --- a/kernel/reboot.c +++ b/kernel/reboot.c @@ -569,22 +569,6 @@ static int legacy_pm_power_off(struct sys_off_data *data) return NOTIFY_DONE; } -/* - * Register sys-off handlers for legacy PM callbacks. This allows legacy - * PM callbacks co-exist with the new sys-off API. - * - * TODO: Remove legacy handlers once all legacy PM users will be switched - * to the sys-off based APIs. - */ -static int __init legacy_pm_init(void) -{ - register_sys_off_handler(SYS_OFF_MODE_POWER_OFF, SYS_OFF_PRIO_DEFAULT, - legacy_pm_power_off, NULL); - - return 0; -} -core_initcall(legacy_pm_init); - static void do_kernel_power_off_prepare(void) { blocking_notifier_call_chain(&power_off_prep_handler_list, 0, NULL); @@ -670,6 +654,18 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd, if (ret) return ret; + /* + * Register sys-off handler for the legacy PM callback. This allows + * legacy PM callbacks co-exist with the new sys-off API. + * + * TODO: Remove legacy handler once all legacy PM users will be + * switched to the sys-off based APIs. + */ + if (pm_power_off) + register_sys_off_handler(SYS_OFF_MODE_POWER_OFF, + SYS_OFF_PRIO_DEFAULT, + legacy_pm_power_off, NULL); + /* Instead of trying to make the power_off code look like * halt when pm_power_off is not set do it the easy way. */
Hi Dmitry, On Tue, May 24, 2022 at 3:41 PM Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > On 5/24/22 16:14, Geert Uytterhoeven wrote: > > On Tue, May 10, 2022 at 1:33 AM Dmitry Osipenko > > <dmitry.osipenko@collabora.com> wrote: > >> Add kernel_can_power_off() helper that replaces open-coded checks of > >> the global pm_power_off variable. This is a necessary step towards > >> supporting chained power-off handlers. > >> > >> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> > > > > Thanks for your patch, which is now commit 0e2110d2e910e44c > > ("kernel/reboot: Add kernel_can_power_off()") in pm/linux-next. > > > > This causes the "poweroff" command (Debian nfsroot) to no longer > > cleanly halt the system on arm32 systems, but fail with a panic > > instead: > > > > -reboot: System halted > > +reboot: Power down > > +Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000 > > +CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted > > 5.18.0-rc7-shmobile-00007-g0e2110d2e910 #1274 > > +Hardware name: Generic R-Car Gen2 (Flattened Device Tree) > > + unwind_backtrace from show_stack+0x10/0x14 > > + show_stack from dump_stack_lvl+0x40/0x4c > > + dump_stack_lvl from panic+0xf4/0x330 > > + panic from do_exit+0x1c8/0x8e4 > > + do_exit from __do_sys_reboot+0x174/0x1fc > > + __do_sys_reboot from ret_fast_syscall+0x0/0x54 > > +Exception stack(0xf0815fa8 to 0xf0815ff0) > > +5fa0: 004e6954 00000000 fee1dead 28121969 4321fedc f0d94600 > > +5fc0: 004e6954 00000000 00000000 00000058 befa0c78 00000000 befa0c10 004e56f8 > > +5fe0: 00000058 befa0b6c b6ec8d45 b6e4a746 > > +---[ end Kernel panic - not syncing: Attempted to kill init! > > exitcode=0x00000000 ]--- > > > > On arm64, "poweroff" causes a clean "reboot: Power down" before/after. > > > > On both arm32 and arm64, the same handlers are registered: > > - SYS_OFF_MODE_POWER_OFF_PREPARE: legacy_pm_power_off_prepare > > - SYS_OFF_MODE_POWER_OFF: legacy_pm_power_off > > > > On both arm32 and arm64, legacy_pm_power_off_prepare() is called. > > On both arm32 and arm64, legacy_pm_power_off() does not seem to > > be called. > > > > On arm32, both pm_power_off_prepare and pm_power_off are NULL. > > On arm64, pm_power_off_prepare is NULL, and > > pm_power_off is psci_sys_poweroff. > > > > Do you have a clue? > > Thanks! > > Thank you, Geert! I see the problem, the kernel_can_power_off() checks whether power-off handler is registered, but it's always registered because legacy_pm_power_off is registered unconditionally. So it causes trouble for platforms that don't have power-off handler installed at all. All platforms that I tested have a power-off handler, so now wonder that I didn't notice this before. > > This change should fix the problem, please give it a try: Thank you, that fixes the problem for me! Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 5/24/22 18:03, Geert Uytterhoeven wrote: > Hi Dmitry, > > On Tue, May 24, 2022 at 3:41 PM Dmitry Osipenko > <dmitry.osipenko@collabora.com> wrote: >> On 5/24/22 16:14, Geert Uytterhoeven wrote: >>> On Tue, May 10, 2022 at 1:33 AM Dmitry Osipenko >>> <dmitry.osipenko@collabora.com> wrote: >>>> Add kernel_can_power_off() helper that replaces open-coded checks of >>>> the global pm_power_off variable. This is a necessary step towards >>>> supporting chained power-off handlers. >>>> >>>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> >>> >>> Thanks for your patch, which is now commit 0e2110d2e910e44c >>> ("kernel/reboot: Add kernel_can_power_off()") in pm/linux-next. >>> >>> This causes the "poweroff" command (Debian nfsroot) to no longer >>> cleanly halt the system on arm32 systems, but fail with a panic >>> instead: >>> >>> -reboot: System halted >>> +reboot: Power down >>> +Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000 >>> +CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted >>> 5.18.0-rc7-shmobile-00007-g0e2110d2e910 #1274 >>> +Hardware name: Generic R-Car Gen2 (Flattened Device Tree) >>> + unwind_backtrace from show_stack+0x10/0x14 >>> + show_stack from dump_stack_lvl+0x40/0x4c >>> + dump_stack_lvl from panic+0xf4/0x330 >>> + panic from do_exit+0x1c8/0x8e4 >>> + do_exit from __do_sys_reboot+0x174/0x1fc >>> + __do_sys_reboot from ret_fast_syscall+0x0/0x54 >>> +Exception stack(0xf0815fa8 to 0xf0815ff0) >>> +5fa0: 004e6954 00000000 fee1dead 28121969 4321fedc f0d94600 >>> +5fc0: 004e6954 00000000 00000000 00000058 befa0c78 00000000 befa0c10 004e56f8 >>> +5fe0: 00000058 befa0b6c b6ec8d45 b6e4a746 >>> +---[ end Kernel panic - not syncing: Attempted to kill init! >>> exitcode=0x00000000 ]--- >>> >>> On arm64, "poweroff" causes a clean "reboot: Power down" before/after. >>> >>> On both arm32 and arm64, the same handlers are registered: >>> - SYS_OFF_MODE_POWER_OFF_PREPARE: legacy_pm_power_off_prepare >>> - SYS_OFF_MODE_POWER_OFF: legacy_pm_power_off >>> >>> On both arm32 and arm64, legacy_pm_power_off_prepare() is called. >>> On both arm32 and arm64, legacy_pm_power_off() does not seem to >>> be called. >>> >>> On arm32, both pm_power_off_prepare and pm_power_off are NULL. >>> On arm64, pm_power_off_prepare is NULL, and >>> pm_power_off is psci_sys_poweroff. >>> >>> Do you have a clue? >>> Thanks! >> >> Thank you, Geert! I see the problem, the kernel_can_power_off() checks whether power-off handler is registered, but it's always registered because legacy_pm_power_off is registered unconditionally. So it causes trouble for platforms that don't have power-off handler installed at all. All platforms that I tested have a power-off handler, so now wonder that I didn't notice this before. >> >> This change should fix the problem, please give it a try: > > Thank you, that fixes the problem for me! > > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Great! I'll send the proper patch soon.
diff --git a/include/linux/reboot.h b/include/linux/reboot.h index 6b951d68c0c7..c52f77ee4ddd 100644 --- a/include/linux/reboot.h +++ b/include/linux/reboot.h @@ -149,6 +149,7 @@ extern void kernel_restart_prepare(char *cmd); extern void kernel_restart(char *cmd); extern void kernel_halt(void); extern void kernel_power_off(void); +extern bool kernel_can_power_off(void); void ctrl_alt_del(void); diff --git a/kernel/reboot.c b/kernel/reboot.c index eaede35f45e2..982e58c11ce8 100644 --- a/kernel/reboot.c +++ b/kernel/reboot.c @@ -523,6 +523,18 @@ void do_kernel_power_off(void) atomic_notifier_call_chain(&power_off_handler_list, 0, NULL); } +/** + * kernel_can_power_off - check whether system can be powered off + * + * Returns true if power-off handler is registered and system can be + * powered off, false otherwise. + */ +bool kernel_can_power_off(void) +{ + return !atomic_notifier_call_chain_is_empty(&power_off_handler_list); +} +EXPORT_SYMBOL_GPL(kernel_can_power_off); + /** * kernel_power_off - power_off the system * @@ -581,7 +593,7 @@ SYSCALL_DEFINE4(reboot, int, magic1, int, magic2, unsigned int, cmd, /* Instead of trying to make the power_off code look like * halt when pm_power_off is not set do it the easy way. */ - if ((cmd == LINUX_REBOOT_CMD_POWER_OFF) && !pm_power_off) + if ((cmd == LINUX_REBOOT_CMD_POWER_OFF) && !kernel_can_power_off()) cmd = LINUX_REBOOT_CMD_HALT; mutex_lock(&system_transition_mutex);
Add kernel_can_power_off() helper that replaces open-coded checks of the global pm_power_off variable. This is a necessary step towards supporting chained power-off handlers. Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> --- include/linux/reboot.h | 1 + kernel/reboot.c | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-)