Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1262941 > unrolled thread

[PATCH v2] alarmtimer: fix unexpected rtc interrupt when system resume from S3

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system resume from S3 zhuo-hao.lee@intel.com - 2015-11-05 06:50 +0100
    RE: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system  resume from S3 "Lee, Zhuo-hao" <zhuo-hao.lee@intel.com> - 2015-11-09 03:30 +0100
    Re: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system  resume from S3 John Stultz <john.stultz@linaro.org> - 2015-11-13 21:40 +0100
      RE: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system  resume from S3 "Lee, Zhuo-hao" <zhuo-hao.lee@intel.com> - 2015-11-14 15:20 +0100

#1262941 — [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system resume from S3

From: zhuo-hao <zhuo-hao.lee@intel.com>

Before the system go to suspend (S3), if user create a timer with clockid
CLOCK_REALTIME_ALARM/CLOCK_BOOTTIME_ALARM and set a "large" timeout value
to this timer. The function alarmtimer_suspend will be called to setup
a timeout value to RTC timer to avoid the system sleep over time. However,
if the system wakeup early than RTC timeout, the RTC timer will not be cleared.
And this will cause the hpet_rtc_interrupt come unexpectedly until the RTC
timeout. To fix this problem, just adding alarmtimer_resume to cancel the
RTC timer.

Signed-off-by: Zhuo-hao Lee <zhuo-hao.lee@intel.com>
---
 kernel/time/alarmtimer.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 7fbba63..e840ed8 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -271,11 +271,27 @@ static int alarmtimer_suspend(struct device *dev)
 		__pm_wakeup_event(ws, MSEC_PER_SEC);
 	return ret;
 }
+
+static int alarmtimer_resume(struct device *dev)
+{
+	struct rtc_device *rtc;
+
+	rtc = alarmtimer_get_rtcdev();
+	if (rtc)
+		rtc_timer_cancel(rtc, &rtctimer);
+	return 0;
+}
+
 #else
 static int alarmtimer_suspend(struct device *dev)
 {
 	return 0;
 }
+
+static int alarmtimer_resume(struct device *dev)
+{
+	return 0;
+}
 #endif
 
 static void alarmtimer_freezerset(ktime_t absexp, enum alarmtimer_type type)
@@ -800,6 +816,7 @@ out:
 /* Suspend hook structures */
 static const struct dev_pm_ops alarmtimer_pm_ops = {
 	.suspend = alarmtimer_suspend,
+	.resume = alarmtimer_resume,
 };
 
 static struct platform_driver alarmtimer_driver = {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1265281 — RE: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system resume from S3

Hi tglx & all,

I wrote a simple program which can always hit this problem.
Please use gcc to compile the following code and then "./a.out 10000 1" and watch the /proc/interrupts, 
and you will see that the RTC interrupt will come unexpectedly after system resumed.
This abnormal interrupt should be removed. I tried on the latest linux kernel, 
the bug still exists. So, could you please help to review this patch? Or give me some feedback?


#include <stdio.h>
#include <sys/time.h>
#include <sys/types.h>
#include <fcntl.h>
#include <unistd.h>
#include <errno.h>
#include <sys/timerfd.h>
#include <time.h>
#include <stdlib.h>
#include <stdint.h>

#define handle_error(msg) \
            do { perror(msg); exit(EXIT_FAILURE); } while (0)

int main(int argc, char *argv[])
{
    struct itimerspec new_value;
    int fd, sec, enable_suspend;
    struct timespec now;
    uint64_t exp;
    ssize_t s;
    pid_t pid;

    if (argc != 3) {
        fprintf(stderr, "%s secs enable_suspend\n",argv[0]);
        exit(EXIT_FAILURE);
    }

    sec = atoi(argv[1]);
    enable_suspend = atoi(argv[2]);

    if (clock_gettime(CLOCK_REALTIME, &now) == -1)
        handle_error("clock_gettime");

    /* Create a CLOCK_REALTIME absolute timer with initial
       expiration and interval as specified in command line */

    new_value.it_value.tv_sec = now.tv_sec + sec;
    new_value.it_value.tv_nsec = now.tv_nsec;
    new_value.it_interval.tv_sec = 0;
    new_value.it_interval.tv_nsec = 0;

    fd = timerfd_create(CLOCK_REALTIME_ALARM, 0);
    if (fd == -1)
        handle_error("timerfd_create");

    if (timerfd_settime(fd, TFD_TIMER_ABSTIME, &new_value, NULL) == -1)
        handle_error("timerfd_settime");

    system("cat /proc/driver/rtc | grep rtc_time");
    system("cat /proc/interrupts | grep -E 'CPU0|rtc0'");
    printf("\n\ntimer started: wait %d seconds\n\n", sec);
    pid = fork();
    if (pid == -1) {
        fprintf(stderr, "fork failed\n");
        exit(EXIT_FAILURE);
    } else if (pid == 0) {
        if(enable_suspend){
            sleep(5);
/* for chromeOS */
 //           system("echo 0 > /sys/class/rtc/rtc0/wakealarm");
//            system("echo +10 > /sys/class/rtc/rtc0/wakealarm");
//            system("powerd_dbus_suspend"); 

/* for Ubuntu */
            system("sudo pm-suspend"); 
        }
        exit(EXIT_SUCCESS);
    } else {
        s = read(fd, &exp, sizeof(uint64_t));
        if (s != sizeof(uint64_t))
            handle_error("read");

        system("cat /proc/driver/rtc |grep rtc_time");
        system("cat /proc/interrupts | grep -E 'CPU0|rtc0'");
    
        exit(EXIT_SUCCESS);
    }
}


Thanks
Lee, Zhuo-hao

-----Original Message-----
From: Lee, Zhuo-hao 
Sent: Thursday, November 5, 2015 1:50 PM
To: linux-kernel@vger.kernel.org
Cc: Thomas Gleixner; zhuohao.lee82@gmail.com; Lee, Zhuo-hao
Subject: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system resume from S3

From: zhuo-hao <zhuo-hao.lee@intel.com>

Before the system go to suspend (S3), if user create a timer with clockid CLOCK_REALTIME_ALARM/CLOCK_BOOTTIME_ALARM and set a "large" timeout value to this timer. The function alarmtimer_suspend will be called to setup a timeout value to RTC timer to avoid the system sleep over time. However, if the system wakeup early than RTC timeout, the RTC timer will not be cleared.
And this will cause the hpet_rtc_interrupt come unexpectedly until the RTC timeout. To fix this problem, just adding alarmtimer_resume to cancel the RTC timer.

Signed-off-by: Zhuo-hao Lee <zhuo-hao.lee@intel.com>
---
 kernel/time/alarmtimer.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c index 7fbba63..e840ed8 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -271,11 +271,27 @@ static int alarmtimer_suspend(struct device *dev)
 		__pm_wakeup_event(ws, MSEC_PER_SEC);
 	return ret;
 }
