From nobody Tue Feb 06 10:13:46 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4TTfGj52R4z598Bk; Tue, 6 Feb 2024 10:13:49 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4TTfGj4Kzqz48BH; Tue, 6 Feb 2024 10:13:49 +0000 (UTC) (envelope-from avg@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1707214429; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=DOntLQu/iMREhV/gydCaCBlY2dVsZzQkOC4maQG6P6M=; b=VNGlyyoWBFnrK9JqVAQBmMbnNyvFUBLWfnW3Xm+rlJLbShdteu2d9SbUo2idFxSWKpe5ud rI0PEh93ZCMn9iMpv3dcklyZH2sOUnJuHRddbuzUtApLf0cG2eBCn5OELe29tzkWRXZDzi YQEt2eP2baxkPd4J93aGOOZjCsSGv9rOLlbSDc8rqUfFgeZp7K8d22nMplSSgpo63EztO3 Unsvl2RF540SQ0kDEH8ZR9IhkOxas/GBhuqjZOcuh1iqJ1ElyXwnrvd/H2OMYqrMHtcVqG kMl0Og7Wtd8FgY9hyFtksXBzeOcaEtp2UHu4gkuqenJDah1vNIPoU/v6K7gI8A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1707214429; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=DOntLQu/iMREhV/gydCaCBlY2dVsZzQkOC4maQG6P6M=; b=rsTkOrtHoSyEvedKibsd5v+0auiKSfP17tn/jHWMW8I+JpwDKRnvj9IVIXbBlda7NLZq/H kZrOAJKImVFnpDMAsuQ0r+JzniBdaMVLd17kXZPIl9x4F1yz8h9HbTXbq7qQ6MDGX+mKIx 6xZJsmVyV8D9MUl5ZFCE4nEpdUGuVRC4zoM5I0toF9+WbHP+r99FXRKrKzshvE9HMzJgNC CNWT2UdKKTmliMA28iMCaAZ3tSps+OGQblGrNxM0/3YwowLn6dr5W26GkyyTE+/PWrL9U/ zeD8XUxjDpFWHcbg44cLsastD7+sZP6KMfS90S1YcX2IDd/OJSjD/T55VZxnxw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1707214429; a=rsa-sha256; cv=none; b=tO/atHZ62f9HpbufsCweAc9m7mLnRvx7t6TEw8iGPPLMdAQHAg6rAIqTWcsH+4V9iZZhcs skSkpeS0u0UZ6z5rjohbzSpL4h1TH8XsCbe5MdubQME/r5oYws4HGkYfSH/3y295PrNWPk ftC08CXWl0a0IWQXJTIfPz7InOblWSyOLO0Qq7jKpbOly+OKMxMPV1UPehsBvC20R7km2u 6k5hbBPqfhJH0V9MGkjg8UoEzcoevGO+78G5Cfp7xMgw5n+svJmsn4BW0xIXuR8iv3n8y/ 8xgcPL0BRccKj/0PQlb8BemiPbWByYGacjqRCRd6OqM7foSGu3ZyPM2yR3eSZw== Received: from [192.168.0.88] (unknown [93.188.39.137]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) (Authenticated sender: avg/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 4TTfGj00K8zRBv; Tue, 6 Feb 2024 10:13:48 +0000 (UTC) (envelope-from avg@FreeBSD.org) Message-ID: <72def5a9-ffcc-4dcc-9b85-875ba7f46539@FreeBSD.org> Date: Tue, 6 Feb 2024 12:13:46 +0200 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: git: e4ab361e5394 - main - fix poweroff regression from 9cdf326b4f by delaying shutdown_halt From: Andriy Gapon To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 06/02/2024 11:41, Andriy Gapon wrote: > The branch main has been updated by avg: > > URL: https://cgit.FreeBSD.org/src/commit/?id=e4ab361e53945a6c3e9d68c5e5ffc11de40a35f2 > > commit e4ab361e53945a6c3e9d68c5e5ffc11de40a35f2 > Author: Andriy Gapon > AuthorDate: 2024-02-06 08:55:13 +0000 > Commit: Andriy Gapon > 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. I want to add that having many handlers with priorities expressed like SHUTDOWN_PRI_LAST ± N while some of those handlers have implicit inter-dependencies (interactions, interference) also does not help to see a clear picture. Perhaps it would be better to handle all (reasonable) RB flag combinations centrally in kern_reboot and then dispatch events like shutdown_reset, shutdown_poweroff, etc. Handlers for those events would have a single and simple job of performing that one action (perhaps failing and letting another handler try). Also, I would split reboot howto into command and flag portions, so that only one command can be specified at a time. E.g., I would consider RB_AUTOBOOT ("RB_REBOOT"), RB_POWEROFF, RB_HALT to be distinct commands. Then, flags like RB_NOSYNC or RB_DUMP could be optional flags. As an aside, some flags documented for reboot(2) do not seem to have much to do with reboot. E.g., RB_DFLTROOT affects how a system boots up, but not how the system goes for a reboot. Not surprisingly, that option is not handled by anything kicked off with reboot(2). Maybe, it would make more sense if we had fast reboot support and the running kernel could instruct the next kernel directly. But, it's still a bit weird that flags like RB_POWEROFF and RB_DFLTROOT belong in the same domain and can be set together. -- Andriy Gapon