Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] acpi: button: Provide option for power button to directly signal init
@ 2020-01-11  2:21 Josh Triplett
  2020-01-30 21:07 ` Rafael J. Wysocki
  0 siblings, 1 reply; 3+ messages in thread
From: Josh Triplett @ 2020-01-11  2:21 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown, Jonathan Corbet, Arjan van de Ven
  Cc: linux-doc, linux-kernel, linux-acpi

Virtual machines and containers often use an ACPI power button event to
tell the machine to shut down gracefully.

Provide an optional, extremely lightweight way to handle this event by
signaling init directly, rather than running a separate daemon (such as
acpid or systemd-logind) that adds to startup time and VM image
complexity.

By default, the power button will continue to notify userspace through
the input layer. With the button.power_signal parameter set, the
power button will instead send the configured signal to init. (For
instance, sending SIGINT will make the power button simulate
ctrl-alt-del.)

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
 drivers/acpi/button.c                           | 11 +++++++++++
 2 files changed, 17 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index ade4e6ec23e0..bbb598e148f4 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -450,6 +450,12 @@
 			firmware feature for flushing multiple hpte entries
 			at a time.
 
+	button.power_signal=
+			[ACPI] When the power button is pressed, send this
+			signal number to the init process. If set to 0
+			(default), do not send a signal.
+			Format: integer
+
 	c101=		[NET] Moxa C101 synchronous serial card
 
 	cachesize=	[BUGS=X86-32] Override level 2 CPU cache size detection.
diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index b758b45737f5..923259f132d6 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -14,6 +14,7 @@
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/proc_fs.h>
+#include <linux/sched/signal.h>
 #include <linux/seq_file.h>
 #include <linux/input.h>
 #include <linux/slab.h>
@@ -167,6 +168,10 @@ static unsigned long lid_report_interval __read_mostly = 500;
 module_param(lid_report_interval, ulong, 0644);
 MODULE_PARM_DESC(lid_report_interval, "Interval (ms) between lid key events");
 
+static int power_signal __read_mostly = 0;
+module_param(power_signal, int, 0644);
+MODULE_PARM_DESC(power_signal, "Power button sends this signal to init");
+
 /* --------------------------------------------------------------------------
                               FS Interface (/proc)
    -------------------------------------------------------------------------- */
@@ -426,6 +431,12 @@ static void acpi_button_notify(struct acpi_device *device, u32 event)
 			if (button->suspended)
 				break;
 
+			if (power_signal
+			    && button->type == ACPI_BUTTON_TYPE_POWER) {
+				kill_cad_pid(power_signal, 1);
+				break;
+			}
+
 			keycode = test_bit(KEY_SLEEP, input->keybit) ?
 						KEY_SLEEP : KEY_POWER;
 			input_report_key(input, keycode, 1);
-- 
2.25.0.rc2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] acpi: button: Provide option for power button to directly signal init
  2020-01-11  2:21 [PATCH] acpi: button: Provide option for power button to directly signal init Josh Triplett
@ 2020-01-30 21:07 ` Rafael J. Wysocki
  2020-02-05  0:24   ` Josh Triplett
  0 siblings, 1 reply; 3+ messages in thread
From: Rafael J. Wysocki @ 2020-01-30 21:07 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Rafael J. Wysocki, Len Brown, Jonathan Corbet, Arjan van de Ven,
	open list:DOCUMENTATION, Linux Kernel Mailing List,
	ACPI Devel Maling List

On Sat, Jan 11, 2020 at 3:21 AM Josh Triplett <josh@joshtriplett.org> wrote:
>
> Virtual machines and containers often use an ACPI power button event to
> tell the machine to shut down gracefully.
>
> Provide an optional, extremely lightweight way to handle this event by
> signaling init directly, rather than running a separate daemon (such as
> acpid or systemd-logind) that adds to startup time and VM image
> complexity.

Well, I'm not convinced.

Even though the patch looks straightforward, the approach really is
quite not so conceptually and honestly it looks like a band-aid.

Also I'm not quite sure why the ACPI button driver is the target of
this and not the input layer, for instance.

> By default, the power button will continue to notify userspace through
> the input layer. With the button.power_signal parameter set, the
> power button will instead send the configured signal to init. (For
> instance, sending SIGINT will make the power button simulate
> ctrl-alt-del.)
>
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
>  drivers/acpi/button.c                           | 11 +++++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index ade4e6ec23e0..bbb598e148f4 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -450,6 +450,12 @@
>                         firmware feature for flushing multiple hpte entries
>                         at a time.
>
> +       button.power_signal=
> +                       [ACPI] When the power button is pressed, send this
> +                       signal number to the init process. If set to 0
> +                       (default), do not send a signal.
> +                       Format: integer
> +
>         c101=           [NET] Moxa C101 synchronous serial card
>
>         cachesize=      [BUGS=X86-32] Override level 2 CPU cache size detection.
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index b758b45737f5..923259f132d6 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -14,6 +14,7 @@
>  #include <linux/init.h>
>  #include <linux/types.h>
>  #include <linux/proc_fs.h>
> +#include <linux/sched/signal.h>
>  #include <linux/seq_file.h>
>  #include <linux/input.h>
>  #include <linux/slab.h>
> @@ -167,6 +168,10 @@ static unsigned long lid_report_interval __read_mostly = 500;
>  module_param(lid_report_interval, ulong, 0644);
>  MODULE_PARM_DESC(lid_report_interval, "Interval (ms) between lid key events");
>
> +static int power_signal __read_mostly = 0;
> +module_param(power_signal, int, 0644);
> +MODULE_PARM_DESC(power_signal, "Power button sends this signal to init");
> +
>  /* --------------------------------------------------------------------------
>                                FS Interface (/proc)
>     -------------------------------------------------------------------------- */
> @@ -426,6 +431,12 @@ static void acpi_button_notify(struct acpi_device *device, u32 event)
>                         if (button->suspended)
>                                 break;
>
> +                       if (power_signal
> +                           && button->type == ACPI_BUTTON_TYPE_POWER) {
> +                               kill_cad_pid(power_signal, 1);
> +                               break;
> +                       }
> +
>                         keycode = test_bit(KEY_SLEEP, input->keybit) ?
>                                                 KEY_SLEEP : KEY_POWER;
>                         input_report_key(input, keycode, 1);
> --
> 2.25.0.rc2
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] acpi: button: Provide option for power button to directly signal init
  2020-01-30 21:07 ` Rafael J. Wysocki
