Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-24 Thread Guenter Roeck
On Thu, Apr 25, 2013 at 09:38:22AM +0800, Li Zefan wrote:
> On 2013/4/25 6:42, Guenter Roeck wrote:
> > On Mon, Apr 08, 2013 at 04:34:26PM -0400, Prarit Bhargava wrote:
> >>
> >>
> >> On 04/08/2013 04:19 PM, John Stultz wrote:
> >>> On 04/08/2013 05:47 AM, Prarit Bhargava wrote:
> >>
> 
>  A simple check for an overflow can resolve this problem.  Using KTIME_MAX
>  instead of the overflow value will result in the hrtimer function being 
>  run,
>  and the reprogramming of the timer after that.
> 
>  Signed-off-by: Prarit Bhargava 
>  Cc: Thomas Gleixner 
>  Cc: John Stultz 
> >>>
> >>> Prarit: Should this be tagged for -stable?
> >>
> >> John,
> >>
> >> Yes, this should go to -stable.  cc'd.
> >>
> > Hi,
> > 
> > I am a bit surprised that this patch has not found its way into mainline 
> > yet,
> > as everyone seems to agree that it is a candidate for -stable.
> > 
> > I hit this problem very reliably (ie with each boot) with 3.8.x on systems
> > which have no RTC and run systemd. Seen with Freescale P5040 as well as
> > a Broadcom MIPS based system.
> > 
> 
> FYI, we also hit this warning with 3.4-rt.
> 
You are lucky if it is just a warning for you. In my case the system is
reliably dead. Guess there are not (yet) many users out there using
systemd (or something similar) on a system with no RTC.

While I am not too happy about the delay to get the patch integrated,
I am glad that Prarit found and fixed the problem. Saved me a lot of time.

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


Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-24 Thread Li Zefan
On 2013/4/25 6:42, Guenter Roeck wrote:
> On Mon, Apr 08, 2013 at 04:34:26PM -0400, Prarit Bhargava wrote:
>>
>>
>> On 04/08/2013 04:19 PM, John Stultz wrote:
>>> On 04/08/2013 05:47 AM, Prarit Bhargava wrote:
>>

 A simple check for an overflow can resolve this problem.  Using KTIME_MAX
 instead of the overflow value will result in the hrtimer function being 
 run,
 and the reprogramming of the timer after that.

 Signed-off-by: Prarit Bhargava 
 Cc: Thomas Gleixner 
 Cc: John Stultz 
>>>
>>> Prarit: Should this be tagged for -stable?
>>
>> John,
>>
>> Yes, this should go to -stable.  cc'd.
>>
> Hi,
> 
> I am a bit surprised that this patch has not found its way into mainline yet,
> as everyone seems to agree that it is a candidate for -stable.
> 
> I hit this problem very reliably (ie with each boot) with 3.8.x on systems
> which have no RTC and run systemd. Seen with Freescale P5040 as well as
> a Broadcom MIPS based system.
> 

FYI, we also hit this warning with 3.4-rt.

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


Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-24 Thread John Stultz

On 04/24/2013 05:35 PM, Guenter Roeck wrote:

On Wed, Apr 24, 2013 at 05:05:03PM -0700, John Stultz wrote:

On 04/24/2013 03:42 PM, Guenter Roeck wrote:

On Mon, Apr 08, 2013 at 04:34:26PM -0400, Prarit Bhargava wrote:

On 04/08/2013 04:19 PM, John Stultz wrote:

On 04/08/2013 05:47 AM, Prarit Bhargava wrote:

A simple check for an overflow can resolve this problem.  Using KTIME_MAX
instead of the overflow value will result in the hrtimer function being run,
and the reprogramming of the timer after that.

Signed-off-by: Prarit Bhargava 
Cc: Thomas Gleixner 
Cc: John Stultz 

Prarit: Should this be tagged for -stable?

John,

Yes, this should go to -stable.  cc'd.


Hi,

I am a bit surprised that this patch has not found its way into mainline yet,
as everyone seems to agree that it is a candidate for -stable.

It just has to land upstream first, which is likely in the next week
or so when the 3.10 merge window opens. I'd have thought it would be
sooner but 3.9 is taking longer to close then I expected (and I
didn't think it was urgent enough to drop in at the last minute
before the 3.9 release was made).


Guess I am a bit lost in process.

If this is going to be in -stable, it will presumably end up in 3.9.x as well as
in earlier releases. So why wasn't it pushed into 3.9-rcX to start with ?


I usually only want to push changes to -rc6+ if they are really 
critical, affecting lots of folks and fixing issues introduced in the 
same cycle. By getting less critical fixes merged during a normal merge 
window, then backporting them to affected -stable trees, we get better 
test coverage and less chance for further bugs to be introduced at the 
last minute before the release is made.


Its maybe a bit overly conservative, but I'm less and less into 
late-night heroics these days. ;)


thanks
-john


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


Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-24 Thread Guenter Roeck
On Wed, Apr 24, 2013 at 05:05:03PM -0700, John Stultz wrote:
> On 04/24/2013 03:42 PM, Guenter Roeck wrote:
> >On Mon, Apr 08, 2013 at 04:34:26PM -0400, Prarit Bhargava wrote:
> >>
> >>On 04/08/2013 04:19 PM, John Stultz wrote:
> >>>On 04/08/2013 05:47 AM, Prarit Bhargava wrote:
> A simple check for an overflow can resolve this problem.  Using KTIME_MAX
> instead of the overflow value will result in the hrtimer function being 
> run,
> and the reprogramming of the timer after that.
> 
> Signed-off-by: Prarit Bhargava 
> Cc: Thomas Gleixner 
> Cc: John Stultz 
> >>>Prarit: Should this be tagged for -stable?
> >>John,
> >>
> >>Yes, this should go to -stable.  cc'd.
> >>
> >Hi,
> >
> >I am a bit surprised that this patch has not found its way into mainline yet,
> >as everyone seems to agree that it is a candidate for -stable.
> 
> It just has to land upstream first, which is likely in the next week
> or so when the 3.10 merge window opens. I'd have thought it would be
> sooner but 3.9 is taking longer to close then I expected (and I
> didn't think it was urgent enough to drop in at the last minute
> before the 3.9 release was made).
> 
Guess I am a bit lost in process.

If this is going to be in -stable, it will presumably end up in 3.9.x as well as
in earlier releases. So why wasn't it pushed into 3.9-rcX to start with ?

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


Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-24 Thread John Stultz

On 04/24/2013 03:42 PM, Guenter Roeck wrote:

On Mon, Apr 08, 2013 at 04:34:26PM -0400, Prarit Bhargava wrote:


On 04/08/2013 04:19 PM, John Stultz wrote:

On 04/08/2013 05:47 AM, Prarit Bhargava wrote:

A simple check for an overflow can resolve this problem.  Using KTIME_MAX
instead of the overflow value will result in the hrtimer function being run,
and the reprogramming of the timer after that.

Signed-off-by: Prarit Bhargava 
Cc: Thomas Gleixner 
Cc: John Stultz 

Prarit: Should this be tagged for -stable?

John,

Yes, this should go to -stable.  cc'd.


Hi,

I am a bit surprised that this patch has not found its way into mainline yet,
as everyone seems to agree that it is a candidate for -stable.


It just has to land upstream first, which is likely in the next week or 
so when the 3.10 merge window opens. I'd have thought it would be sooner 
but 3.9 is taking longer to close then I expected (and I didn't think it 
was urgent enough to drop in at the last minute before the 3.9 release 
was made).


thanks
-john

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


Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-24 Thread Guenter Roeck
On Mon, Apr 08, 2013 at 04:34:26PM -0400, Prarit Bhargava wrote:
> 
> 
> On 04/08/2013 04:19 PM, John Stultz wrote:
> > On 04/08/2013 05:47 AM, Prarit Bhargava wrote:
> 
> >>
> >> A simple check for an overflow can resolve this problem.  Using KTIME_MAX
> >> instead of the overflow value will result in the hrtimer function being 
> >> run,
> >> and the reprogramming of the timer after that.
> >>
> >> Signed-off-by: Prarit Bhargava 
> >> Cc: Thomas Gleixner 
> >> Cc: John Stultz 
> > 
> > Prarit: Should this be tagged for -stable?
> 
> John,
> 
> Yes, this should go to -stable.  cc'd.
> 
Hi,

I am a bit surprised that this patch has not found its way into mainline yet,
as everyone seems to agree that it is a candidate for -stable.

I hit this problem very reliably (ie with each boot) with 3.8.x on systems
which have no RTC and run systemd. Seen with Freescale P5040 as well as
a Broadcom MIPS based system.

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


Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-10 Thread Guenter Roeck
On Mon, Apr 08, 2013 at 01:19:23PM -0700, John Stultz wrote:
> On 04/08/2013 05:47 AM, Prarit Bhargava wrote:
> >2nd submit: I did quite a bit of testing with this patch and I don't see any
> >ill effects.  I've tested across several large and small x86 systems, and a
> >ppc system for good measure.
> >
> >P.
> >
> >8<---
> >`
> >The settimeofday01 test in the LTP testsuite effectively does
> >
> > gettimeofday(current time);
> > settimeofday(Jan 1, 1970 + 100 seconds);
> > settimeofday(current time);
> >
> >This test causes a stack trace to be displayed on the console during the
> >setting of timeofday to Jan 1, 1970 + 100 seconds:
> >
> >[  131.066751] [ cut here ]
> >[  131.096448] WARNING: at kernel/time/clockevents.c:209 
> >clockevents_program_event+0x135/0x140()
> >[  131.104935] Hardware name: Dinar
> >[  131.108150] Modules linked in: sg nfsv3 nfs_acl nfsv4 auth_rpcgss nfs 
> >dns_resolver fscache lockd sunrpc nf_conntrack_netbios_ns 
> >nf_conntrack_broadcast ipt_MASQUERADE ip6table_mangle ip6t_REJECT 
> >nf_conntrack_ipv6 nf_defrag_ipv6 iptable_nat nf_nat_ipv4 nf_nat 
> >iptable_mangle ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack 
> >nf_conntrack ebtable_filter ebtables ip6table_filter ip6_tables 
> >iptable_filter ip_tables kvm_amd kvm sp5100_tco bnx2 i2c_piix4 crc32c_intel 
> >k10temp fam15h_power ghash_clmulni_intel amd64_edac_mod pcspkr serio_raw 
> >edac_mce_amd edac_core microcode xfs libcrc32c sr_mod sd_mod cdrom 
> >ata_generic crc_t10dif pata_acpi radeon i2c_algo_bit drm_kms_helper ttm drm 
> >ahci pata_atiixp libahci libata usb_storage i2c_core dm_mirror 
> >dm_region_hash dm_log dm_mod
> >[  131.176784] Pid: 0, comm: swapper/28 Not tainted 3.8.0+ #6
> >[  131.182248] Call Trace:
> >[  131.184684][] warn_slowpath_common+0x7f/0xc0
> >[  131.191312]  [] warn_slowpath_null+0x1a/0x20
> >[  131.197131]  [] clockevents_program_event+0x135/0x140
> >[  131.203721]  [] tick_program_event+0x24/0x30
> >[  131.209534]  [] hrtimer_interrupt+0x131/0x230
> >[  131.215437]  [] ? cpufreq_p4_target+0x130/0x130
> >[  131.221509]  [] smp_apic_timer_interrupt+0x69/0x99
> >[  131.227839]  [] apic_timer_interrupt+0x6d/0x80
> >[  131.233816][] ? sched_clock_cpu+0xc5/0x120
> >[  131.240267]  [] ? cpuidle_wrap_enter+0x50/0xa0
> >[  131.246252]  [] ? cpuidle_wrap_enter+0x49/0xa0
> >[  131.252238]  [] cpuidle_enter_tk+0x10/0x20
> >[  131.257877]  [] cpuidle_idle_call+0xa9/0x260
> >[  131.263692]  [] cpu_idle+0xaf/0x120
> >[  131.268727]  [] start_secondary+0x255/0x257
> >[  131.274449] ---[ end trace 1151a50552231615 ]---
> >
> >When we change the system time to a low value like this, the value of
> >timekeeper->offs_real will be a negative value.
> >
> >It seems that the WARN occurs because an hrtimer has been started in the time
> >between the releasing of the timekeeper lock and the IPI call (via a call to
> >on_each_cpu) in clock_was_set() in the do_settimeofday() code.  The end 
> >result
> >is that a REALTIME_CLOCK timer has been added with softexpires = expires =
> >KTIME_MAX.  The hrtimer_interrupt() fires/is called and the loop at
> >kernel/hrtimer.c:1289 is executed.  In this loop the code subtracts the
> >clock base's offset (which was set to timekeeper->offs_real in
> >do_settimeofday()) from the current hrtimer_cpu_base->expiry value (which
> >was KTIME_MAX):
> >
> > KTIME_MAX - (a negative value) = overflow
> >
> >A simple check for an overflow can resolve this problem.  Using KTIME_MAX
> >instead of the overflow value will result in the hrtimer function being run,
> >and the reprogramming of the timer after that.
> >
> >Signed-off-by: Prarit Bhargava 
> >Cc: Thomas Gleixner 
> >Cc: John Stultz 
> 
> Prarit: Should this be tagged for -stable?
> 
Please. I am hitting this problem with 3.8.6 on powerpc. The patch fixes it for
me.

Tested-by: Guenter Roeck 

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


Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-08 Thread John Stultz

On 04/08/2013 01:34 PM, Prarit Bhargava wrote:


On 04/08/2013 04:19 PM, John Stultz wrote:

On 04/08/2013 05:47 AM, Prarit Bhargava wrote:

A simple check for an overflow can resolve this problem.  Using KTIME_MAX
instead of the overflow value will result in the hrtimer function being run,
and the reprogramming of the timer after that.

Signed-off-by: Prarit Bhargava 
Cc: Thomas Gleixner 
Cc: John Stultz 

Prarit: Should this be tagged for -stable?

John,

Yes, this should go to -stable.  cc'd.

Also, added the cc to the commit.

thanks
-john

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


Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-08 Thread Prarit Bhargava


On 04/08/2013 04:19 PM, John Stultz wrote:
> On 04/08/2013 05:47 AM, Prarit Bhargava wrote:

>>
>> A simple check for an overflow can resolve this problem.  Using KTIME_MAX
>> instead of the overflow value will result in the hrtimer function being run,
>> and the reprogramming of the timer after that.
>>
>> Signed-off-by: Prarit Bhargava 
>> Cc: Thomas Gleixner 
>> Cc: John Stultz 
> 
> Prarit: Should this be tagged for -stable?

John,

Yes, this should go to -stable.  cc'd.

P.

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


Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-08 Thread John Stultz

On 04/08/2013 05:47 AM, Prarit Bhargava wrote:

2nd submit: I did quite a bit of testing with this patch and I don't see any
ill effects.  I've tested across several large and small x86 systems, and a
ppc system for good measure.

P.

8<---
`
The settimeofday01 test in the LTP testsuite effectively does

 gettimeofday(current time);
 settimeofday(Jan 1, 1970 + 100 seconds);
 settimeofday(current time);

This test causes a stack trace to be displayed on the console during the
setting of timeofday to Jan 1, 1970 + 100 seconds:

[  131.066751] [ cut here ]
[  131.096448] WARNING: at kernel/time/clockevents.c:209 
clockevents_program_event+0x135/0x140()
[  131.104935] Hardware name: Dinar
[  131.108150] Modules linked in: sg nfsv3 nfs_acl nfsv4 auth_rpcgss nfs 
dns_resolver fscache lockd sunrpc nf_conntrack_netbios_ns 
nf_conntrack_broadcast ipt_MASQUERADE ip6table_mangle ip6t_REJECT 
nf_conntrack_ipv6 nf_defrag_ipv6 iptable_nat nf_nat_ipv4 nf_nat iptable_mangle 
ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack 
ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter ip_tables 
kvm_amd kvm sp5100_tco bnx2 i2c_piix4 crc32c_intel k10temp fam15h_power 
ghash_clmulni_intel amd64_edac_mod pcspkr serio_raw edac_mce_amd edac_core 
microcode xfs libcrc32c sr_mod sd_mod cdrom ata_generic crc_t10dif pata_acpi 
radeon i2c_algo_bit drm_kms_helper ttm drm ahci pata_atiixp libahci libata 
usb_storage i2c_core dm_mirror dm_region_hash dm_log dm_mod
[  131.176784] Pid: 0, comm: swapper/28 Not tainted 3.8.0+ #6
[  131.182248] Call Trace:
[  131.184684][] warn_slowpath_common+0x7f/0xc0
[  131.191312]  [] warn_slowpath_null+0x1a/0x20
[  131.197131]  [] clockevents_program_event+0x135/0x140
[  131.203721]  [] tick_program_event+0x24/0x30
[  131.209534]  [] hrtimer_interrupt+0x131/0x230
[  131.215437]  [] ? cpufreq_p4_target+0x130/0x130
[  131.221509]  [] smp_apic_timer_interrupt+0x69/0x99
[  131.227839]  [] apic_timer_interrupt+0x6d/0x80
[  131.233816][] ? sched_clock_cpu+0xc5/0x120
[  131.240267]  [] ? cpuidle_wrap_enter+0x50/0xa0
[  131.246252]  [] ? cpuidle_wrap_enter+0x49/0xa0
[  131.252238]  [] cpuidle_enter_tk+0x10/0x20
[  131.257877]  [] cpuidle_idle_call+0xa9/0x260
[  131.263692]  [] cpu_idle+0xaf/0x120
[  131.268727]  [] start_secondary+0x255/0x257
[  131.274449] ---[ end trace 1151a50552231615 ]---

When we change the system time to a low value like this, the value of
timekeeper->offs_real will be a negative value.

It seems that the WARN occurs because an hrtimer has been started in the time
between the releasing of the timekeeper lock and the IPI call (via a call to
on_each_cpu) in clock_was_set() in the do_settimeofday() code.  The end result
is that a REALTIME_CLOCK timer has been added with softexpires = expires =
KTIME_MAX.  The hrtimer_interrupt() fires/is called and the loop at
kernel/hrtimer.c:1289 is executed.  In this loop the code subtracts the
clock base's offset (which was set to timekeeper->offs_real in
do_settimeofday()) from the current hrtimer_cpu_base->expiry value (which
was KTIME_MAX):

KTIME_MAX - (a negative value) = overflow

A simple check for an overflow can resolve this problem.  Using KTIME_MAX
instead of the overflow value will result in the hrtimer function being run,
and the reprogramming of the timer after that.

Signed-off-by: Prarit Bhargava 
Cc: Thomas Gleixner 
Cc: John Stultz 


Prarit: Should this be tagged for -stable?

thanks
-john

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


Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-08 Thread John Stultz

On 04/08/2013 07:53 AM, Rik van Riel wrote:

On 04/08/2013 08:47 AM, Prarit Bhargava wrote:


When we change the system time to a low value like this, the value of
timekeeper->offs_real will be a negative value.

It seems that the WARN occurs because an hrtimer has been started in 
the time
between the releasing of the timekeeper lock and the IPI call (via a 
call to
on_each_cpu) in clock_was_set() in the do_settimeofday() code. The 
end result
is that a REALTIME_CLOCK timer has been added with softexpires = 
expires =

KTIME_MAX.  The hrtimer_interrupt() fires/is called and the loop at
kernel/hrtimer.c:1289 is executed.  In this loop the code subtracts the
clock base's offset (which was set to timekeeper->offs_real in
do_settimeofday()) from the current hrtimer_cpu_base->expiry value 
(which

was KTIME_MAX):

KTIME_MAX - (a negative value) = overflow

A simple check for an overflow can resolve this problem.  Using 
KTIME_MAX
instead of the overflow value will result in the hrtimer function 
being run,

and the reprogramming of the timer after that.

>

Signed-off-by: Prarit Bhargava 
Cc: Thomas Gleixner 
Cc: John Stultz 


Reviewed-by: Rik van Riel 



Queued in my fortglx/3.10/time branch.

thanks
-john


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


Re: [PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-08 Thread Rik van Riel

On 04/08/2013 08:47 AM, Prarit Bhargava wrote:


When we change the system time to a low value like this, the value of
timekeeper->offs_real will be a negative value.

It seems that the WARN occurs because an hrtimer has been started in the time
between the releasing of the timekeeper lock and the IPI call (via a call to
on_each_cpu) in clock_was_set() in the do_settimeofday() code.  The end result
is that a REALTIME_CLOCK timer has been added with softexpires = expires =
KTIME_MAX.  The hrtimer_interrupt() fires/is called and the loop at
kernel/hrtimer.c:1289 is executed.  In this loop the code subtracts the
clock base's offset (which was set to timekeeper->offs_real in
do_settimeofday()) from the current hrtimer_cpu_base->expiry value (which
was KTIME_MAX):

KTIME_MAX - (a negative value) = overflow

A simple check for an overflow can resolve this problem.  Using KTIME_MAX
instead of the overflow value will result in the hrtimer function being run,
and the reprogramming of the timer after that.

>

Signed-off-by: Prarit Bhargava 
Cc: Thomas Gleixner 
Cc: John Stultz 


Reviewed-by: Rik van Riel 

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


[PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-04-08 Thread Prarit Bhargava
2nd submit: I did quite a bit of testing with this patch and I don't see any
ill effects.  I've tested across several large and small x86 systems, and a
ppc system for good measure.

P.

8<---
`
The settimeofday01 test in the LTP testsuite effectively does

gettimeofday(current time);
settimeofday(Jan 1, 1970 + 100 seconds);
settimeofday(current time);

This test causes a stack trace to be displayed on the console during the
setting of timeofday to Jan 1, 1970 + 100 seconds:

[  131.066751] [ cut here ]
[  131.096448] WARNING: at kernel/time/clockevents.c:209 
clockevents_program_event+0x135/0x140()
[  131.104935] Hardware name: Dinar
[  131.108150] Modules linked in: sg nfsv3 nfs_acl nfsv4 auth_rpcgss nfs 
dns_resolver fscache lockd sunrpc nf_conntrack_netbios_ns 
nf_conntrack_broadcast ipt_MASQUERADE ip6table_mangle ip6t_REJECT 
nf_conntrack_ipv6 nf_defrag_ipv6 iptable_nat nf_nat_ipv4 nf_nat iptable_mangle 
ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack 
ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter ip_tables 
kvm_amd kvm sp5100_tco bnx2 i2c_piix4 crc32c_intel k10temp fam15h_power 
ghash_clmulni_intel amd64_edac_mod pcspkr serio_raw edac_mce_amd edac_core 
microcode xfs libcrc32c sr_mod sd_mod cdrom ata_generic crc_t10dif pata_acpi 
radeon i2c_algo_bit drm_kms_helper ttm drm ahci pata_atiixp libahci libata 
usb_storage i2c_core dm_mirror dm_region_hash dm_log dm_mod
[  131.176784] Pid: 0, comm: swapper/28 Not tainted 3.8.0+ #6
[  131.182248] Call Trace:
[  131.184684][] warn_slowpath_common+0x7f/0xc0
[  131.191312]  [] warn_slowpath_null+0x1a/0x20
[  131.197131]  [] clockevents_program_event+0x135/0x140
[  131.203721]  [] tick_program_event+0x24/0x30
[  131.209534]  [] hrtimer_interrupt+0x131/0x230
[  131.215437]  [] ? cpufreq_p4_target+0x130/0x130
[  131.221509]  [] smp_apic_timer_interrupt+0x69/0x99
[  131.227839]  [] apic_timer_interrupt+0x6d/0x80
[  131.233816][] ? sched_clock_cpu+0xc5/0x120
[  131.240267]  [] ? cpuidle_wrap_enter+0x50/0xa0
[  131.246252]  [] ? cpuidle_wrap_enter+0x49/0xa0
[  131.252238]  [] cpuidle_enter_tk+0x10/0x20
[  131.257877]  [] cpuidle_idle_call+0xa9/0x260
[  131.263692]  [] cpu_idle+0xaf/0x120
[  131.268727]  [] start_secondary+0x255/0x257
[  131.274449] ---[ end trace 1151a50552231615 ]---

When we change the system time to a low value like this, the value of
timekeeper->offs_real will be a negative value.

It seems that the WARN occurs because an hrtimer has been started in the time
between the releasing of the timekeeper lock and the IPI call (via a call to
on_each_cpu) in clock_was_set() in the do_settimeofday() code.  The end result
is that a REALTIME_CLOCK timer has been added with softexpires = expires =
KTIME_MAX.  The hrtimer_interrupt() fires/is called and the loop at
kernel/hrtimer.c:1289 is executed.  In this loop the code subtracts the
clock base's offset (which was set to timekeeper->offs_real in
do_settimeofday()) from the current hrtimer_cpu_base->expiry value (which
was KTIME_MAX):

KTIME_MAX - (a negative value) = overflow

A simple check for an overflow can resolve this problem.  Using KTIME_MAX
instead of the overflow value will result in the hrtimer function being run,
and the reprogramming of the timer after that.

Signed-off-by: Prarit Bhargava 
Cc: Thomas Gleixner 
Cc: John Stultz 
---
 kernel/hrtimer.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index cc47812..17eafd7 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1309,6 +1309,8 @@ retry:
 
expires = ktime_sub(hrtimer_get_expires(timer),
base->offset);
+   if (expires.tv64 < 0)
+   expires.tv64 = KTIME_MAX;
if (expires.tv64 < expires_next.tv64)
expires_next = expires;
break;
-- 
1.7.9.3

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


[RFE PATCH] hrtimer, add expiry time overflow check in hrtimer_interrupt

2013-03-28 Thread Prarit Bhargava
The settimeofday01 test in the LTP testsuite effectively does

gettimeofday(current time);
settimeofday(Jan 1, 1970 + 100 seconds);
settimeofday(current time);

This test causes a stack trace to be displayed on the console during the
setting of timeofday to Jan 1, 1970 + 100 seconds:

[  131.066751] [ cut here ]
[  131.096448] WARNING: at kernel/time/clockevents.c:209 
clockevents_program_event+0x135/0x140()
[  131.104935] Hardware name: Dinar
[  131.108150] Modules linked in: sg nfsv3 nfs_acl nfsv4 auth_rpcgss nfs 
dns_resolver fscache lockd sunrpc nf_conntrack_netbios_ns 
nf_conntrack_broadcast ipt_MASQUERADE ip6table_mangle ip6t_REJECT 
nf_conntrack_ipv6 nf_defrag_ipv6 iptable_nat nf_nat_ipv4 nf_nat iptable_mangle 
ipt_REJECT nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack 
ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter ip_tables 
kvm_amd kvm sp5100_tco bnx2 i2c_piix4 crc32c_intel k10temp fam15h_power 
ghash_clmulni_intel amd64_edac_mod pcspkr serio_raw edac_mce_amd edac_core 
microcode xfs libcrc32c sr_mod sd_mod cdrom ata_generic crc_t10dif pata_acpi 
radeon i2c_algo_bit drm_kms_helper ttm drm ahci pata_atiixp libahci libata 
usb_storage i2c_core dm_mirror dm_region_hash dm_log dm_mod
[  131.176784] Pid: 0, comm: swapper/28 Not tainted 3.8.0+ #6
[  131.182248] Call Trace:
[  131.184684][] warn_slowpath_common+0x7f/0xc0
[  131.191312]  [] warn_slowpath_null+0x1a/0x20
[  131.197131]  [] clockevents_program_event+0x135/0x140
[  131.203721]  [] tick_program_event+0x24/0x30
[  131.209534]  [] hrtimer_interrupt+0x131/0x230
[  131.215437]  [] ? cpufreq_p4_target+0x130/0x130
[  131.221509]  [] smp_apic_timer_interrupt+0x69/0x99
[  131.227839]  [] apic_timer_interrupt+0x6d/0x80
[  131.233816][] ? sched_clock_cpu+0xc5/0x120
[  131.240267]  [] ? cpuidle_wrap_enter+0x50/0xa0
[  131.246252]  [] ? cpuidle_wrap_enter+0x49/0xa0
[  131.252238]  [] cpuidle_enter_tk+0x10/0x20
[  131.257877]  [] cpuidle_idle_call+0xa9/0x260
[  131.263692]  [] cpu_idle+0xaf/0x120
[  131.268727]  [] start_secondary+0x255/0x257
[  131.274449] ---[ end trace 1151a50552231615 ]---

When we change the system time to a low value like this, the value of
timekeeper->offs_real will be a negative value.

It seems that the WARN occurs because an hrtimer has been started in the time
between the releasing of the timekeeper lock and the IPI call (via a call to
on_each_cpu) in clock_was_set() in the do_settimeofday() code.  The end result
is that a REALTIME_CLOCK timer has been added with softexpires = expires =
KTIME_MAX.  The hrtimer_interrupt() fires/is called and the loop at
kernel/hrtimer.c:1289 is executed.  In this loop the code subtracts the
clock base's offset (which was set to timekeeper->offs_real in
do_settimeofday()) from the current hrtimer_cpu_base->expiry value (which
was KTIME_MAX):

KTIME_MAX - (a negative value) = overflow

A simple check for an overflow can resolve this problem.  Using KTIME_MAX
instead of the overflow value will result in the hrtimer function being run,
and the reprogramming of the timer after that.

Signed-off-by: Prarit Bhargava 
Cc: Thomas Gleixner 
Cc: John Stultz 
---
 kernel/hrtimer.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index cc47812..17eafd7 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1309,6 +1309,8 @@ retry:
 
expires = ktime_sub(hrtimer_get_expires(timer),
base->offset);
+   if (expires.tv64 < 0)
+   expires.tv64 = KTIME_MAX;
if (expires.tv64 < expires_next.tv64)
expires_next = expires;
break;
-- 
1.7.9.3

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