Bug 276784

Summary: Can not power off with shutdown -p
Product: Base System Reporter: Katsuyuki Miyoshi <katsubsd>
Component: kernAssignee: Andriy Gapon <avg>
Status: Closed FIXED    
Severity: Affects Only Me CC: avg, freebsd, ish
Priority: ---    
Version: CURRENT   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
proposed fix none

Description Katsuyuki Miyoshi 2024-02-02 14:43:20 UTC 
=============================================================================
commit 9cdf326b4faef97f0d3314b5dd693308ac494d48
Author:     Andriy Gapon <avg@FreeBSD.org>
AuthorDate: Mon Dec 20 13:01:56 2021 +0200
Commit:     Andriy Gapon <avg@FreeBSD.org>
CommitDate: Sun Jan 28 15:04:55 2024 +0200

    run acpi_shutdown_final later to give other handlers a chance
    
    For example, shutdown_panic wants to produce some output and maybe take
    some input before a system is actually reset.
    
    The change should only make difference for the case of system reset
    (reboot), poweroff and halt should not be affected.
    
    The change makes difference only if hw.acpi.handle_reboot is set.  It
    used to default to zero until r213755 / ac731af5670c7.

diff --git a/sys/dev/acpica/acpi.c b/sys/dev/acpica/acpi.c
index 9196c446ae80..7d1fc10afb9e 100644
--- a/sys/dev/acpica/acpi.c
+++ b/sys/dev/acpica/acpi.c
@@ -675,7 +675,7 @@ acpi_attach(device_t dev)
 
     /* Register our shutdown handler. */
     EVENTHANDLER_REGISTER(shutdown_final, acpi_shutdown_final, sc,
-	SHUTDOWN_PRI_LAST);
+	SHUTDOWN_PRI_LAST + 150);
 
     /*
      * Register our acpi event handlers.
=============================================================================

Since this commit I can no longer power off with shutdown -p.

=============================================================================
# shutdown -p now
...
Waiting (max 60 seconds) for system process `vnlru' to stop... done
Waiting (max 60 seconds) for system process `syncer' to stop...
Syncing disks, vnodes remaining... 0 0 0 0 0 done
All buffers synced.
Uptime: 1m42s
uhub4: detached
uhub0: detached
uhub4: detached
...
...
uhub2: detached

The operating system has halted.
Please press any key to reboot.
[The system stalled with power on.]
=============================================================================

I reverted the changes in the above commit. And I can now power off with shutdown -p.
Comment 1 Masachika ISHIZUKA 2024-02-03 03:28:17 UTC 
(In reply to Katsuyuki Miyoshi from comment #0)
Thank you very much.
After reverting acpi.c to main-n267924-0f4e8037332f, I can power off my dell pc (vostro 3267) by 'shutdown -p now'.
Comment 2 Andriy Gapon freebsd_committer freebsd_triage 2024-02-03 10:06:01 UTC 
Created attachment 248154 [details]
proposed fix

Could you please test this patch instead?
Comment 3 Katsuyuki Miyoshi 2024-02-03 12:58:56 UTC 
(In reply to Andriy Gapon from comment #2)
After applying this patch, shutdown -p works fine.
Thank you!
Comment 4 Masachika ISHIZUKA 2024-02-03 14:12:59 UTC 
(In reply to Andriy Gapon from comment #2)
This patch works fine on dell vostro 3267, too.
Comment 5 commit-hook freebsd_committer freebsd_triage 2024-02-06 09:42:06 UTC 
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e4ab361e53945a6c3e9d68c5e5ffc11de40a35f2

commit e4ab361e53945a6c3e9d68c5e5ffc11de40a35f2
Author:     Andriy Gapon <avg@FreeBSD.org>
AuthorDate: 2024-02-06 08:55:13 +0000
Commit:     Andriy Gapon <avg@FreeBSD.org>
CommitDate: 2024-02-06 08:55:13 +0000

    fix poweroff regression from 9cdf326b4f by delaying shutdown_halt

    The regression affected ACPI-based systems without EFI poweroff support
    (including VMs).

    The key reason for the regression is that I overlooked that poweroff is
    requested by RB_POWEROFF | RB_HALT combination of flags.  In my opinion,
    that command is a bit bipolar, but since we've been doing that forever,
    then so be it.  Because of that flag combination, the order of
    shutdown_final handlers that check for either flag does matter.

    Some additional complexity comes from platform-specific shutdown_final
    handlers that aim to handle multiple reboot options at once.  E.g.,
    acpi_shutdown_final handles both poweroff and reboot / reset.  As
    explained in 9cdf326b4f, such a handler must run after shutdown_panic to
    give it a chance.  But as the change revealed, the handler must also run
    before shutdown_halt, so that the system can actually power off before
    entering the halt limbo.

    Previously, shutdown_panic and shutdown_halt had the same priority which
    appears to be incompatible with handlers that can do both poweroff and
    reset.

    The above also applies to power cycle handlers.

    PR:             276784
    Reported by:    many
    Tested by:      Katsuyuki Miyoshi <katsubsd@gmail.com>,
                    Masachika ISHIZUKA <ish@amail.plala.or.jp>
    Fixes:          9cdf326b4fae run acpi_shutdown_final later to give other handlers a chance
    MFC after:      1 week

 sys/kern/kern_shutdown.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Comment 6 Andriy Gapon freebsd_committer freebsd_triage 2024-02-06 09:43:07 UTC 
Thank you for testing!

The fix has just been committed.