Discussion:
RFC: enable suspend to idle
(too old to reply)
Thomas Blume
2018-03-01 13:40:22 UTC
Permalink
Hi,

It seems that major vendors are dropping support of sleep mode S3 in favor of
the new suspend to idle method.
We need to enable this new feature in order to be able to suspend to RAM on
such machines.
The kernel side of this feature has already been implemented:

https://patchwork.kernel.org/patch/9873163

but we need to activate it in userspace.
Each device, supporting suspend to idle from kernel side, has a file called
wakeup in sysfs.
Echoing "enabled" into this file will enable the feature.

As a proof of concept, I have created below udev rule and helper script, which
works on my testmachine.
Obviously, like that it isn't portable to other distros, but I'd like to get
comments whether this is the way to go.
If I get positive feedback, I'll try a portable approach using a binary helper.

udev rule:

-->
ACTION=="add", ATTR{power/wakeup}=="disabled", IMPORT{program}="/usr/lib/udev/get-wakeup-devices.sh %p"
ENV{RESUME_FROM_IDLE}=="1", ATTR{power/wakeup}="enabled"
--<

helper script:

-->
#!/bin/sh

SYSDEVS=$(find /sys/devices/pci* -name wakeup)
NETWORKDEVS=$(lspci | sed -n '/Ethernet controller/s/^\([[:graph:]]*\).*/\1/p')
USBINPUTDEVS=$(find /sys/devices/pci*/*/usb* -name input)
OTHERDEVS=$(find /sys/devices -name wakeup | sed -n '/serio/p;/pnp/p;/LNXPWRBN/p')

#get ethernet and usb devices
for sysdev in $SYSDEVS; do
PCIID=$(echo $sysdev | sed -n 's#/sys/devices/pci[[:digit:]]*:[[:digit:]]*/[[:digit:]]*:\([[:digit:]]*:[[:alnum:]]*.[[:alnum:]]\).*#\1#p')
for netdev in $NETWORKDEVS; do
[[ "$PCIID" =~ $netdev ]] && [[ ! "$WAKEUPDEVS" =~ ${sysdev%/power/wakeup} ]] && WAKEUPDEVS+="$sysdev ";
done
for usbinputdev in $USBINPUTDEVS; do
[[ $usbinputdev =~ $PCIID ]] && [[ ! "$WAKEUPDEVS" =~ ${sysdev%/power/wakeup} ]] && WAKEUPDEVS+="$sysdev ";
done
done

#get ps2, plug and play and power button devices
for otherdev in $OTHERDEVS; do
[[ "$WAKEUPDEVS" =~ ${otherdev%/power/wakeup} ]] || WAKEUPDEVS+="$otherdev ";
done

[[ "$WAKEUPDEVS" =~ "$1/power/wakeup" ]] && echo "RESUME_FROM_IDLE=1"
--<


Regards
Thomas Blume
--
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
Maxfeldstr. 5 / D-90409 Nürnberg / Phone: +49-911-740 53 - 0 / VOIP: 3919
GPG 2048R/2CD4D3E8 9A50 048F 1C73 59AA 4D2E 424E B3C6 3FD9 2CD4 D3E8
Lennart Poettering
2018-03-01 14:17:14 UTC
Permalink
Post by Thomas Blume
Hi,
It seems that major vendors are dropping support of sleep mode S3 in favor of
the new suspend to idle method.
We need to enable this new feature in order to be able to suspend to RAM on
such machines.
https://patchwork.kernel.org/patch/9873163
but we need to activate it in userspace.
Each device, supporting suspend to idle from kernel side, has a file called
wakeup in sysfs.
Echoing "enabled" into this file will enable the feature.
As a proof of concept, I have created below udev rule and helper script, which
works on my testmachine.
Obviously, like that it isn't portable to other distros, but I'd like to get
comments whether this is the way to go.
If I get positive feedback, I'll try a portable approach using a binary helper.
-->
ACTION=="add", ATTR{power/wakeup}=="disabled", IMPORT{program}="/usr/lib/udev/get-wakeup-devices.sh %p"
ENV{RESUME_FROM_IDLE}=="1", ATTR{power/wakeup}="enabled"
Not following here. This doesn't appear like something where userspace
should be involved. We generally avoid udev rules whose only job is to
"shortcut" kernel events back into the kernel. Why doesn't the kernel
set this up properly anyway on its own?

Lennart
--
Lennart Poettering, Red Hat
Oliver Neukum
2018-03-01 14:25:40 UTC
Permalink
Post by Lennart Poettering
Post by Thomas Blume
As a proof of concept, I have created below udev rule and helper script, which
works on my testmachine.
Obviously, like that it isn't portable to other distros, but I'd like to get
comments whether this is the way to go.
If I get positive feedback, I'll try a portable approach using a binary helper.
-->
ACTION=="add", ATTR{power/wakeup}=="disabled", IMPORT{program}="/usr/lib/udev/get-wakeup-devices.sh %p"
ENV{RESUME_FROM_IDLE}=="1", ATTR{power/wakeup}="enabled"
Not following here. This doesn't appear like something where userspace
should be involved. We generally avoid udev rules whose only job is to
"shortcut" kernel events back into the kernel. Why doesn't the kernel
set this up properly anyway on its own?
The kernel must not set policy on what is a source of wake ups. Setting
this up so that we do not get a regression in functionality compared
to old style S3 (whose policy is in firmware) falls to user space,
more specifically udev.

Regards
Oliver
Lennart Poettering
2018-03-01 15:09:00 UTC
Permalink
Post by Oliver Neukum
Post by Lennart Poettering
Post by Thomas Blume
As a proof of concept, I have created below udev rule and helper script, which
works on my testmachine.
Obviously, like that it isn't portable to other distros, but I'd like to get
comments whether this is the way to go.
If I get positive feedback, I'll try a portable approach using a binary helper.
-->
ACTION=="add", ATTR{power/wakeup}=="disabled", IMPORT{program}="/usr/lib/udev/get-wakeup-devices.sh %p"
ENV{RESUME_FROM_IDLE}=="1", ATTR{power/wakeup}="enabled"
Not following here. This doesn't appear like something where userspace
should be involved. We generally avoid udev rules whose only job is to
"shortcut" kernel events back into the kernel. Why doesn't the kernel
set this up properly anyway on its own?
The kernel must not set policy on what is a source of wake ups. Setting
this up so that we do not get a regression in functionality compared
to old style S3 (whose policy is in firmware) falls to user space,
more specifically udev.
And where would udev have that information from? I mean, if it turns
this on for all devices, then why can't the kernel do that on its own?

Lennart
--
Lennart Poettering, Red Hat
Thomas Blume
2018-03-01 15:13:53 UTC
Permalink
Post by Lennart Poettering
Post by Oliver Neukum
The kernel must not set policy on what is a source of wake ups. Setting
this up so that we do not get a regression in functionality compared
to old style S3 (whose policy is in firmware) falls to user space,
more specifically udev.
And where would udev have that information from? I mean, if it turns
this on for all devices, then why can't the kernel do that on its own?
We don't want all devices for which the kernel is supporting wake on idle, to
act as wakeup device.
Ideally this will be a config option with reasonable defaults.
The udev rule should consider this.

Thomas Blume
--
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
Maxfeldstr. 5 / D-90409 Nürnberg / Phone: +49-911-740 53 - 0 / VOIP: 3919
GPG 2048R/2CD4D3E8 9A50 048F 1C73 59AA 4D2E 424E B3C6 3FD9 2CD4 D3E8
Lennart Poettering
2018-03-02 09:18:00 UTC
Permalink
Post by Thomas Blume
Post by Lennart Poettering
Post by Oliver Neukum
The kernel must not set policy on what is a source of wake ups. Setting
this up so that we do not get a regression in functionality compared
to old style S3 (whose policy is in firmware) falls to user space,
more specifically udev.
And where would udev have that information from? I mean, if it turns
this on for all devices, then why can't the kernel do that on its own?
We don't want all devices for which the kernel is supporting wake on idle, to
act as wakeup device.
But for which ones would you want that?
Post by Thomas Blume
Ideally this will be a config option with reasonable defaults.
But why wouldn't that be a kernel option? I mean, so far the goal was
to encode "reasonable defaults" in the kernel itself, so that
userspace is only used when those "reasonable defaults" do not apply
onto one local case.

Really, already for compatibility reasons the kernel should just carry
the "reasonable defaults", so that it's not necessary to match it up
with a udev version that carries the right policy for it.

Lennart
--
Lennart Poettering, Red Hat
Oliver Neukum
2018-03-05 15:19:55 UTC
Permalink
Post by Lennart Poettering
But why wouldn't that be a kernel option? I mean, so far the goal was
to encode "reasonable defaults" in the kernel itself, so that
userspace is only used when those "reasonable defaults" do not apply
onto one local case.
Really, already for compatibility reasons the kernel should just carry
the "reasonable defaults", so that it's not necessary to match it up
with a udev version that carries the right policy for it.
Well, no. The kernel must carry conservative defaults that do no harm
in any case. Setting defaults sensible for the class of systems systemd
runs on is the job of udev.

For now we are running with defaults taken from firmware, which can be
expected to be tailored to the system it comes with.
Falling back to conservative defaults would mean a regression in
functionality.

Regards
Oliver
Lennart Poettering
2018-03-08 22:22:25 UTC
Permalink
Post by Oliver Neukum
Post by Lennart Poettering
But why wouldn't that be a kernel option? I mean, so far the goal was
to encode "reasonable defaults" in the kernel itself, so that
userspace is only used when those "reasonable defaults" do not apply
onto one local case.
Really, already for compatibility reasons the kernel should just carry
the "reasonable defaults", so that it's not necessary to match it up
with a udev version that carries the right policy for it.
Well, no. The kernel must carry conservative defaults that do no harm
in any case. Setting defaults sensible for the class of systems systemd
runs on is the job of udev.
For now we are running with defaults taken from firmware, which can be
expected to be tailored to the system it comes with.
Falling back to conservative defaults would mean a regression in
functionality.
I don't get it. If there's a "regression" in the kernel's behaviour,
then perhaps the kernel should be fixed there.

But again: we so far have not shipped rules with udev whose exclusive
job is to push default policy into the kernel that the kernel might as
well just apply on its own. And I don't think we should start with
that now.

Yes, udev is the right place for applying *local* policy,
i.e. specific deviations from the default that apply to specific
systems a user/admin maintains.

Yes, udev is also the right place for applying *generic* policy that
the kernel couldn't come up with all alone, i.e. that requires access
to other userspace components (let's say, the user database or such).

But no, udev is *not* the right place for default policy that might as
well be in the kernel anyway.

Sorry,

Lennart
--
Lennart Poettering, Red Hat
Mantas Mikulėnas
2018-03-09 08:13:33 UTC
Permalink
Post by Oliver Neukum
Post by Lennart Poettering
But why wouldn't that be a kernel option? I mean, so far the goal was
to encode "reasonable defaults" in the kernel itself, so that
userspace is only used when those "reasonable defaults" do not apply
onto one local case.
Really, already for compatibility reasons the kernel should just carry
the "reasonable defaults", so that it's not necessary to match it up
with a udev version that carries the right policy for it.
Well, no. The kernel must carry conservative defaults that do no harm
in any case. Setting defaults sensible for the class of systems systemd
runs on is the job of udev.
What would set sensible defaults on systems which don't run systemd nor
udev?
--
Mantas Mikulėnas
Loading...