+
+static int alarmtimer_resume(struct device *dev) {
+	struct rtc_device *rtc;
+
+	rtc = alarmtimer_get_rtcdev();
+	if (rtc)
+		rtc_timer_cancel(rtc, &rtctimer);
+	return 0;
+}
+
 #else
 static int alarmtimer_suspend(struct device *dev)  {
 	return 0;
 }
+
+static int alarmtimer_resume(struct device *dev) {
+	return 0;
+}
 #endif
 
 static void alarmtimer_freezerset(ktime_t absexp, enum alarmtimer_type type) @@ -800,6 +816,7 @@ out:
 /* Suspend hook structures */
 static const struct dev_pm_ops alarmtimer_pm_ops = {
 	.suspend = alarmtimer_suspend,
+	.resume = alarmtimer_resume,
 };
 
 static struct platform_driver alarmtimer_driver = {
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269208 — Re: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system resume from S3

On Wed, Nov 4, 2015 at 9:50 PM,  <zhuo-hao.lee@intel.com> wrote:
> From: zhuo-hao <zhuo-hao.lee@intel.com>
>

If you could, please CC me on future submissions? I need to add an
entry to MAINTAINERS for alarmtimers. :)

> Before the system go to suspend (S3), if user create a timer with clockid
> CLOCK_REALTIME_ALARM/CLOCK_BOOTTIME_ALARM and set a "large" timeout value
> to this timer. The function alarmtimer_suspend will be called to setup
> a timeout value to RTC timer to avoid the system sleep over time. However,
> if the system wakeup early than RTC timeout, the RTC timer will not be cleared.
> And this will cause the hpet_rtc_interrupt come unexpectedly until the RTC
> timeout. To fix this problem, just adding alarmtimer_resume to cancel the
> RTC timer.

So conceptually the patch makes sense, though I'm not totally sure I
understand the failure you describe.

We have some alarmtimer set for 2 hours from now.
We suspend, and the alarmtimer code sets an rtctimer to wake us up in
2 hours week.
We resume a few minutes later due to user interaction or other
wakeups, but the rtctimer is still set.
A little less then two hours later (while the system has been awake
the whole time), the RTC hardware fires and we run the rtctimer.
???? Something problematic here w/ the hpet_rtc_interrupt?
The alarmtimer's hrtimer fires as normal.


