diff mbox

UBUNTU: SAUCE: drivers/rtc/interface.c: ignore exprired times when enqueing new timers (LP: #1333569)

Message ID 1420548537-1537-2-git-send-email-colin.king@canonical.com
State New
Headers show

Commit Message

Colin Ian King Jan. 6, 2015, 12:48 p.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

The current enqueuing does not trigger an alarm if any expired timers
already exist on the timerqueue. This can occur when a RTC wake alarm
is used to wake a machine out of hibernate and the resumed state has
old expired timers that have not been removed from the timer queue.
This fix skips over any expired timers and triggers an alarm if there
are no pending timers on the timerqueue.

The bug was found running the example RTC timer program from
Documentation/rtc.txt; it runs fine before a hibernate but will block
forever on RTC reads after a resume from a hibernate that is woken
up using a RTC wakealarm.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/rtc/interface.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Colin Ian King Jan. 6, 2015, 2:05 p.m. UTC | #1 
On 06/01/15 12:48, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The current enqueuing does not trigger an alarm if any expired timers
> already exist on the timerqueue. This can occur when a RTC wake alarm
> is used to wake a machine out of hibernate and the resumed state has
> old expired timers that have not been removed from the timer queue.
> This fix skips over any expired timers and triggers an alarm if there
> are no pending timers on the timerqueue.
> 
> The bug was found running the example RTC timer program from
> Documentation/rtc.txt; it runs fine before a hibernate but will block
> forever on RTC reads after a resume from a hibernate that is woken
> up using a RTC wakealarm.

BugLink: http://bugs.launchpad.net/bugs/1333569

> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/rtc/interface.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index 5b2717f..ca9d9fc 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -780,9 +780,23 @@ EXPORT_SYMBOL_GPL(rtc_irq_set_freq);
>   */
>  static int rtc_timer_enqueue(struct rtc_device *rtc, struct rtc_timer *timer)
>  {
> +	struct timerqueue_node *next = timerqueue_getnext(&rtc->timerqueue);
> +	struct rtc_time tm;
> +	ktime_t now;
> +
>  	timer->enabled = 1;
> +	__rtc_read_time(rtc, &tm);
> +	now = rtc_tm_to_ktime(tm);
> +
> +	/* Skip over expired timers */
> +	while (next) {
> +		if (next->expires.tv64 >= now.tv64)
> +			break;
> +		next = timerqueue_iterate_next(next);
> +	}
> +
>  	timerqueue_add(&rtc->timerqueue, &timer->node);
> -	if (&timer->node == timerqueue_getnext(&rtc->timerqueue)) {
> +	if (!next) {
>  		struct rtc_wkalrm alarm;
>  		int err;
>  		alarm.time = rtc_ktime_to_tm(timer->node.expires);
>
Andy Whitcroft Jan. 6, 2015, 4:23 p.m. UTC | #2 
On Tue, Jan 06, 2015 at 12:48:57PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The current enqueuing does not trigger an alarm if any expired timers
> already exist on the timerqueue. This can occur when a RTC wake alarm
> is used to wake a machine out of hibernate and the resumed state has
> old expired timers that have not been removed from the timer queue.
> This fix skips over any expired timers and triggers an alarm if there
> are no pending timers on the timerqueue.
> 
> The bug was found running the example RTC timer program from
> Documentation/rtc.txt; it runs fine before a hibernate but will block
> forever on RTC reads after a resume from a hibernate that is woken
> up using a RTC wakealarm.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/rtc/interface.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index 5b2717f..ca9d9fc 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -780,9 +780,23 @@ EXPORT_SYMBOL_GPL(rtc_irq_set_freq);
>   */
>  static int rtc_timer_enqueue(struct rtc_device *rtc, struct rtc_timer *timer)
>  {
> +	struct timerqueue_node *next = timerqueue_getnext(&rtc->timerqueue);
> +	struct rtc_time tm;
> +	ktime_t now;
> +
>  	timer->enabled = 1;
> +	__rtc_read_time(rtc, &tm);
> +	now = rtc_tm_to_ktime(tm);
> +
> +	/* Skip over expired timers */
> +	while (next) {
> +		if (next->expires.tv64 >= now.tv64)
> +			break;
> +		next = timerqueue_iterate_next(next);
> +	}
> +
>  	timerqueue_add(&rtc->timerqueue, &timer->node);
> -	if (&timer->node == timerqueue_getnext(&rtc->timerqueue)) {
> +	if (!next) {
>  		struct rtc_wkalrm alarm;
>  		int err;
>  		alarm.time = rtc_ktime_to_tm(timer->node.expires);
> -- 

Seems reasonable.  You might submit this again, copying akpm if it is
being ignored.  He is good at asking the right people.

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
Chris J Arges Jan. 6, 2015, 4:58 p.m. UTC | #3 
On 01/06/2015 06:48 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The current enqueuing does not trigger an alarm if any expired timers
> already exist on the timerqueue. This can occur when a RTC wake alarm
> is used to wake a machine out of hibernate and the resumed state has
> old expired timers that have not been removed from the timer queue.
> This fix skips over any expired timers and triggers an alarm if there
> are no pending timers on the timerqueue.
> 
> The bug was found running the example RTC timer program from
> Documentation/rtc.txt; it runs fine before a hibernate but will block
> forever on RTC reads after a resume from a hibernate that is woken
> up using a RTC wakealarm.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/rtc/interface.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> index 5b2717f..ca9d9fc 100644
> --- a/drivers/rtc/interface.c
> +++ b/drivers/rtc/interface.c
> @@ -780,9 +780,23 @@ EXPORT_SYMBOL_GPL(rtc_irq_set_freq);
>   */
>  static int rtc_timer_enqueue(struct rtc_device *rtc, struct rtc_timer *timer)
>  {
> +	struct timerqueue_node *next = timerqueue_getnext(&rtc->timerqueue);
> +	struct rtc_time tm;
> +	ktime_t now;
> +
>  	timer->enabled = 1;
> +	__rtc_read_time(rtc, &tm);
> +	now = rtc_tm_to_ktime(tm);
> +
> +	/* Skip over expired timers */
> +	while (next) {
> +		if (next->expires.tv64 >= now.tv64)
> +			break;
> +		next = timerqueue_iterate_next(next);
> +	}
> +
>  	timerqueue_add(&rtc->timerqueue, &timer->node);
> -	if (&timer->node == timerqueue_getnext(&rtc->timerqueue)) {
> +	if (!next) {
>  		struct rtc_wkalrm alarm;
>  		int err;
>  		alarm.time = rtc_ktime_to_tm(timer->node.expires);
> 

Looks reasonable, and ops_lock should be already held when calling
rtc_timer_enqueue.

Acked-by: Chris J Arges <chris.j.arges@canonical.com>
diff mbox

Patch

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index 5b2717f..ca9d9fc 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -780,9 +780,23 @@  EXPORT_SYMBOL_GPL(rtc_irq_set_freq);
  */
 static int rtc_timer_enqueue(struct rtc_device *rtc, struct rtc_timer *timer)
 {
+	struct timerqueue_node *next = timerqueue_getnext(&rtc->timerqueue);
+	struct rtc_time tm;
+	ktime_t now;
+
 	timer->enabled = 1;
+	__rtc_read_time(rtc, &tm);
+	now = rtc_tm_to_ktime(tm);
+
+	/* Skip over expired timers */
+	while (next) {
+		if (next->expires.tv64 >= now.tv64)
+			break;
+		next = timerqueue_iterate_next(next);
+	}
+
 	timerqueue_add(&rtc->timerqueue, &timer->node);
-	if (&timer->node == timerqueue_getnext(&rtc->timerqueue)) {
+	if (!next) {
 		struct rtc_wkalrm alarm;
 		int err;
 		alarm.time = rtc_ktime_to_tm(timer->node.expires);