Re: [systemd-devel] RFC: enable suspend to idle

[Mantas Mikulėnas]

On Mon, Mar 5, 2018 at 5:19 PM, Oliver Neukum  wrote:

> On Fri, 2018-03-02 at 10:18 +0100, Lennart Poettering wrote:
>
> > 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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] RFC: enable suspend to idle

[Lennart Poettering]

On Mo, 05.03.18 16:19, Oliver Neukum (oneu...@suse.com) wrote:

> On Fri, 2018-03-02 at 10:18 +0100, Lennart Poettering wrote:
> 
> > 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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] RFC: enable suspend to idle

[Oliver Neukum]

On Fri, 2018-03-02 at 10:18 +0100, Lennart Poettering wrote:

> 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


___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] RFC: enable suspend to idle

[Lennart Poettering]

On Do, 01.03.18 16:13, Thomas Blume (thomas.bl...@suse.com) wrote:

> On Thu, 1 Mar 2018, Lennart Poettering wrote:
> 
> > > 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?

> 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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] RFC: enable suspend to idle

[Thomas Blume]

On Thu, 1 Mar 2018, Lennart Poettering wrote:


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___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] RFC: enable suspend to idle

[Lennart Poettering]

On Do, 01.03.18 15:25, Oliver Neukum (oneu...@suse.com) wrote:

> Am Donnerstag, den 01.03.2018, 15:17 +0100 schrieb Lennart Poettering:
> > On Do, 01.03.18 14:40, Thomas Blume (thomas.bl...@suse.com) wrote:
> 
> > > 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"
> > 
> > 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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] RFC: enable suspend to idle

[Lennart Poettering]

On Do, 01.03.18 14:40, Thomas Blume (thomas.bl...@suse.com) wrote:

> 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"

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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] RFC: enable suspend to idle

[Oliver Neukum]

Am Donnerstag, den 01.03.2018, 15:17 +0100 schrieb Lennart Poettering:
> On Do, 01.03.18 14:40, Thomas Blume (thomas.bl...@suse.com) wrote:

> > 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"
> 
> 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

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] RFC: enable suspend to idle

[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.
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___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel