diff mbox

[Regression,Revert,request] Excessive delay or hang during resume from system suspend due to a hrtimer commit

Message ID alpine.LFD.2.02.1207161035500.32033@ionos (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Thomas Gleixner July 16, 2012, 9:47 a.m. UTC
On Sun, 15 Jul 2012, Rafael J. Wysocki wrote:
> To everyone involved: the fact that this change, which was likely to introduce
> regressions from the look of it alone, has been pushed to Linus (an to -stable
> at the same time!) so late in the cycle, is seriuosly disappointing.

Well, we spent an massive amount of time in testing, reviewing and
discussion and it definitely did not break suspend/resume here.

This was not pushed without a lot of thoughts and in fact what you are
seing is another long standing bug in the timekeeping resume code,
which was just papered over by the incorrect handling of the clock was
set cases in the other parts of the system.

Does the following patch fix the problem for you ?

@John: Should that clear ntp as well or is it enough to set ntp_error
       to 0 ?

/me really goes on vacation now.

Thanks,

	tglx

---------

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

Comments

Thomas Gleixner July 16, 2012, 11:15 a.m. UTC | #1
On Mon, 16 Jul 2012, Rafael J. Wysocki wrote:

> On Monday, July 16, 2012, Thomas Gleixner wrote:
> > On Sun, 15 Jul 2012, Rafael J. Wysocki wrote:
> > > To everyone involved: the fact that this change, which was likely to introduce
> > > regressions from the look of it alone, has been pushed to Linus (an to -stable
> > > at the same time!) so late in the cycle, is seriuosly disappointing.
> > 
> > Well, we spent an massive amount of time in testing, reviewing and
> > discussion and it definitely did not break suspend/resume here.
> 
> I'm not saying that you didn't consider it thoroughly, but unfortunately you
> did overlook this particular issue, didn't you?
> 
> > This was not pushed without a lot of thoughts and in fact what you are
> > seing is another long standing bug in the timekeeping resume code,
> > which was just papered over by the incorrect handling of the clock was
> > set cases in the other parts of the system.
> > 
> > Does the following patch fix the problem for you ?
> 
> Yes, it does, thanks!
> 
> > @John: Should that clear ntp as well or is it enough to set ntp_error
> >        to 0 ?
> > 
> > /me really goes on vacation now.
> 
> So who's going to take care of the patch? :-)

I'm still packing gear. So i'll push it into timers/urgent.

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael Wysocki July 16, 2012, 11:16 a.m. UTC | #2
On Monday, July 16, 2012, Thomas Gleixner wrote:
> On Sun, 15 Jul 2012, Rafael J. Wysocki wrote:
> > To everyone involved: the fact that this change, which was likely to introduce
> > regressions from the look of it alone, has been pushed to Linus (an to -stable
> > at the same time!) so late in the cycle, is seriuosly disappointing.
> 
> Well, we spent an massive amount of time in testing, reviewing and
> discussion and it definitely did not break suspend/resume here.

I'm not saying that you didn't consider it thoroughly, but unfortunately you
did overlook this particular issue, didn't you?

> This was not pushed without a lot of thoughts and in fact what you are
> seing is another long standing bug in the timekeeping resume code,
> which was just papered over by the incorrect handling of the clock was
> set cases in the other parts of the system.
> 
> Does the following patch fix the problem for you ?

Yes, it does, thanks!

> @John: Should that clear ntp as well or is it enough to set ntp_error
>        to 0 ?
> 
> /me really goes on vacation now.

So who's going to take care of the patch? :-)

Rafael


> ---------
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index 269b1fe..3447cfa 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -717,6 +717,7 @@ static void timekeeping_resume(void)
>  	timekeeper.clock->cycle_last = timekeeper.clock->read(timekeeper.clock);
>  	timekeeper.ntp_error = 0;
>  	timekeeping_suspended = 0;
> +	timekeeping_update(false);
>  	write_sequnlock_irqrestore(&timekeeper.lock, flags);
>  
>  	touch_softlockup_watchdog();
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Schwab July 16, 2012, 12:48 p.m. UTC | #3
"Rafael J. Wysocki" <rjw@sisk.pl> writes:

> On Monday, July 16, 2012, Thomas Gleixner wrote:
>> Does the following patch fix the problem for you ?
>
> Yes, it does, thanks!

Works for me as well.

Andreas.
diff mbox

Patch

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 269b1fe..3447cfa 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -717,6 +717,7 @@  static void timekeeping_resume(void)
 	timekeeper.clock->cycle_last = timekeeper.clock->read(timekeeper.clock);
 	timekeeper.ntp_error = 0;
 	timekeeping_suspended = 0;
+	timekeeping_update(false);
 	write_sequnlock_irqrestore(&timekeeper.lock, flags);
 
 	touch_softlockup_watchdog();