@ 2020-02-05  0:24   ` Josh Triplett
  0 siblings, 0 replies; 3+ messages in thread
From: Josh Triplett @ 2020-02-05  0:24 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Len Brown, Jonathan Corbet, Arjan van de Ven,
	open list:DOCUMENTATION, Linux Kernel Mailing List,
	ACPI Devel Maling List

On Thu, Jan 30, 2020 at 10:07:09PM +0100, Rafael J. Wysocki wrote:
> On Sat, Jan 11, 2020 at 3:21 AM Josh Triplett <josh@joshtriplett.org> wrote:
> >
> > Virtual machines and containers often use an ACPI power button event to
> > tell the machine to shut down gracefully.
> >
> > Provide an optional, extremely lightweight way to handle this event by
> > signaling init directly, rather than running a separate daemon (such as
> > acpid or systemd-logind) that adds to startup time and VM image
> > complexity.
> 
> Well, I'm not convinced.

I would be happy to talk through other possible options.

> Even though the patch looks straightforward, the approach really is
> quite not so conceptually and honestly it looks like a band-aid.

I'm not sure what makes it conceptually non-straightforward. I'll freely
admit that it isn't *elegant*. But it also seems inelegant to me to need
to start and run an entire userspace daemon to watch for a key, or to
add such logic into a domain-specific workload's setup and event loop.

I'm entirely open to changing the approach. The goal I'm aiming for is
to allow cloud systems and VMs that signal shutdown via an ACPI power
button to run a more minimal userspace.

> Also I'm not quite sure why the ACPI button driver is the target of
> this and not the input layer, for instance.

I don't have any objections to putting this in another part of the
stack, but input in particular seems like it would potentially affect
the normal event-processing path.

- Josh Triplett

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-11  2:21 [PATCH] acpi: button: Provide option for power button to directly signal init Josh Triplett
2020-01-30 21:07 ` Rafael J. Wysocki
2020-02-05  0:24   ` Josh Triplett

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git