Again, your fix seems reasonable, but I also feel like when the RTC
hardware spuriously fires and we trigger the rtctimer logic, there
shouldn't be any real problems there. So I want to make sure we're not
covering up some underlying issue w/ this fix.

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1269486 — RE: [PATCH v2] alarmtimer: fix unexpected rtc interrupt when system resume from S3

PklmIHlvdSBjb3VsZCwgcGxlYXNlIENDIG1lIG9uIGZ1dHVyZSBzdWJtaXNzaW9ucz8gSSBuZWVk
IHRvIGFkZCBhbiBlbnRyeSB0byBNQUlOVEFJTkVSUyA+Zm9yIGFsYXJtdGltZXJzLiA6KQ0KDQpP
aywgSSB3aWxsIGRvIHRoYXQgb24gdGhlIGZ1dHVyZSBzdWJtaXNzaW9ucyA6KQ0KDQo+PiBCZWZv
cmUgdGhlIHN5c3RlbSBnbyB0byBzdXNwZW5kIChTMyksIGlmIHVzZXIgY3JlYXRlIGEgdGltZXIg
d2l0aCANCj4+IGNsb2NraWQgQ0xPQ0tfUkVBTFRJTUVfQUxBUk0vQ0xPQ0tfQk9PVFRJTUVfQUxB
Uk0gYW5kIHNldCBhICJsYXJnZSIgDQo+PiB0aW1lb3V0IHZhbHVlIHRvIHRoaXMgdGltZXIuIFRo
ZSBmdW5jdGlvbiBhbGFybXRpbWVyX3N1c3BlbmQgd2lsbCBiZSANCj4+IGNhbGxlZCB0byBzZXR1
cCBhIHRpbWVvdXQgdmFsdWUgdG8gUlRDIHRpbWVyIHRvIGF2b2lkIHRoZSBzeXN0ZW0gc2xlZXAg
DQo+PiBvdmVyIHRpbWUuIEhvd2V2ZXIsIGlmIHRoZSBzeXN0ZW0gd2FrZXVwIGVhcmx5IHRoYW4g
UlRDIHRpbWVvdXQsIHRoZSBSVEMgdGltZXIgd2lsbCBub3QgYmUgY2xlYXJlZC4NCj4+IEFuZCB0
aGlzIHdpbGwgY2F1c2UgdGhlIGhwZXRfcnRjX2ludGVycnVwdCBjb21lIHVuZXhwZWN0ZWRseSB1
bnRpbCB0aGUgDQo+PiBSVEMgdGltZW91dC4gVG8gZml4IHRoaXMgcHJvYmxlbSwganVzdCBhZGRp
bmcgYWxhcm10aW1lcl9yZXN1bWUgdG8gDQo+PiBjYW5jZWwgdGhlIFJUQyB0aW1lci4NCg0KPlNv
IGNvbmNlcHR1YWxseSB0aGUgcGF0Y2ggbWFrZXMgc2Vuc2UsIHRob3VnaCBJJ20gbm90IHRvdGFs
bHkgc3VyZSBJIHVuZGVyc3RhbmQgdGhlIGZhaWx1cmUgeW91IGRlc2NyaWJlLg0KDQpJIGhhZCBw
b3N0ZWQgYSBzbWFsbCBwcm9ncmFtIHdoaWNoIGFsd2F5cyBoaXQgdGhpcyBidWcgaWYgc3lzdGVt
IHdha2UgdXAgZWFybGllciB0aGFuIHNldHRpbmcgdGltZSwNCkRpZCB5b3UgcmVjZWl2ZSBpdD8N
Cmh0dHBzOi8vbGttbC5vcmcvbGttbC8yMDE1LzExLzgvMzI2DQoNCj5XZSBoYXZlIHNvbWUgYWxh
cm10aW1lciBzZXQgZm9yIDIgaG91cnMgZnJvbSBub3cuDQo+V2Ugc3VzcGVuZCwgYW5kIHRoZSBh
bGFybXRpbWVyIGNvZGUgc2V0cyBhbiBydGN0aW1lciB0byB3YWtlIHVzIHVwIGluDQo+MiBob3Vy
cyB3ZWVrLg0KPldlIHJlc3VtZSBhIGZldyBtaW51dGVzIGxhdGVyIGR1ZSB0byB1c2VyIGludGVy
YWN0aW9uIG9yIG90aGVyIHdha2V1cHMsIGJ1dCB0aGUgcnRjdGltZXIgaXMgc3RpbGwgc2V0Lg0K
PkEgbGl0dGxlIGxlc3MgdGhlbiB0d28gaG91cnMgbGF0ZXIgKHdoaWxlIHRoZSBzeXN0ZW0gaGFz
IGJlZW4gYXdha2UgdGhlIHdob2xlIHRpbWUpLCB0aGUgUlRDIGhhcmR3YXJlIGZpcmVzIGFuZCB3
ZSBydW4gdGhlIHJ0Y3RpbWVyLg0KPj8/Pz8gU29tZXRoaW5nIHByb2JsZW1hdGljIGhlcmUgdy8g
dGhlIGhwZXRfcnRjX2ludGVycnVwdD8NCg0KWWVzLiANCklmIGFwcGxpY2F0aW9uIHVzZSB0aW1l
cmZkX2NyZWF0ZShDTE9DS19SRUFMVElNRV9BTEFSTSwgMCkgdG8gY3JlYXRlIGEgdGltZXIsIGFu
ZCB1c2UgdGltZXJmZF9zZXR0aW1lKCkgdG8gc2V0IDIgaG91cnMgdGltZXIuIEFuZCB0aGVuLCB0
aGUgdXNlciBqdXN0IHRyaWdnZXIgdGhlIHN5c3RlbSBnbyB0byBzdXNwZW5kLCB0aGlzIGJ1ZyB3
aWxsIGJlIGhpdC4gDQpCZWZvcmUgdGhlIHN5c3RlbSBnbyB0byBzdXNwZW5kLCBhbGFybXRpbWVy
IHdpbGwgc2V0ICJjdXJyZW50IHRpbWUgKyAyaHIiIHRvIHJ0YyB0aW1lciB0byBhdm9pZCB0aGUg
c3lzdGVtIHNsZWVwIG92ZXIgdGltZS4gSWYgdGhlIHN5c3RlbSB3YWtlIHVwIGVhcmxpZXIgdGhh
biAyIGhvdXJzIChmb3IgZXhhbXBsZSwgdXNlciBwcmVzcyB0aGUgd2FrZSB1cCBrZXkpLCB0aGUg
aHBldF9ydGNfaW50ZXJydXB0IHdpbGwgYmUgZmlyZWQgY29udGludW91c2x5IHVudGlsICJjdXJy
ZW50IHRpbWUgKyAyaHIiIHJlYWNoZWQuIFRoaXMgYWJub3JtYWwgaW50ZXJydXB0IHdpbGwgY29z
dCBzb21lIHN5c3RlbSBwZXJmb3JtYW5jZSBhbmQgc2hvdWxkIGJlIGF2b2lkZWQuDQoNCj5UaGUg
YWxhcm10aW1lcidzIGhydGltZXIgZmlyZXMgYXMgbm9ybWFsLg0KDQo+QWdhaW4sIHlvdXIgZml4
IHNlZW1zIHJlYXNvbmFibGUsIGJ1dCBJIGFsc28gZmVlbCBsaWtlIHdoZW4gdGhlIFJUQyBoYXJk
d2FyZSBzcHVyaW91c2x5IGZpcmVzIGFuZCB3ZSB0cmlnZ2VyIHRoZSBydGN0aW1lciA+bG9naWMs
IHRoZXJlIHNob3VsZG4ndCBiZSBhbnkgcmVhbCBwcm9ibGVtcyB0aGVyZS4gU28gSSB3YW50IHRv
IG1ha2Ugc3VyZSB3ZSdyZSBub3QgY292ZXJpbmcgdXAgc29tZSB1bmRlcmx5aW5nIGlzc3VlIHcv
ID50aGlzIGZpeC4NCg0KQWdyZWUsIHRoZXJlIGhhdmUgdHdvIHByb2JsZW1zIG9uIHRoaXMgYnVn
Og0KKDEpLiBhbGFybXRpbWVyIGNyZWF0ZSBhIHJ0YyB3YWtlIHVwIHRpbWVyIGhvd2V2ZXIgYWxh
cm10aW1lciB3b24ndCByZW1vdmUgdGhhdCB0aW1lciBpZiB0aGUgc3lzdGVtIHdha2UgdXAgZWFy
bGllcg0KKDIpLiBydGMgd2FrZSB1cCB0aW1lciB3aWxsIHRyaWdnZXIgaHBldF9ydGNfaW50ZXJy
dXB0IGNvbnRpbnVvdXNseSB1bnRpbCB0aW1lciB0aW1lb3V0Lg0KVGhpcyBwYXRjaCBvbmx5IGZp
eGVkICgxKS4gRml4aW5nICgxKSBjYW4gYXZvaWQgKDIpLg0KSG93ZXZlciwgVGhlICgyKSBpcyBh
bm90aGVyIHN0b3J5IHdoaWNoIGl0IGlzIG5vdCBjb3ZlcmVkIGJ5IHRoaXMgcGF0Y2guDQoNCg0K
VGhhbmtzDQpMZWUsIFpodW8taGFvDQoNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web