logind.CanSuspendThenHibernate returns success, trying to actually do suspend-then-hibernate fails

[jwrdegoede]

I'm seeing $subject on a Bay Trail tablet where dmesg says the following about the rtc:

[ 2.597524] rtc_cmos 00:00: registered as rtc0
[ 2.597529] rtc_cmos 00:00: no alarms, y3k, 242 bytes nvram
[ 2.964475] rtc_cmos 00:00: setting system clock to 2018-08-31 21:29:15 UTC (1535750955)

Notice the "no alarms", gnome-settings-daemon does a logind.CanSuspendThenHibernate call to see if hibernate-then-suspend can be used, this returns true on this device despite the "no alarms" in dmesg.

When g-s-d then asks logind to suspend-then-hibernate this fails like this:

Aug 31 21:30:18 localhost.localdomain systemd-sleep[2237]: Failed to write '1535761817' to /sys/class/rtc/rtc0/wakealarm: Input/output error
Aug 31 21:30:18 localhost.localdomain systemd[1]: systemd-suspend-then-hibernate.service: Main process exited, code=exited, status=1/FAILURE
Aug 31 21:30:18 localhost.localdomain systemd[1]: systemd-suspend-then-hibernate.service: Failed with result 'exit-code'.
Aug 31 21:30:18 localhost.localdomain systemd[1]: Failed to start Suspend; Hibernate if not used for a period of time.
Aug 31 21:30:18 localhost.localdomain systemd[1]: Dependency failed for Suspend; Hibernate if not used for a period of time.
Aug 31 21:30:18 localhost.localdomain audit[1]: SERVICE_START pid=1 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 msg='unit=systemd-suspend-then-hibernate comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? res=failed'
Aug 31 21:30:18 localhost.localdomain systemd[1]: suspend-then-hibernate.target: Job suspend-then-hibernate.target/start failed with result 'dependency'.
Aug 31 21:30:18 localhost.localdomain systemd[1]: sleep.target: Unit not needed anymore. Stopping.
Aug 31 21:30:18 localhost.localdomain systemd[1]: Stopped target Sleep.
Aug 31 21:30:18 localhost.localdomain systemd-logind[757]: Operation 'sleep' finished.

This causes the tablet to wakeup again immediately when the power-button is pressed to suspend the device.

[lucaswerkmeister]

If I’m reading the kernel source correctly, the message “no alarms” and the EIO error are generated in case of !is_valid_irq(cmos->irq). The same condition also guards reads from the wakeup file, as far as I can see, so perhaps logind should try to perform a read from the file to determine whether wakeups are supported or not.

Can you check what cat /sys/class/rtc/rtc0/wakealarm reports on this tablet?

[lucaswerkmeister]

(systemd’s can_s2h function already tries to access the wakeup file in W_OK mode, but it looks like that’s not enough to guarantee that the file actually supports writing. Perhaps this also depends on the RTC driver.)

[jwrdegoede]

"cat /sys/class/rtc/rtc0/wakealarm" reports nothing on this tablet, it results in a direct EOF / return to commandline (same as a "cat /dev/null").

[lucaswerkmeister]

Hm… do you know if there are any other ways to detect the non-availability of alarms on that device, other than the equivalent of journalctl -k --grep '^rtc.*no alarms'? :/

[jwrdegoede]

That is a valid point, so I've taken a good look at the rtc-cmos.c driver and come to the conclusion that it needs to be fixed there.

After adding the below patch to the kernel there no longer is a /sys/class/rtc/rtc0/wakealarm file and everything works as it should. So this really is a kernel issue and this issue can be closed.

I will submit the below patch upstream right away.

From 916475d5af7ad2d77e761dc35af7b90bf5bd517f Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Tue, 4 Sep 2018 10:19:44 +0200
Subject: [PATCH] rtc: cmos: Do not export alarm rtc_ops when we do not support
 alarms

When there is no IRQ configured for the RTC, the rtc-cmos code does not
support alarms, all alarm rtc_ops fail with -EIO / -EINVAL.

The rtc-core expects a rtc driver which does not support rtc alarms to
not have alarm ops at all. Otherwise the wakealarm sysfs attr will read
as empty rather then returning an error, making it impossible for
userspace to find out beforehand if alarms are supported.

A system without an IRQ for the RTC before this patch:
[root@localhost ~]# cat /sys/class/rtc/rtc0/wakealarm
[root@localhost ~]#

After this patch:
[root@localhost ~]# cat /sys/class/rtc/rtc0/wakealarm
cat: /sys/class/rtc/rtc0/wakealarm: No such file or directory
[root@localhost ~]#

This fixes gnome-session + systemd trying to use suspend-then-hibernate,
which causes systemd to abort the suspend when writing the RTC alarm fails.

BugLink: https://github.com/systemd/systemd/issues/9988
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/rtc/rtc-cmos.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index cd3a2411bc2f..490cdbca4430 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -244,6 +244,7 @@ static int cmos_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 	struct cmos_rtc	*cmos = dev_get_drvdata(dev);
 	unsigned char	rtc_control;
 
+	/* This not only a rtc_op, but also called directly */
 	if (!is_valid_irq(cmos->irq))
 		return -EIO;
 
@@ -439,6 +440,7 @@ static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 	unsigned char mon, mday, hrs, min, sec, rtc_control;
 	int ret;
 
+	/* This not only a rtc_op, but also called directly */
 	if (!is_valid_irq(cmos->irq))
 		return -EIO;
 
@@ -503,9 +505,6 @@ static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled)
 	struct cmos_rtc	*cmos = dev_get_drvdata(dev);
 	unsigned long	flags;
 
-	if (!is_valid_irq(cmos->irq))
-		return -EINVAL;
-
 	spin_lock_irqsave(&rtc_lock, flags);
 
 	if (enabled)
@@ -566,6 +565,12 @@ static const struct rtc_class_ops cmos_rtc_ops = {
 	.alarm_irq_enable	= cmos_alarm_irq_enable,
 };
 
+static const struct rtc_class_ops cmos_rtc_ops_no_alarm = {
+	.read_time		= cmos_read_time,
+	.set_time		= cmos_set_time,
+	.proc			= cmos_procfs,
+};
+
 /*----------------------------------------------------------------*/
 
 /*
@@ -842,9 +847,12 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq)
 			dev_dbg(dev, "IRQ %d is already in use\n", rtc_irq);
 			goto cleanup1;
 		}
+
+		cmos_rtc.rtc->ops = &cmos_rtc_ops;
+	} else {
+		cmos_rtc.rtc->ops = &cmos_rtc_ops_no_alarm;
 	}
 
-	cmos_rtc.rtc->ops = &cmos_rtc_ops;
 	cmos_rtc.rtc->nvram_old_abi = true;
 	retval = rtc_register_device(cmos_rtc.rtc);
 	if (retval)
-- 
2.19.0.rc0