[git pull] timer fix

[git pull] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-fixes-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git timers-fixes-for-linus

 Thanks,

	Ingo

------------------>
Pavel Emelyanov (1):
      x86: fix hpet timer reinit for x86_64


 arch/x86/kernel/hpet.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index 64d5ad0..ec319d1 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -1075,7 +1075,7 @@ static void hpet_rtc_timer_reinit(void)
 		hpet_t1_cmp += delta;
 		hpet_writel(hpet_t1_cmp, HPET_T1_CMP);
 		lost_ints++;
-	} while ((long)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);
+	} while ((long)(hpet_readl(HPET_COUNTER) - (u32)hpet_t1_cmp) > 0);
 
 	if (lost_ints) {
 		if (hpet_rtc_flags & RTC_PIE)

Re: [git pull] timer fix

[Linus Torvalds]

On Wed, 4 Feb 2009, Ingo Molnar wrote:
>
> Pavel Emelyanov (1):
>       x86: fix hpet timer reinit for x86_64
> 
> 
>  arch/x86/kernel/hpet.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
> index 64d5ad0..ec319d1 100644
> --- a/arch/x86/kernel/hpet.c
> +++ b/arch/x86/kernel/hpet.c
> @@ -1075,7 +1075,7 @@ static void hpet_rtc_timer_reinit(void)
>  		hpet_t1_cmp += delta;
>  		hpet_writel(hpet_t1_cmp, HPET_T1_CMP);
>  		lost_ints++;
> -	} while ((long)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);
> +	} while ((long)(hpet_readl(HPET_COUNTER) - (u32)hpet_t1_cmp) > 0);

This is bordering on not being correct.

It may happen to _work_, but the fact is, you want a 32-bit signed 
compare, not a 64-bit subtract that just happens to work. So the proper 
fix is to just make it do

	} while ((s32)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);

Otherwise you always end up depending on very subtle internal logic, and 
the exact types of the things involved.

In particular, think about when HPET_COUNTER or hpet_t1_cmp overflows in 
32 bits, and what you want to happen. If you do the subtract add test in 
64 bits, it will simply do the wrong thing. Think what happens if 
hpet_t1_cmp is actually _larger_ than HPET_COUNTER, but overflowed in 32 
bits, and you're now looking at:

	(long) (0xffffffff - 0x00000001)

which is actually > 0, so the thing will continue to loop INCORRECTLY. It 
should have stopped (and _would_ have stopped on 32-bit x86).

In contrast, look at what happens if you do the subtracting (or at least 
test the _result_ of the subtract) in the right size:

	(s32) (0xffffffff - 0x00000001) 

which becomes -2, which is not larger than 0, which means that we exit 
(which is correct, because the comparator value is actually ahead of the 
current count: 0x00000001 is _ahead_ of 0xffffffff, even if it's smaller 
in an "unsigned long".

So I'm not going to pull it. This cast is simply wrong.

Either cast the result of the subtract to "s32" (or "int", whatever), or 
cast _both_ of them to (s32) so that the subtract is done in a signed 
type, and then the expansion to (long) will still be right - but 
unnecessary - in the sign.

			Linus

Re: [git pull] timer fix

[Linus Torvalds]

On Wed, 4 Feb 2009, Linus Torvalds wrote:
> 
> Either cast the result of the subtract to "s32" (or "int", whatever), or 
> cast _both_ of them to (s32) so that the subtract is done in a signed 
> type, and then the expansion to (long) will still be right - but 
> unnecessary - in the sign.

Btw, doing it with a nice helper macro or function is also perhaps a good 
idea, at least if these "compare hpet values" things happen more than 
once. 

Look at "time_after()" in <linux/jiffies.h> to see how to do these kinds 
of "comparisons of things that may overflow" really carefully. You 
absolutely need to do the compare in a size that is no larger than the 
size of the actual values (and in the case of HPET, it's 32-bit, at least 
the way we do things now - I guess HPET's _could_ be 64-bit, but we don't 
read more than 32 bits or whatever).

So <linux/jiffies.h> does the cast to "(long)", but it does so because the 
incoming values really have type "unsigned long" and are valid in all 
bits.

			Linus

Re: [git pull] timer fix

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, 4 Feb 2009, Ingo Molnar wrote:
> >
> > Pavel Emelyanov (1):
> >       x86: fix hpet timer reinit for x86_64
> > 
> > 
> >  arch/x86/kernel/hpet.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
> > index 64d5ad0..ec319d1 100644
> > --- a/arch/x86/kernel/hpet.c
> > +++ b/arch/x86/kernel/hpet.c
> > @@ -1075,7 +1075,7 @@ static void hpet_rtc_timer_reinit(void)
> >  		hpet_t1_cmp += delta;
> >  		hpet_writel(hpet_t1_cmp, HPET_T1_CMP);
> >  		lost_ints++;
> > -	} while ((long)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);
> > +	} while ((long)(hpet_readl(HPET_COUNTER) - (u32)hpet_t1_cmp) > 0);
> 
> This is bordering on not being correct.

yeah, i had to look twice. The only reason i left it that way was because i 
couldnt reproduce the problem and hpet is hellishly fragile and this patch 
was tested so i chickened out.

OTOH that fragility is partly because such constructs have piled up so you 
very much have a valid point ...

We'll clean this up. I've already added the clean 32-bit casts - which also 
has another advantage: it does not actually trust the hw to always return 
32-bit values - it explicitly cuts to 32 bits and does signed arithmetics on 
that. Will also do the helper function cleanup to abstract the counter 
arithmetics away.

> In particular, think about when HPET_COUNTER or hpet_t1_cmp overflows in 
> 32 bits, and what you want to happen. If you do the subtract add test in 
> 64 bits, it will simply do the wrong thing. Think what happens if 
> hpet_t1_cmp is actually _larger_ than HPET_COUNTER, but overflowed in 32 
> bits, and you're now looking at:
> 
> 	(long) (0xffffffff - 0x00000001)
> 
> which is actually > 0, so the thing will continue to loop INCORRECTLY. It 
> should have stopped (and _would_ have stopped on 32-bit x86).

yeah, allowing that to happen is just wrong.

	Ingo

Re: [git pull] timer fix

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 	} while ((s32)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);

that is also more efficient code by 6 bytes:

before:

ffffffff81020856:	ff c3                	inc    %ebx
ffffffff81020858:	48 05 f0 00 00 00    	add    $0xf0,%rax
ffffffff8102085e:	8b 00                	mov    (%rax),%eax
ffffffff81020860:	8b 15 2a 79 85 00    	mov    0x85792a(%rip),%edx        # ffffffff81878190 <hpet_t1_cmp>
ffffffff81020866:	89 c0                	mov    %eax,%eax
ffffffff81020868:	48 29 d0             	sub    %rdx,%rax
ffffffff8102086b:	48 85 c0             	test   %rax,%rax
ffffffff8102086e:	7f bf                	jg     ffffffff8102082f <hpet_rtc_interrupt+0x68>
ffffffff81020870:	85 db                	test   %ebx,%ebx

after:

ffffffff81020856:	ff c3                	inc    %ebx
ffffffff81020858:	48 05 f0 00 00 00    	add    $0xf0,%rax
ffffffff8102085e:	8b 00                	mov    (%rax),%eax
ffffffff81020860:	2b 05 2a 79 85 00    	sub    0x85792a(%rip),%eax        # ffffffff81878190 <hpet_t1_cmp>
ffffffff81020866:	85 c0                	test   %eax,%eax
ffffffff81020868:	7f c5                	jg     ffffffff8102082f <hpet_rtc_interrupt+0x68>
ffffffff8102086a:	85 db                	test   %ebx,%ebx

Kirill, Pavel, could you please re-test the updated commit attached below?

	Ingo

---------------->
>From 66a36a1e95fe9de9c6a56f0bcd01f4ba21929f86 Mon Sep 17 00:00:00 2001
From: Pavel Emelyanov <xemul@openvz.org>
Date: Wed, 4 Feb 2009 13:40:31 +0300
Subject: [PATCH] x86: fix hpet timer reinit for x86_64

There's a small problem with hpet_rtc_reinit function - it checks
for the:

	hpet_readl(HPET_COUNTER) - hpet_t1_cmp > 0

to continue increasing both the HPET_T1_CMP (register) and the
hpet_t1_cmp (variable).

But since the HPET_COUNTER is always 32-bit, if the hpet_t1_cmp
is 64-bit this condition will always be FALSE once the latter hits
the 32-bit boundary, and we can have a situation, when we don't
increase the HPET_T1_CMP register high enough.

The result - timer stops ticking, since HPET_T1_CMP becomes less,
than the COUNTER and never increased again.

The solution is (based on Linus's suggestion) to compare 64-bits
(on 64-bit x86), but to do the comparison on 32-bit signed
integers.

Reported-by: Kirill Korotaev <dev@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/hpet.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index 64d5ad0..c761f91 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -1075,7 +1075,7 @@ static void hpet_rtc_timer_reinit(void)
 		hpet_t1_cmp += delta;
 		hpet_writel(hpet_t1_cmp, HPET_T1_CMP);
 		lost_ints++;
-	} while ((long)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);
+	} while ((s32)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);
 
 	if (lost_ints) {
 		if (hpet_rtc_flags & RTC_PIE)

Re: [git pull] timer fix

[H. Peter Anvin]

Ingo Molnar wrote:
> 
> The solution is (based on Linus's suggestion) to compare 64-bits
                                                   ^not, presumably
> (on 64-bit x86), but to do the comparison on 32-bit signed
> integers.

	-hpa

Re: [git pull] timer fix

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

> Ingo Molnar wrote:
>>
>> The solution is (based on Linus's suggestion) to compare 64-bits
>                                                   ^not, presumably
>> (on 64-bit x86), but to do the comparison on 32-bit signed
>> integers.

fixed, thanks!

	Ingo

Re: [git pull] timer fix

[Kirill Korotaev]

ACK, it works. Why not save another 4 bytes of .bss then by changing
hept_t1_cmp to u32? ;-)

Kirill


On 2/5/09 1:58 AM, "Ingo Molnar" <mingo@elte.hu> wrote:

> 
> 
> * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
>>       } while ((s32)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);
> 
> that is also more efficient code by 6 bytes:
> 
> before:
> 
> ffffffff81020856:       ff c3                   inc    %ebx
> ffffffff81020858:       48 05 f0 00 00 00       add    $0xf0,%rax
> ffffffff8102085e:       8b 00                   mov    (%rax),%eax
> ffffffff81020860:       8b 15 2a 79 85 00       mov    0x85792a(%rip),%edx
> # ffffffff81878190 <hpet_t1_cmp>
> ffffffff81020866:       89 c0                   mov    %eax,%eax
> ffffffff81020868:       48 29 d0                sub    %rdx,%rax
> ffffffff8102086b:       48 85 c0                test   %rax,%rax
> ffffffff8102086e:       7f bf                   jg     ffffffff8102082f
> <hpet_rtc_interrupt+0x68>
> ffffffff81020870:       85 db                   test   %ebx,%ebx
> 
> after:
> 
> ffffffff81020856:       ff c3                   inc    %ebx
> ffffffff81020858:       48 05 f0 00 00 00       add    $0xf0,%rax
> ffffffff8102085e:       8b 00                   mov    (%rax),%eax
> ffffffff81020860:       2b 05 2a 79 85 00       sub    0x85792a(%rip),%eax
> # ffffffff81878190 <hpet_t1_cmp>
> ffffffff81020866:       85 c0                   test   %eax,%eax
> ffffffff81020868:       7f c5                   jg     ffffffff8102082f
> <hpet_rtc_interrupt+0x68>
> ffffffff8102086a:       85 db                   test   %ebx,%ebx
> 
> Kirill, Pavel, could you please re-test the updated commit attached below?
> 
>         Ingo
> 
> ---------------->
> From 66a36a1e95fe9de9c6a56f0bcd01f4ba21929f86 Mon Sep 17 00:00:00 2001
> From: Pavel Emelyanov <xemul@openvz.org>
> Date: Wed, 4 Feb 2009 13:40:31 +0300
> Subject: [PATCH] x86: fix hpet timer reinit for x86_64
> 
> There's a small problem with hpet_rtc_reinit function - it checks
> for the:
> 
>         hpet_readl(HPET_COUNTER) - hpet_t1_cmp > 0
> 
> to continue increasing both the HPET_T1_CMP (register) and the
> hpet_t1_cmp (variable).
> 
> But since the HPET_COUNTER is always 32-bit, if the hpet_t1_cmp
> is 64-bit this condition will always be FALSE once the latter hits
> the 32-bit boundary, and we can have a situation, when we don't
> increase the HPET_T1_CMP register high enough.
> 
> The result - timer stops ticking, since HPET_T1_CMP becomes less,
> than the COUNTER and never increased again.
> 
> The solution is (based on Linus's suggestion) to compare 64-bits
> (on 64-bit x86), but to do the comparison on 32-bit signed
> integers.
> 
> Reported-by: Kirill Korotaev <dev@openvz.org>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  arch/x86/kernel/hpet.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
> index 64d5ad0..c761f91 100644
> --- a/arch/x86/kernel/hpet.c
> +++ b/arch/x86/kernel/hpet.c
> @@ -1075,7 +1075,7 @@ static void hpet_rtc_timer_reinit(void)
>                 hpet_t1_cmp += delta;
>                 hpet_writel(hpet_t1_cmp, HPET_T1_CMP);
>                 lost_ints++;
> -       } while ((long)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);
> +       } while ((s32)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);
> 
>         if (lost_ints) {
>                 if (hpet_rtc_flags & RTC_PIE)

Re: [git pull] timer fix

[Pavel Emelyanov]

Kirill Korotaev wrote:
> ACK, it works. Why not save another 4 bytes of .bss then by changing
> hept_t1_cmp to u32? ;-)

Hm... .bss you say? :) The bloat-o-meter results would be:

For the original fix with casting hpet_t1_cmp to u32 inside the loop
add/remove: 0/0 grow/shrink: 1/0 up/down: 2/0 (2)
function                                     old     new   delta
hpet_rtc_interrupt                           741     743      +2

For the fix with casting the substitution result to s32
add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-2 (-2)
function                                     old     new   delta
hpet_rtc_interrupt                           741     739      -2

For the proposed by Kirill type change of the hpet_t1_cmp
add/remove: 0/0 grow/shrink: 0/3 up/down: 0/-6 (-6)
function                                     old     new   delta
hpet_rtc_timer_init                          186     185      -1
hpet_rtc_interrupt                           741     740      -1
hpet_t1_cmp                                    8       4      -4

That's the fix:

diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index 64d5ad0..dfbbb94 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -897,7 +897,7 @@ static unsigned long hpet_rtc_flags;
 static int hpet_prev_update_sec;
 static struct rtc_time hpet_alarm_time;
 static unsigned long hpet_pie_count;
-static unsigned long hpet_t1_cmp;
+static u32 hpet_t1_cmp;
 static unsigned long hpet_default_delta;
 static unsigned long hpet_pie_delta;
 static unsigned long hpet_pie_limit;

Reported-by: Kirill Korotaev <dev@openvz.org>
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

This one also works. Should I re-send this patch in a proper way?

> Kirill
> 
> 
> On 2/5/09 1:58 AM, "Ingo Molnar" <mingo@elte.hu> wrote:
> 
>>
>> * Linus Torvalds <torvalds@linux-foundation.org> wrote:
>>
>>>       } while ((s32)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);
>> that is also more efficient code by 6 bytes:
>>
>> before:
>>
>> ffffffff81020856:       ff c3                   inc    %ebx
>> ffffffff81020858:       48 05 f0 00 00 00       add    $0xf0,%rax
>> ffffffff8102085e:       8b 00                   mov    (%rax),%eax
>> ffffffff81020860:       8b 15 2a 79 85 00       mov    0x85792a(%rip),%edx
>> # ffffffff81878190 <hpet_t1_cmp>
>> ffffffff81020866:       89 c0                   mov    %eax,%eax
>> ffffffff81020868:       48 29 d0                sub    %rdx,%rax
>> ffffffff8102086b:       48 85 c0                test   %rax,%rax
>> ffffffff8102086e:       7f bf                   jg     ffffffff8102082f
>> <hpet_rtc_interrupt+0x68>
>> ffffffff81020870:       85 db                   test   %ebx,%ebx
>>
>> after:
>>
>> ffffffff81020856:       ff c3                   inc    %ebx
>> ffffffff81020858:       48 05 f0 00 00 00       add    $0xf0,%rax
>> ffffffff8102085e:       8b 00                   mov    (%rax),%eax
>> ffffffff81020860:       2b 05 2a 79 85 00       sub    0x85792a(%rip),%eax
>> # ffffffff81878190 <hpet_t1_cmp>
>> ffffffff81020866:       85 c0                   test   %eax,%eax
>> ffffffff81020868:       7f c5                   jg     ffffffff8102082f
>> <hpet_rtc_interrupt+0x68>
>> ffffffff8102086a:       85 db                   test   %ebx,%ebx
>>
>> Kirill, Pavel, could you please re-test the updated commit attached below?
>>
>>         Ingo
>>
>> ---------------->
>> From 66a36a1e95fe9de9c6a56f0bcd01f4ba21929f86 Mon Sep 17 00:00:00 2001
>> From: Pavel Emelyanov <xemul@openvz.org>
>> Date: Wed, 4 Feb 2009 13:40:31 +0300
>> Subject: [PATCH] x86: fix hpet timer reinit for x86_64
>>
>> There's a small problem with hpet_rtc_reinit function - it checks
>> for the:
>>
>>         hpet_readl(HPET_COUNTER) - hpet_t1_cmp > 0
>>
>> to continue increasing both the HPET_T1_CMP (register) and the
>> hpet_t1_cmp (variable).
>>
>> But since the HPET_COUNTER is always 32-bit, if the hpet_t1_cmp
>> is 64-bit this condition will always be FALSE once the latter hits
>> the 32-bit boundary, and we can have a situation, when we don't
>> increase the HPET_T1_CMP register high enough.
>>
>> The result - timer stops ticking, since HPET_T1_CMP becomes less,
>> than the COUNTER and never increased again.
>>
>> The solution is (based on Linus's suggestion) to compare 64-bits
>> (on 64-bit x86), but to do the comparison on 32-bit signed
>> integers.
>>
>> Reported-by: Kirill Korotaev <dev@openvz.org>
>> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
>> Signed-off-by: Ingo Molnar <mingo@elte.hu>
>> ---
>>  arch/x86/kernel/hpet.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
>> index 64d5ad0..c761f91 100644
>> --- a/arch/x86/kernel/hpet.c
>> +++ b/arch/x86/kernel/hpet.c
>> @@ -1075,7 +1075,7 @@ static void hpet_rtc_timer_reinit(void)
>>                 hpet_t1_cmp += delta;
>>                 hpet_writel(hpet_t1_cmp, HPET_T1_CMP);
>>                 lost_ints++;
>> -       } while ((long)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);
>> +       } while ((s32)(hpet_readl(HPET_COUNTER) - hpet_t1_cmp) > 0);
>>
>>         if (lost_ints) {
>>                 if (hpet_rtc_flags & RTC_PIE)
> 
> 
> 

Re: [git pull] timer fix

[Ingo Molnar]

* Pavel Emelyanov <xemul@openvz.org> wrote:

> Reported-by: Kirill Korotaev <dev@openvz.org>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> 
> This one also works. Should I re-send this patch in a proper way?

Yes, please do. Also, please have a look at the helper function cleanup 
suggestion from Linus - such a patch would be welcome too.

	Ingo

Re: [git pull] timer fix

[Ray Lee]

On Thu, Feb 5, 2009 at 1:58 AM, Pavel Emelyanov <xemul@openvz.org> wrote:
> Kirill Korotaev wrote:
>> ACK, it works. Why not save another 4 bytes of .bss then by changing
>> hept_t1_cmp to u32? ;-)
>
> Hm... .bss you say? :) The bloat-o-meter results would be:
>
> For the original fix with casting hpet_t1_cmp to u32 inside the loop
> add/remove: 0/0 grow/shrink: 1/0 up/down: 2/0 (2)
> function                                     old     new   delta
> hpet_rtc_interrupt                           741     743      +2
>
> For the fix with casting the substitution result to s32
> add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-2 (-2)
> function                                     old     new   delta
> hpet_rtc_interrupt                           741     739      -2
>
> For the proposed by Kirill type change of the hpet_t1_cmp
> add/remove: 0/0 grow/shrink: 0/3 up/down: 0/-6 (-6)
> function                                     old     new   delta
> hpet_rtc_timer_init                          186     185      -1
> hpet_rtc_interrupt                           741     740      -1
> hpet_t1_cmp                                    8       4      -4
>
> That's the fix:
>
> diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
> index 64d5ad0..dfbbb94 100644
> --- a/arch/x86/kernel/hpet.c
> +++ b/arch/x86/kernel/hpet.c
> @@ -897,7 +897,7 @@ static unsigned long hpet_rtc_flags;
>  static int hpet_prev_update_sec;
>  static struct rtc_time hpet_alarm_time;
>  static unsigned long hpet_pie_count;
> -static unsigned long hpet_t1_cmp;
> +static u32 hpet_t1_cmp;
>  static unsigned long hpet_default_delta;
>  static unsigned long hpet_pie_delta;
>  static unsigned long hpet_pie_limit;
>
> Reported-by: Kirill Korotaev <dev@openvz.org>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
>
> This one also works. Should I re-send this patch in a proper way?

Ugh. This version seems too subtle for a new reader coming into this
code. I'd prefer the fix be kept in the comparison site, where people
already expect wraparound issues.

If you don't have time to add the helper, I'd really rather see the
version with the explicit cast acting as documentation, rather than a
test that magically works due to type issues.

Dunno, maybe it's just me.

[git pull] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-fixes-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git timers-fixes-for-linus

 Thanks,

	Ingo

------------------>
Peter Zijlstra (1):
      timers: more consistently use clock vs timer


 kernel/posix-cpu-timers.c |   60 ++++++++++++++++++++++----------------------
 1 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index 2313a4c..e976e50 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/posix-cpu-timers.c
@@ -681,6 +681,33 @@ static void cpu_timer_fire(struct k_itimer *timer)
 }
 
 /*
+ * Sample a process (thread group) timer for the given group_leader task.
+ * Must be called with tasklist_lock held for reading.
+ */
+static int cpu_timer_sample_group(const clockid_t which_clock,
+				  struct task_struct *p,
+				  union cpu_time_count *cpu)
+{
+	struct task_cputime cputime;
+
+	thread_group_cputimer(p, &cputime);
+	switch (CPUCLOCK_WHICH(which_clock)) {
+	default:
+		return -EINVAL;
+	case CPUCLOCK_PROF:
+		cpu->cpu = cputime_add(cputime.utime, cputime.stime);
+		break;
+	case CPUCLOCK_VIRT:
+		cpu->cpu = cputime.utime;
+		break;
+	case CPUCLOCK_SCHED:
+		cpu->sched = cputime.sum_exec_runtime + task_delta_exec(p);
+		break;
+	}
+	return 0;
+}
+
+/*
  * Guts of sys_timer_settime for CPU timers.
  * This is called with the timer locked and interrupts disabled.
  * If we return TIMER_RETRY, it's necessary to release the timer's lock
@@ -741,7 +768,7 @@ int posix_cpu_timer_set(struct k_itimer *timer, int flags,
 	if (CPUCLOCK_PERTHREAD(timer->it_clock)) {
 		cpu_clock_sample(timer->it_clock, p, &val);
 	} else {
-		cpu_clock_sample_group(timer->it_clock, p, &val);
+		cpu_timer_sample_group(timer->it_clock, p, &val);
 	}
 
 	if (old) {
@@ -889,7 +916,7 @@ void posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec *itp)
 			read_unlock(&tasklist_lock);
 			goto dead;
 		} else {
-			cpu_clock_sample_group(timer->it_clock, p, &now);
+			cpu_timer_sample_group(timer->it_clock, p, &now);
 			clear_dead = (unlikely(p->exit_state) &&
 				      thread_group_empty(p));
 		}
@@ -1244,7 +1271,7 @@ void posix_cpu_timer_schedule(struct k_itimer *timer)
 			clear_dead_task(timer, now);
 			goto out_unlock;
 		}
-		cpu_clock_sample_group(timer->it_clock, p, &now);
+		cpu_timer_sample_group(timer->it_clock, p, &now);
 		bump_cpu_timer(timer, now);
 		/* Leave the tasklist_lock locked for the call below.  */
 	}
@@ -1409,33 +1436,6 @@ void run_posix_cpu_timers(struct task_struct *tsk)
 }
 
 /*
- * Sample a process (thread group) timer for the given group_leader task.
- * Must be called with tasklist_lock held for reading.
- */
-static int cpu_timer_sample_group(const clockid_t which_clock,
-				  struct task_struct *p,
-				  union cpu_time_count *cpu)
-{
-	struct task_cputime cputime;
-
-	thread_group_cputimer(p, &cputime);
-	switch (CPUCLOCK_WHICH(which_clock)) {
-	default:
-		return -EINVAL;
-	case CPUCLOCK_PROF:
-		cpu->cpu = cputime_add(cputime.utime, cputime.stime);
-		break;
-	case CPUCLOCK_VIRT:
-		cpu->cpu = cputime.utime;
-		break;
-	case CPUCLOCK_SCHED:
-		cpu->sched = cputime.sum_exec_runtime + task_delta_exec(p);
-		break;
-	}
-	return 0;
-}
-
-/*
  * Set one of the process-wide special case CPU timers.
  * The tsk->sighand->siglock must be held by the caller.
  * The *newval argument is relative and we update it to be absolute, *oldval

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-fixes-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git timers-fixes-for-linus

 Thanks,

	Ingo

------------------>
Eero Nurkkala (1):
      NOHZ: Properly feed cpufreq ondemand governor


 kernel/time/tick-sched.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index d3f1ef4..a3562ce 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -222,6 +222,15 @@ void tick_nohz_stop_sched_tick(int inidle)
 
 	cpu = smp_processor_id();
 	ts = &per_cpu(tick_cpu_sched, cpu);
+
+	/*
+	 * Call to tick_nohz_start_idle stops the last_update_time from being
+	 * updated. Thus, it must not be called in the event we are called from
+	 * irq_exit() with the prior state different than idle.
+	 */
+	if (!inidle && !ts->inidle)
+		goto end;
+
 	now = tick_nohz_start_idle(ts);
 
 	/*
@@ -239,9 +248,6 @@ void tick_nohz_stop_sched_tick(int inidle)
 	if (unlikely(ts->nohz_mode == NOHZ_MODE_INACTIVE))
 		goto end;
 
-	if (!inidle && !ts->inidle)
-		goto end;
-
 	ts->inidle = 1;
 
 	if (need_resched())

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-fixes-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git timers-fixes-for-linus

 Thanks,

	Ingo

------------------>
Hiroshi Shimamoto (1):
      posix-timers: Fix oops in clock_nanosleep() with CLOCK_MONOTONIC_RAW


 kernel/posix-timers.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 052ec4d..d089d05 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -202,6 +202,12 @@ static int no_timer_create(struct k_itimer *new_timer)
 	return -EOPNOTSUPP;
 }
 
+static int no_nsleep(const clockid_t which_clock, int flags,
+		     struct timespec *tsave, struct timespec __user *rmtp)
+{
+	return -EOPNOTSUPP;
+}
+
 /*
  * Return nonzero if we know a priori this clockid_t value is bogus.
  */
@@ -254,6 +260,7 @@ static __init int init_posix_timers(void)
 		.clock_get = posix_get_monotonic_raw,
 		.clock_set = do_posix_clock_nosettime,
 		.timer_create = no_timer_create,
+		.nsleep = no_nsleep,
 	};
 
 	register_posix_clock(CLOCK_REALTIME, &clock_realtime);

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-fixes-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git timers-fixes-for-linus

 Thanks,

	Ingo

------------------>
Stanislaw Gruszka (1):
      posix_cpu_timers_exit_group(): Do not use thread_group_cputimer()


 kernel/posix-cpu-timers.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index bece7c0..e33a21c 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/posix-cpu-timers.c
@@ -521,11 +521,12 @@ void posix_cpu_timers_exit(struct task_struct *tsk)
 }
 void posix_cpu_timers_exit_group(struct task_struct *tsk)
 {
-	struct task_cputime cputime;
+	struct signal_struct *const sig = tsk->signal;
 
-	thread_group_cputimer(tsk, &cputime);
 	cleanup_timers(tsk->signal->cpu_timers,
-		       cputime.utime, cputime.stime, cputime.sum_exec_runtime);
+		       cputime_add(tsk->utime, sig->utime),
+		       cputime_add(tsk->stime, sig->stime),
+		       tsk->se.sum_exec_runtime + sig->sum_sched_runtime);
 }
 
 static void clear_dead_task(struct k_itimer *timer, union cpu_time_count now)

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-fixes-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git timers-fixes-for-linus

 Thanks,

	Ingo

------------------>
Martin Schwidefsky (1):
      clocksource: Resume clocksource without taking the clocksource mutex


 kernel/time/clocksource.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 0911334..5e18c6a 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -394,15 +394,11 @@ void clocksource_resume(void)
 {
 	struct clocksource *cs;
 
-	mutex_lock(&clocksource_mutex);
-
 	list_for_each_entry(cs, &clocksource_list, list)
 		if (cs->resume)
 			cs->resume();
 
 	clocksource_resume_watchdog();
-
-	mutex_unlock(&clocksource_mutex);
 }
 
 /**

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-fixes-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git timers-fixes-for-linus

 Thanks,

	Ingo

------------------>
Roland Dreier (1):
      hrtimer: Remove overly verbose "switch to high res mode" message


 kernel/hrtimer.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index e5d98ce..4267279 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -720,8 +720,6 @@ static int hrtimer_switch_to_hres(void)
 	/* "Retrigger" the interrupt to get things going */
 	retrigger_next_event(NULL);
 	local_irq_restore(flags);
-	printk(KERN_DEBUG "Switched to high resolution mode on CPU %d\n",
-	       smp_processor_id());
 	return 1;
 }
 

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-fixes-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git timers-fixes-for-linus

 Thanks,

	Ingo

------------------>
Thomas Gleixner (1):
      clocksource: Prevent potential kgdb dead lock


 kernel/time/clocksource.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index e85c234..1370083 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -343,7 +343,19 @@ static void clocksource_resume_watchdog(void)
 {
 	unsigned long flags;
 
-	spin_lock_irqsave(&watchdog_lock, flags);
+	/*
+	 * We use trylock here to avoid a potential dead lock when
+	 * kgdb calls this code after the kernel has been stopped with
+	 * watchdog_lock held. When watchdog_lock is held we just
+	 * return and accept, that the watchdog might trigger and mark
+	 * the monitored clock source (usually TSC) unstable.
+	 *
+	 * This does not affect the other caller clocksource_resume()
+	 * because at this point the kernel is UP, interrupts are
+	 * disabled and nothing can hold watchdog_lock.
+	 */
+	if (!spin_trylock_irqsave(&watchdog_lock, flags))
+		return;
 	clocksource_reset_watchdog();
 	spin_unlock_irqrestore(&watchdog_lock, flags);
 }
@@ -458,8 +470,8 @@ void clocksource_resume(void)
  * clocksource_touch_watchdog - Update watchdog
  *
  * Update the watchdog after exception contexts such as kgdb so as not
- * to incorrectly trip the watchdog.
- *
+ * to incorrectly trip the watchdog. This might fail when the kernel
+ * was stopped in code which holds watchdog_lock.
  */
 void clocksource_touch_watchdog(void)
 {

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-fixes-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git timers-fixes-for-linus

 Thanks,

	Ingo

------------------>
Kees Cook (1):
      timer debug: Hide kernel addresses via %pK in /proc/timer_list


 kernel/time/timer_list.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c
index 32a19f9..3258455 100644
--- a/kernel/time/timer_list.c
+++ b/kernel/time/timer_list.c
@@ -41,7 +41,7 @@ static void print_name_offset(struct seq_file *m, void *sym)
 	char symname[KSYM_NAME_LEN];
 
 	if (lookup_symbol_name((unsigned long)sym, symname) < 0)
-		SEQ_printf(m, "<%p>", sym);
+		SEQ_printf(m, "<%pK>", sym);
 	else
 		SEQ_printf(m, "%s", symname);
 }
@@ -112,7 +112,7 @@ next_one:
 static void
 print_base(struct seq_file *m, struct hrtimer_clock_base *base, u64 now)
 {
-	SEQ_printf(m, "  .base:       %p\n", base);
+	SEQ_printf(m, "  .base:       %pK\n", base);
 	SEQ_printf(m, "  .index:      %d\n",
 			base->index);
 	SEQ_printf(m, "  .resolution: %Lu nsecs\n",

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-fixes-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git timers-fixes-for-linus

 Thanks,

	Ingo

------------------>
Thomas Gleixner (1):
      clockevents: Prevent oneshot mode when broadcast device is periodic


 kernel/time/tick-broadcast.c |   10 ++++++++++
 kernel/time/tick-common.c    |    6 +++++-
 kernel/time/tick-internal.h  |    3 +++
 3 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 48b2761..a3b5aff 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -600,4 +600,14 @@ int tick_broadcast_oneshot_active(void)
 	return tick_broadcast_device.mode == TICKDEV_MODE_ONESHOT;
 }
 
+/*
+ * Check whether the broadcast device supports oneshot.
+ */
+bool tick_broadcast_oneshot_available(void)
+{
+	struct clock_event_device *bc = tick_broadcast_device.evtdev;
+
+	return bc ? bc->features & CLOCK_EVT_FEAT_ONESHOT : false;
+}
+
 #endif
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 051bc80..ed228ef 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -51,7 +51,11 @@ int tick_is_oneshot_available(void)
 {
 	struct clock_event_device *dev = __this_cpu_read(tick_cpu_device.evtdev);
 
-	return dev && (dev->features & CLOCK_EVT_FEAT_ONESHOT);
+	if (!dev || !(dev->features & CLOCK_EVT_FEAT_ONESHOT))
+		return 0;
+	if (!(dev->features & CLOCK_EVT_FEAT_C3STOP))
+		return 1;
+	return tick_broadcast_oneshot_available();
 }
 
 /*
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 290eefb..f65d3a7 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -36,6 +36,7 @@ extern void tick_shutdown_broadcast_oneshot(unsigned int *cpup);
 extern int tick_resume_broadcast_oneshot(struct clock_event_device *bc);
 extern int tick_broadcast_oneshot_active(void);
 extern void tick_check_oneshot_broadcast(int cpu);
+bool tick_broadcast_oneshot_available(void);
 # else /* BROADCAST */
 static inline void tick_broadcast_setup_oneshot(struct clock_event_device *bc)
 {
@@ -46,6 +47,7 @@ static inline void tick_broadcast_switch_to_oneshot(void) { }
 static inline void tick_shutdown_broadcast_oneshot(unsigned int *cpup) { }
 static inline int tick_broadcast_oneshot_active(void) { return 0; }
 static inline void tick_check_oneshot_broadcast(int cpu) { }
+static inline bool tick_broadcast_oneshot_available(void) { return true; }
 # endif /* !BROADCAST */
 
 #else /* !ONESHOT */
@@ -76,6 +78,7 @@ static inline int tick_resume_broadcast_oneshot(struct clock_event_device *bc)
 	return 0;
 }
 static inline int tick_broadcast_oneshot_active(void) { return 0; }
+static inline bool tick_broadcast_oneshot_available(void) { return false; }
 #endif /* !TICK_ONESHOT */
 
 /*

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timer-fixes-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git timer-fixes-for-linus

This fixes the scheduler bug discussed in the:

 "2.6.39-rc4+: Kernel leaking memory during FS scanning, regression?"

lkml thread.

 Thanks,

	Ingo

------------------>
Thomas Gleixner (1):
      hrtimer: Initialize CLOCK_ID to HRTIMER_BASE table statically

Zhangfei Gao (1):
      rtc: max8925: Call dev_set_drvdata before rtc_device_register


 drivers/rtc/rtc-max8925.c |    3 ++-
 kernel/hrtimer.c          |   10 +++++-----
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-max8925.c b/drivers/rtc/rtc-max8925.c
index 174036d..20494b5 100644
--- a/drivers/rtc/rtc-max8925.c
+++ b/drivers/rtc/rtc-max8925.c
@@ -257,6 +257,8 @@ static int __devinit max8925_rtc_probe(struct platform_device *pdev)
 		goto out_irq;
 	}
 
+	dev_set_drvdata(&pdev->dev, info);
+
 	info->rtc_dev = rtc_device_register("max8925-rtc", &pdev->dev,
 					&max8925_rtc_ops, THIS_MODULE);
 	ret = PTR_ERR(info->rtc_dev);
@@ -265,7 +267,6 @@ static int __devinit max8925_rtc_probe(struct platform_device *pdev)
 		goto out_rtc;
 	}
 
-	dev_set_drvdata(&pdev->dev, info);
 	platform_set_drvdata(pdev, info);
 
 	return 0;
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 9017478..87fdb3f 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -81,7 +81,11 @@ DEFINE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases) =
 	}
 };
 
-static int hrtimer_clock_to_base_table[MAX_CLOCKS];
+static int hrtimer_clock_to_base_table[MAX_CLOCKS] = {
+	[CLOCK_REALTIME]	= HRTIMER_BASE_REALTIME,
+	[CLOCK_MONOTONIC]	= HRTIMER_BASE_MONOTONIC,
+	[CLOCK_BOOTTIME]	= HRTIMER_BASE_BOOTTIME,
+};
 
 static inline int hrtimer_clockid_to_base(clockid_t clock_id)
 {
@@ -1722,10 +1726,6 @@ static struct notifier_block __cpuinitdata hrtimers_nb = {
 
 void __init hrtimers_init(void)
 {
-	hrtimer_clock_to_base_table[CLOCK_REALTIME] = HRTIMER_BASE_REALTIME;
-	hrtimer_clock_to_base_table[CLOCK_MONOTONIC] = HRTIMER_BASE_MONOTONIC;
-	hrtimer_clock_to_base_table[CLOCK_BOOTTIME] = HRTIMER_BASE_BOOTTIME;
-
 	hrtimer_cpu_notify(&hrtimers_nb, (unsigned long)CPU_UP_PREPARE,
 			  (void *)(long)smp_processor_id());
 	register_cpu_notifier(&hrtimers_nb);

Re: Linux 3.1-rc9

[Linus Torvalds]

On Thu, Oct 13, 2011 at 4:25 PM, Simon Kirby <sim@hostway.ca> wrote:
>
> Looks good. No hangs or crashes for two days on any of them running
> 3.1-rc9 plus this patch. Not sure if you want to deuglify it, but it
> seems to work...
>
> Tested-by: Simon Kirby <sim@hostway.ca>

Peter, what's the status of this one?

Quite frankly, I personally consider it to be broken - why are we
introducing this new lock for this very special thing? A spinlock to
protect a *single* word of counter seems broken.

It seems more likely that the real bug is that kernel/sched_stats.h
currently takes cputimer->lock without disabling interrupts. Everybody
else uses irq-safe locking, why would sched_stats.h not need that?

However, I don't see why that spinlock is needed at all. Why aren't
those fields just atomics (or at least just "sum_exec_runtime")? And
why does "cputime_add()" exist at all? It seems to always be just a
plain add, and nothing else would seem to ever make sense *anyway*?

In other words, none of that code makes any sense to me at all. And
the patch in question that fixes a hang for Simon seems to make it
even worse. Can somebody explain to me why it looks that crappy?

Please?

That stupid definition of cputime_add() has apparently existed as-is
since it was introduced in 2005. Why do we have code like this:

    times->utime = cputime_add(times->utime, t->utime);

instead of just

    times->utime += t->utime;

which seems not just shorter, but more readable too? The reason is not
some type safety in the cputime_add() thing, it's just a macro.

Added Martin and Ingo to the discussion - Martin because he added that
cputime_add in the first place, and Ingo because he gets the most hits
on kernel/sched_stats.h. Guys - you can see the history on lkml.

                                 Linus

Re: Linux 3.1-rc9

[Peter Zijlstra]

On Sun, 2011-10-16 at 18:39 -0700, Linus Torvalds wrote:

> Quite frankly, I personally consider it to be broken - why are we
> introducing this new lock for this very special thing? A spinlock to
> protect a *single* word of counter seems broken.

Well, I thought atomic64_t would be more expensive on 32bit archs, i386
uses the horridly expensive cmpxchg8b thing to implement it.

That said, I'm more than glad to use it.

> However, I don't see why that spinlock is needed at all. Why aren't
> those fields just atomics (or at least just "sum_exec_runtime")? 

Done.

> And
> why does "cputime_add()" exist at all? It seems to always be just a
> plain add, and nothing else would seem to ever make sense *anyway*?

Martin and me were discussing the merit of that only a few weeks ago ;-)

BTW what would we all think about a coccinelle generated patch that
fixes atomic*_add()'s argument order?

---
Subject: cputimer: Cure lock inversion
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Mon Oct 17 11:50:30 CEST 2011

There's a lock inversion between the cputimer->lock and rq->lock; notably
the two callchains involved are:

 update_rlimit_cpu()
   sighand->siglock
   set_process_cpu_timer()
     cpu_timer_sample_group()
       thread_group_cputimer()
         cputimer->lock
         thread_group_cputime()
           task_sched_runtime()
             ->pi_lock
             rq->lock

 scheduler_tick()
   rq->lock
   task_tick_fair()
     update_curr()
       account_group_exec()
         cputimer->lock

Where the first one is enabling a CLOCK_PROCESS_CPUTIME_ID timer, and the
second one is keeping up-to-date.

Note that e8abccb7193 ("posix-cpu-timers: Cure SMP accounting oddities") didn't
introduce this problem, but merely made it much more likely to happen, see how
cpu_timer_sample_group() for the CPUCLOCK_SCHED case also takes rq->lock.

Cure this inversion by removing the need to acquire cputimer->lock in the
update path by converting task_cputime::sum_exec_runtime to an atomic64_t.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 include/linux/sched.h     |    4 ++--
 kernel/fork.c             |    2 +-
 kernel/posix-cpu-timers.c |   41 ++++++++++++++++++++++++-----------------
 kernel/sched.c            |    2 +-
 kernel/sched_rt.c         |    6 ++++--
 kernel/sched_stats.h      |    4 +---
 6 files changed, 33 insertions(+), 26 deletions(-)
Index: linux-2.6/include/linux/sched.h
===================================================================
--- linux-2.6.orig/include/linux/sched.h
+++ linux-2.6/include/linux/sched.h
@@ -474,7 +474,7 @@ struct cpu_itimer {
 struct task_cputime {
 	cputime_t utime;
 	cputime_t stime;
-	unsigned long long sum_exec_runtime;
+	atomic64_t sum_exec_runtime;
 };
 /* Alternate field names when used to cache expirations. */
 #define prof_exp	stime
@@ -485,7 +485,7 @@ struct task_cputime {
 	(struct task_cputime) {					\
 		.utime = cputime_zero,				\
 		.stime = cputime_zero,				\
-		.sum_exec_runtime = 0,				\
+		.sum_exec_runtime = ATOMIC64_INIT(0),		\
 	}
 
 /*
Index: linux-2.6/kernel/fork.c
===================================================================
--- linux-2.6.orig/kernel/fork.c
+++ linux-2.6/kernel/fork.c
@@ -1033,7 +1033,7 @@ static void posix_cpu_timers_init(struct
 {
 	tsk->cputime_expires.prof_exp = cputime_zero;
 	tsk->cputime_expires.virt_exp = cputime_zero;
-	tsk->cputime_expires.sched_exp = 0;
+	atomic64_set(&tsk->cputime_expires.sched_exp, 0);
 	INIT_LIST_HEAD(&tsk->cpu_timers[0]);
 	INIT_LIST_HEAD(&tsk->cpu_timers[1]);
 	INIT_LIST_HEAD(&tsk->cpu_timers[2]);
Index: linux-2.6/kernel/posix-cpu-timers.c
===================================================================
--- linux-2.6.orig/kernel/posix-cpu-timers.c
+++ linux-2.6/kernel/posix-cpu-timers.c
@@ -239,7 +239,7 @@ void thread_group_cputime(struct task_st
 
 	times->utime = sig->utime;
 	times->stime = sig->stime;
-	times->sum_exec_runtime = sig->sum_sched_runtime;
+	atomic64_set(&times->sum_exec_runtime, sig->sum_sched_runtime);
 
 	rcu_read_lock();
 	/* make sure we can trust tsk->thread_group list */
@@ -250,7 +250,7 @@ void thread_group_cputime(struct task_st
 	do {
 		times->utime = cputime_add(times->utime, t->utime);
 		times->stime = cputime_add(times->stime, t->stime);
-		times->sum_exec_runtime += task_sched_runtime(t);
+		atomic64_add(task_sched_runtime(t), &times->sum_exec_runtime);
 	} while_each_thread(tsk, t);
 out:
 	rcu_read_unlock();
@@ -264,8 +264,11 @@ static void update_gt_cputime(struct tas
 	if (cputime_gt(b->stime, a->stime))
 		a->stime = b->stime;
 
-	if (b->sum_exec_runtime > a->sum_exec_runtime)
-		a->sum_exec_runtime = b->sum_exec_runtime;
+	if (atomic64_read(&b->sum_exec_runtime) >
+			atomic64_read(&a->sum_exec_runtime)) {
+		atomic64_set(&a->sum_exec_runtime,
+				atomic64_read(&b->sum_exec_runtime));
+	}
 }
 
 void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
@@ -287,6 +290,8 @@ void thread_group_cputimer(struct task_s
 		update_gt_cputime(&cputimer->cputime, &sum);
 	}
 	*times = cputimer->cputime;
+	atomic64_set(&times->sum_exec_runtime,
+			atomic64_read(&cputimer->cputime.sum_exec_runtime));
 	spin_unlock_irqrestore(&cputimer->lock, flags);
 }
 
@@ -313,7 +318,7 @@ static int cpu_clock_sample_group(const 
 		break;
 	case CPUCLOCK_SCHED:
 		thread_group_cputime(p, &cputime);
-		cpu->sched = cputime.sum_exec_runtime;
+		cpu->sched = atomic64_read(&cputime.sum_exec_runtime);
 		break;
 	}
 	return 0;
@@ -593,9 +598,9 @@ static void arm_timer(struct k_itimer *t
 				cputime_expires->virt_exp = exp->cpu;
 			break;
 		case CPUCLOCK_SCHED:
-			if (cputime_expires->sched_exp == 0 ||
-			    cputime_expires->sched_exp > exp->sched)
-				cputime_expires->sched_exp = exp->sched;
+			if (atomic64_read(&cputime_expires->sched_exp) == 0 ||
+			    atomic64_read(&cputime_expires->sched_exp) > exp->sched)
+				atomic64_set(&cputime_expires->sched_exp, exp->sched);
 			break;
 		}
 	}
@@ -656,7 +661,7 @@ static int cpu_timer_sample_group(const 
 		cpu->cpu = cputime.utime;
 		break;
 	case CPUCLOCK_SCHED:
-		cpu->sched = cputime.sum_exec_runtime + task_delta_exec(p);
+		cpu->sched = atomic64_read(&cputime.sum_exec_runtime) + task_delta_exec(p);
 		break;
 	}
 	return 0;
@@ -947,13 +952,14 @@ static void check_thread_timers(struct t
 
 	++timers;
 	maxfire = 20;
-	tsk->cputime_expires.sched_exp = 0;
+	atomic64_set(&tsk->cputime_expires.sched_exp, 0);
 	while (!list_empty(timers)) {
 		struct cpu_timer_list *t = list_first_entry(timers,
 						      struct cpu_timer_list,
 						      entry);
 		if (!--maxfire || tsk->se.sum_exec_runtime < t->expires.sched) {
-			tsk->cputime_expires.sched_exp = t->expires.sched;
+			atomic64_set(&tsk->cputime_expires.sched_exp,
+				     t->expires.sched);
 			break;
 		}
 		t->firing = 1;
@@ -1049,7 +1055,7 @@ static inline int task_cputime_zero(cons
 {
 	if (cputime_eq(cputime->utime, cputime_zero) &&
 	    cputime_eq(cputime->stime, cputime_zero) &&
-	    cputime->sum_exec_runtime == 0)
+	    atomic64_read(&cputime->sum_exec_runtime) == 0)
 		return 1;
 	return 0;
 }
@@ -1076,7 +1082,7 @@ static void check_process_timers(struct 
 	thread_group_cputimer(tsk, &cputime);
 	utime = cputime.utime;
 	ptime = cputime_add(utime, cputime.stime);
-	sum_sched_runtime = cputime.sum_exec_runtime;
+	sum_sched_runtime = atomic64_read(&cputime.sum_exec_runtime);
 	maxfire = 20;
 	prof_expires = cputime_zero;
 	while (!list_empty(timers)) {
@@ -1161,7 +1167,7 @@ static void check_process_timers(struct 
 
 	sig->cputime_expires.prof_exp = prof_expires;
 	sig->cputime_expires.virt_exp = virt_expires;
-	sig->cputime_expires.sched_exp = sched_expires;
+	atomic64_set(&sig->cputime_expires.sched_exp, sched_expires);
 	if (task_cputime_zero(&sig->cputime_expires))
 		stop_process_timers(sig);
 }
@@ -1255,8 +1261,9 @@ static inline int task_cputime_expired(c
 	    cputime_ge(cputime_add(sample->utime, sample->stime),
 		       expires->stime))
 		return 1;
-	if (expires->sum_exec_runtime != 0 &&
-	    sample->sum_exec_runtime >= expires->sum_exec_runtime)
+	if (atomic64_read(&expires->sum_exec_runtime) != 0 &&
+	    atomic64_read(&sample->sum_exec_runtime) >=
+			atomic64_read(&expires->sum_exec_runtime))
 		return 1;
 	return 0;
 }
@@ -1279,7 +1286,7 @@ static inline int fastpath_timer_check(s
 		struct task_cputime task_sample = {
 			.utime = tsk->utime,
 			.stime = tsk->stime,
-			.sum_exec_runtime = tsk->se.sum_exec_runtime
+			.sum_exec_runtime = ATOMIC64_INIT(tsk->se.sum_exec_runtime),
 		};
 
 		if (task_cputime_expired(&task_sample, &tsk->cputime_expires))
Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c
+++ linux-2.6/kernel/sched.c
@@ -4075,7 +4075,7 @@ void thread_group_times(struct task_stru
 	thread_group_cputime(p, &cputime);
 
 	total = cputime_add(cputime.utime, cputime.stime);
-	rtime = nsecs_to_cputime(cputime.sum_exec_runtime);
+	rtime = nsecs_to_cputime(atomic64_read(&cputime.sum_exec_runtime));
 
 	if (total) {
 		u64 temp = rtime;
Index: linux-2.6/kernel/sched_rt.c
===================================================================
--- linux-2.6.orig/kernel/sched_rt.c
+++ linux-2.6/kernel/sched_rt.c
@@ -1763,8 +1763,10 @@ static void watchdog(struct rq *rq, stru
 
 		p->rt.timeout++;
 		next = DIV_ROUND_UP(min(soft, hard), USEC_PER_SEC/HZ);
-		if (p->rt.timeout > next)
-			p->cputime_expires.sched_exp = p->se.sum_exec_runtime;
+		if (p->rt.timeout > next) {
+			atomic64_set(&p->cputime_expires.sched_exp,
+					p->se.sum_exec_runtime);
+		}
 	}
 }
 
Index: linux-2.6/kernel/sched_stats.h
===================================================================
--- linux-2.6.orig/kernel/sched_stats.h
+++ linux-2.6/kernel/sched_stats.h
@@ -330,7 +330,5 @@ static inline void account_group_exec_ru
 	if (!cputimer->running)
 		return;
 
-	spin_lock(&cputimer->lock);
-	cputimer->cputime.sum_exec_runtime += ns;
-	spin_unlock(&cputimer->lock);
+	atomic64_add(ns, &cputimer->cputime.sum_exec_runtime);
 }

Re: Linux 3.1-rc9

[Linus Torvalds]

On Mon, Oct 17, 2011 at 3:34 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>
> Well, I thought atomic64_t would be more expensive on 32bit archs, i386
> uses the horridly expensive cmpxchg8b thing to implement it.

Ugh, yes. And some of those paths seem to be hot-paths too.

Perhaps more importantly, there are way more accesses to that
'sum_exec_runtime' than the spinlock-variant of the patch implied.

So now with the atomic64 variant, the readers are protected too, and
that ends up being really expensive. That may be the "right thing" to
do, but I'm not sure if it's really acceptable. Also, I see that some
of the atomic regions (that weren't protected by the spinlock *either*
aren't just simple adds: they are code like

+                       if (atomic64_read(&cputime_expires->sched_exp) == 0 ||
+                           atomic64_read(&cputime_expires->sched_exp)
> exp->sched)
+
atomic64_set(&cputime_expires->sched_exp, exp->sched);

in arm_timer(), which was apparently totally unprotected before, and
which is just inappropriate with atomic accesses.

So seeing this, I'm not confident that atomic64 works at all, after all.

Grrr..

                               Linus

Re: Linux 3.1-rc9

[Peter Zijlstra]

On Mon, 2011-10-17 at 07:57 -0700, Linus Torvalds wrote:

> So seeing this, I'm not confident that atomic64 works at all, after all.

I could of course propose this... but I really won't since I'm half
retching by now.. ;-)


---
 include/linux/sched.h     |    7 +++++--
 kernel/posix-cpu-timers.c |    8 +++++---
 kernel/sched_stats.h      |    4 +---
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 41d0237..94bf16f 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -474,7 +474,10 @@ struct cpu_itimer {
 struct task_cputime {
 	cputime_t utime;
 	cputime_t stime;
-	unsigned long long sum_exec_runtime;
+	union {
+		unsigned long long sum_exec_runtime;
+		atomic64_t _sum_exec_runtime;
+	};
 };
 /* Alternate field names when used to cache expirations. */
 #define prof_exp	stime
@@ -485,7 +488,7 @@ struct task_cputime {
 	(struct task_cputime) {					\
 		.utime = cputime_zero,				\
 		.stime = cputime_zero,				\
-		.sum_exec_runtime = 0,				\
+		{ .sum_exec_runtime = 0, },			\
 	}
 
 /*
diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index c8008dd..4808c0d 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/posix-cpu-timers.c
@@ -264,8 +264,8 @@ static void update_gt_cputime(struct task_cputime *a, struct task_cputime *b)
 	if (cputime_gt(b->stime, a->stime))
 		a->stime = b->stime;
 
-	if (b->sum_exec_runtime > a->sum_exec_runtime)
-		a->sum_exec_runtime = b->sum_exec_runtime;
+	if (b->sum_exec_runtime > atomic64_read(&a->_sum_exec_runtime))
+		atomic64_set(&a->_sum_exec_runtime, b->sum_exec_runtime);
 }
 
 void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
@@ -287,6 +287,8 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
 		update_gt_cputime(&cputimer->cputime, &sum);
 	}
 	*times = cputimer->cputime;
+	times->sum_exec_runtime = 
+		atomic64_read(&cputimer->cputime._sum_exec_runtime);
 	spin_unlock_irqrestore(&cputimer->lock, flags);
 }
 
@@ -1279,7 +1281,7 @@ static inline int fastpath_timer_check(struct task_struct *tsk)
 		struct task_cputime task_sample = {
 			.utime = tsk->utime,
 			.stime = tsk->stime,
-			.sum_exec_runtime = tsk->se.sum_exec_runtime
+			{ .sum_exec_runtime = tsk->se.sum_exec_runtime, },
 		};
 
 		if (task_cputime_expired(&task_sample, &tsk->cputime_expires))
diff --git a/kernel/sched_stats.h b/kernel/sched_stats.h
index 331e01b..65dcb76 100644
--- a/kernel/sched_stats.h
+++ b/kernel/sched_stats.h
@@ -330,7 +330,5 @@ static inline void account_group_exec_runtime(struct task_struct *tsk,
 	if (!cputimer->running)
 		return;
 
-	spin_lock(&cputimer->lock);
-	cputimer->cputime.sum_exec_runtime += ns;
-	spin_unlock(&cputimer->lock);
+	atomic64_add(ns, &cputimer->cputime._sum_exec_runtime);
 }

Re: Linux 3.1-rc9

[Linus Torvalds]

On Mon, Oct 17, 2011 at 10:54 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>
> I could of course propose this... but I really won't since I'm half
> retching by now.. ;-)

Wow. Is this "ugly and fragile code week" and I just didn't get the memo?

I do wonder if we might not fix the problem by just taking the
*existing* lock in the right order?

IOW, how nasty would be it be to make "scheduler_tick()" just get the
cputimer->lock outside or rq->lock?

Sure, we'd hold that lock *much* longer than we need, but how much do
we care? Is that a lock that gets contention? It migth be the simple
solution for now - I *would* like to get 3.1 out..

                        Linus

Re: Linux 3.1-rc9

[Peter Zijlstra]

On Mon, 2011-10-17 at 11:31 -0700, Linus Torvalds wrote:
> On Mon, Oct 17, 2011 at 10:54 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> >
> > I could of course propose this... but I really won't since I'm half
> > retching by now.. ;-)
> 
> Wow. Is this "ugly and fragile code week" and I just didn't get the memo?

Do I get a price?

> I do wonder if we might not fix the problem by just taking the
> *existing* lock in the right order?
> 
> IOW, how nasty would be it be to make "scheduler_tick()" just get the
> cputimer->lock outside or rq->lock?
> 
> Sure, we'd hold that lock *much* longer than we need, but how much do
> we care? Is that a lock that gets contention? It migth be the simple
> solution for now - I *would* like to get 3.1 out..

Ah, sadly the tick isn't the only one with the inverted callchain,
pretty much every callchain in the scheduler ends up in update_curr()
one way or another.

The easier way around might be something like this... even when two
threads in a process race to enable this clock the the wasted time is
pretty much of the same order as we would otherwise have wasted spinning
on the lock and the update_gt_cputime() think would end up moving the
clock fwd to the latest outcome any which way.

Humm,. Thomas anything?


---
diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index c8008dd..640ded8 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/posix-cpu-timers.c
@@ -274,9 +274,7 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
 	struct task_cputime sum;
 	unsigned long flags;
 
-	spin_lock_irqsave(&cputimer->lock, flags);
 	if (!cputimer->running) {
-		cputimer->running = 1;
 		/*
 		 * The POSIX timer interface allows for absolute time expiry
 		 * values through the TIMER_ABSTIME flag, therefore we have
@@ -284,8 +282,11 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
 		 * it.
 		 */
 		thread_group_cputime(tsk, &sum);
+		spin_lock_irqsave(&cputimer->lock, flags);
+		cputimer->running = 1;
 		update_gt_cputime(&cputimer->cputime, &sum);
-	}
+	} else
+		spin_lock_irqsave(&cputimer->lock, flags);
 	*times = cputimer->cputime;
 	spin_unlock_irqrestore(&cputimer->lock, flags);
 }

Re: Linux 3.1-rc9

[Thomas Gleixner]

On Mon, 17 Oct 2011, Peter Zijlstra wrote:

> On Mon, 2011-10-17 at 11:31 -0700, Linus Torvalds wrote:
> > On Mon, Oct 17, 2011 at 10:54 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> > >
> > > I could of course propose this... but I really won't since I'm half
> > > retching by now.. ;-)
> > 
> > Wow. Is this "ugly and fragile code week" and I just didn't get the memo?
> 
> Do I get a price?
> 
> > I do wonder if we might not fix the problem by just taking the
> > *existing* lock in the right order?
> > 
> > IOW, how nasty would be it be to make "scheduler_tick()" just get the
> > cputimer->lock outside or rq->lock?
> > 
> > Sure, we'd hold that lock *much* longer than we need, but how much do
> > we care? Is that a lock that gets contention? It migth be the simple
> > solution for now - I *would* like to get 3.1 out..
> 
> Ah, sadly the tick isn't the only one with the inverted callchain,
> pretty much every callchain in the scheduler ends up in update_curr()
> one way or another.
> 
> The easier way around might be something like this... even when two
> threads in a process race to enable this clock the the wasted time is
> pretty much of the same order as we would otherwise have wasted spinning
> on the lock and the update_gt_cputime() think would end up moving the
> clock fwd to the latest outcome any which way.
> 
> Humm,. Thomas anything?
 
No, that should work. It does not make that call path more racy
against exit, which is another trainwreck at least on 32bit machines
which I discovered while looking for the problems with your patch.

thread_group_cputime() reads task->signal->utime/stime/sum_sched_runtime

These fields are updated in __exit_signal() w/o holding
task->signal->cputimer.lock. So nothing prevents that these values
change while we read them.

All callers of thread_group_cputime() except the scheduler callpath
hold sighand lock, which is also taken in __exit_signal().

So your patch does not make that particular case worse.

That said, I really need some sleep before I can make a final
judgement on that horror. The call paths are such an intermingled mess
that it's not funny anymore. I do that tomorrow morning first thing.

Thanks,

	tglx

Re: Linux 3.1-rc9

[Thomas Gleixner]

On Mon, 17 Oct 2011, Thomas Gleixner wrote:
> That said, I really need some sleep before I can make a final
> judgement on that horror. The call paths are such an intermingled mess
> that it's not funny anymore. I do that tomorrow morning first thing.

The patch is safe and the exit race just existed in my confused tired
brain. Peter, can you please provide a changelog. That wants a cc
stable as well, because that deadlock causing commit hit 3.0.7 :(

Thanks,

	tglx

Re: Linux 3.1-rc9

[Peter Zijlstra]

On Tue, 2011-10-18 at 10:39 +0200, Thomas Gleixner wrote:
> On Mon, 17 Oct 2011, Thomas Gleixner wrote:
> > That said, I really need some sleep before I can make a final
> > judgement on that horror. The call paths are such an intermingled mess
> > that it's not funny anymore. I do that tomorrow morning first thing.
> 
> The patch is safe and the exit race just existed in my confused tired
> brain. Peter, can you please provide a changelog. That wants a cc
> stable as well, because that deadlock causing commit hit 3.0.7 :( 

---
Subject: cputimer: Cure lock inversion
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Mon Oct 17 11:50:30 CEST 2011

There's a lock inversion between the cputimer->lock and rq->lock; notably
the two callchains involved are:

 update_rlimit_cpu()
   sighand->siglock
   set_process_cpu_timer()
     cpu_timer_sample_group()
       thread_group_cputimer()
         cputimer->lock
         thread_group_cputime()
           task_sched_runtime()
             ->pi_lock
             rq->lock

 scheduler_tick()
   rq->lock
   task_tick_fair()
     update_curr()
       account_group_exec()
         cputimer->lock

Where the first one is enabling a CLOCK_PROCESS_CPUTIME_ID timer, and
the second one is keeping up-to-date.

This problem was introduced by e8abccb7193 ("posix-cpu-timers: Cure
SMP accounting oddities").

Cure the problem by removing the cputimer->lock and rq->lock nesting,
this leaves concurrent enablers doing duplicate work, but the time
wasted should be on the same order otherwise wasted spinning on the
lock and the greater-than assignment filter should ensure we preserve
monotonicity.

Reported-by: Dave Jones <davej@redhat.com>
Reported-by: Simon Kirby <sim@hostway.ca>
Cc: stable@kernel.org
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 kernel/posix-cpu-timers.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
Index: linux-2.6/kernel/posix-cpu-timers.c
===================================================================
--- linux-2.6.orig/kernel/posix-cpu-timers.c
+++ linux-2.6/kernel/posix-cpu-timers.c
@@ -274,9 +274,7 @@ void thread_group_cputimer(struct task_s
 	struct task_cputime sum;
 	unsigned long flags;
 
-	spin_lock_irqsave(&cputimer->lock, flags);
 	if (!cputimer->running) {
-		cputimer->running = 1;
 		/*
 		 * The POSIX timer interface allows for absolute time expiry
 		 * values through the TIMER_ABSTIME flag, therefore we have
@@ -284,8 +282,11 @@ void thread_group_cputimer(struct task_s
 		 * it.
 		 */
 		thread_group_cputime(tsk, &sum);
+		spin_lock_irqsave(&cputimer->lock, flags);
+		cputimer->running = 1;
 		update_gt_cputime(&cputimer->cputime, &sum);
-	}
+	} else
+		spin_lock_irqsave(&cputimer->lock, flags);
 	*times = cputimer->cputime;
 	spin_unlock_irqrestore(&cputimer->lock, flags);
 }

Re: Linux 3.1-rc9

[Linus Torvalds]

On Tue, Oct 18, 2011 at 2:05 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>
> Subject: cputimer: Cure lock inversion
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date: Mon Oct 17 11:50:30 CEST 2011
>
> There's a lock inversion between the cputimer->lock and rq->lock; notably
> the two callchains involved are:

Thanks, looks nice and small. Simon - can you check that this works for you?

Thomas/Ingo - once confirmed by Simon, should I take it directly or
will this come through your trees?

                                 Linus

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://tesla.tglx.de/git/linux-2.6-tip.git timers-urgent-for-linus

 Thanks,

	Ingo

------------------>
Peter Zijlstra (1):
      cputimer: Cure lock inversion


 kernel/posix-cpu-timers.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index c8008dd..640ded8 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/posix-cpu-timers.c
@@ -274,9 +274,7 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
 	struct task_cputime sum;
 	unsigned long flags;
 
-	spin_lock_irqsave(&cputimer->lock, flags);
 	if (!cputimer->running) {
-		cputimer->running = 1;
 		/*
 		 * The POSIX timer interface allows for absolute time expiry
 		 * values through the TIMER_ABSTIME flag, therefore we have
@@ -284,8 +282,11 @@ void thread_group_cputimer(struct task_struct *tsk, struct task_cputime *times)
 		 * it.
 		 */
 		thread_group_cputime(tsk, &sum);
+		spin_lock_irqsave(&cputimer->lock, flags);
+		cputimer->running = 1;
 		update_gt_cputime(&cputimer->cputime, &sum);
-	}
+	} else
+		spin_lock_irqsave(&cputimer->lock, flags);
 	*times = cputimer->cputime;
 	spin_unlock_irqrestore(&cputimer->lock, flags);
 }

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   HEAD: 7bd36014460f793c19e7d6c94dab67b0afcfcb7f timekeeping: Fix HRTICK related deadlock from ntp lock changes

An NTP related lockup fix.

 Thanks,

	Ingo

------------------>
John Stultz (1):
      timekeeping: Fix HRTICK related deadlock from ntp lock changes


 include/linux/timex.h     | 1 +
 kernel/time/ntp.c         | 6 ++----
 kernel/time/timekeeping.c | 2 ++
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/linux/timex.h b/include/linux/timex.h
index b3726e6..dd3edd7 100644
--- a/include/linux/timex.h
+++ b/include/linux/timex.h
@@ -141,6 +141,7 @@ extern int do_adjtimex(struct timex *);
 extern void hardpps(const struct timespec *, const struct timespec *);
 
 int read_current_timer(unsigned long *timer_val);
+void ntp_notify_cmos_timer(void);
 
 /* The clock frequency of the i8253/i8254 PIT */
 #define PIT_TICK_RATE 1193182ul
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 8f5b3b9..bb22151 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -516,13 +516,13 @@ static void sync_cmos_clock(struct work_struct *work)
 	schedule_delayed_work(&sync_cmos_work, timespec_to_jiffies(&next));
 }
 
-static void notify_cmos_timer(void)
+void ntp_notify_cmos_timer(void)
 {
 	schedule_delayed_work(&sync_cmos_work, 0);
 }
 
 #else
-static inline void notify_cmos_timer(void) { }
+void ntp_notify_cmos_timer(void) { }
 #endif
 
 
@@ -687,8 +687,6 @@ int __do_adjtimex(struct timex *txc, struct timespec *ts, s32 *time_tai)
 	if (!(time_status & STA_NANO))
 		txc->time.tv_usec /= NSEC_PER_USEC;
 
-	notify_cmos_timer();
-
 	return result;
 }
 
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 48b9fff..947ba25 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1703,6 +1703,8 @@ int do_adjtimex(struct timex *txc)
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 
+	ntp_notify_cmos_timer();
+
 	return ret;
 }
 

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 97b9410643475d6557d2517c2aff9fd2221141a9 clockevents: Sanitize ticks to nsec conversion

This tree contains a clockevents regression fix for certain ARM 
subarchitectures.

 Thanks,

	Ingo

------------------>
Thomas Gleixner (1):
      clockevents: Sanitize ticks to nsec conversion


 kernel/time/clockevents.c | 65 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 50 insertions(+), 15 deletions(-)

diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 38959c8..662c579 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -33,29 +33,64 @@ struct ce_unbind {
 	int res;
 };
 
-/**
- * clockevents_delta2ns - Convert a latch value (device ticks) to nanoseconds
- * @latch:	value to convert
- * @evt:	pointer to clock event device descriptor
- *
- * Math helper, returns latch value converted to nanoseconds (bound checked)
- */
-u64 clockevent_delta2ns(unsigned long latch, struct clock_event_device *evt)
+static u64 cev_delta2ns(unsigned long latch, struct clock_event_device *evt,
+			bool ismax)
 {
 	u64 clc = (u64) latch << evt->shift;
+	u64 rnd;
 
 	if (unlikely(!evt->mult)) {
 		evt->mult = 1;
 		WARN_ON(1);
 	}
+	rnd = (u64) evt->mult - 1;
+
+	/*
+	 * Upper bound sanity check. If the backwards conversion is
+	 * not equal latch, we know that the above shift overflowed.
+	 */
+	if ((clc >> evt->shift) != (u64)latch)
+		clc = ~0ULL;
+
+	/*
+	 * Scaled math oddities:
+	 *
+	 * For mult <= (1 << shift) we can safely add mult - 1 to
+	 * prevent integer rounding loss. So the backwards conversion
+	 * from nsec to device ticks will be correct.
+	 *
+	 * For mult > (1 << shift), i.e. device frequency is > 1GHz we
+	 * need to be careful. Adding mult - 1 will result in a value
+	 * which when converted back to device ticks can be larger
+	 * than latch by up to (mult - 1) >> shift. For the min_delta
+	 * calculation we still want to apply this in order to stay
+	 * above the minimum device ticks limit. For the upper limit
+	 * we would end up with a latch value larger than the upper
+	 * limit of the device, so we omit the add to stay below the
+	 * device upper boundary.
+	 *
+	 * Also omit the add if it would overflow the u64 boundary.
+	 */
+	if ((~0ULL - clc > rnd) &&
+	    (!ismax || evt->mult <= (1U << evt->shift)))
+		clc += rnd;
 
 	do_div(clc, evt->mult);
-	if (clc < 1000)
-		clc = 1000;
-	if (clc > KTIME_MAX)
-		clc = KTIME_MAX;
 
-	return clc;
+	/* Deltas less than 1usec are pointless noise */
+	return clc > 1000 ? clc : 1000;
+}
+
+/**
+ * clockevents_delta2ns - Convert a latch value (device ticks) to nanoseconds
+ * @latch:	value to convert
+ * @evt:	pointer to clock event device descriptor
+ *
+ * Math helper, returns latch value converted to nanoseconds (bound checked)
+ */
+u64 clockevent_delta2ns(unsigned long latch, struct clock_event_device *evt)
+{
+	return cev_delta2ns(latch, evt, false);
 }
 EXPORT_SYMBOL_GPL(clockevent_delta2ns);
 
@@ -380,8 +415,8 @@ void clockevents_config(struct clock_event_device *dev, u32 freq)
 		sec = 600;
 
 	clockevents_calc_mult_shift(dev, freq, sec);
-	dev->min_delta_ns = clockevent_delta2ns(dev->min_delta_ticks, dev);
-	dev->max_delta_ns = clockevent_delta2ns(dev->max_delta_ticks, dev);
+	dev->min_delta_ns = cev_delta2ns(dev->min_delta_ticks, dev, false);
+	dev->max_delta_ns = cev_delta2ns(dev->max_delta_ticks, dev, true);
 }
 
 /**

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: e59da0aedb573a347fa501fa63d3ff5055aa1bc7 Merge branch 'clockevents/3.13-fixes' of git://git.linaro.org/people/daniel.lezcano/linux into timers/urgent

It contains a crash fix for the ARM Cadence TTC clock driver.

 Thanks,

	Ingo

------------------>
Soren Brinkmann (1):
      clocksource: cadence_ttc: Fix mutex taken inside interrupt context


 drivers/clocksource/cadence_ttc_timer.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/clocksource/cadence_ttc_timer.c b/drivers/clocksource/cadence_ttc_timer.c
index b2bb3a4b..a92350b 100644
--- a/drivers/clocksource/cadence_ttc_timer.c
+++ b/drivers/clocksource/cadence_ttc_timer.c
@@ -67,11 +67,13 @@
  * struct ttc_timer - This definition defines local timer structure
  *
  * @base_addr:	Base address of timer
+ * @freq:	Timer input clock frequency
  * @clk:	Associated clock source
  * @clk_rate_change_nb	Notifier block for clock rate changes
  */
 struct ttc_timer {
 	void __iomem *base_addr;
+	unsigned long freq;
 	struct clk *clk;
 	struct notifier_block clk_rate_change_nb;
 };
@@ -196,9 +198,8 @@ static void ttc_set_mode(enum clock_event_mode mode,
 
 	switch (mode) {
 	case CLOCK_EVT_MODE_PERIODIC:
-		ttc_set_interval(timer,
-				DIV_ROUND_CLOSEST(clk_get_rate(ttce->ttc.clk),
-					PRESCALE * HZ));
+		ttc_set_interval(timer, DIV_ROUND_CLOSEST(ttce->ttc.freq,
+						PRESCALE * HZ));
 		break;
 	case CLOCK_EVT_MODE_ONESHOT:
 	case CLOCK_EVT_MODE_UNUSED:
@@ -273,6 +274,8 @@ static void __init ttc_setup_clocksource(struct clk *clk, void __iomem *base)
 		return;
 	}
 
+	ttccs->ttc.freq = clk_get_rate(ttccs->ttc.clk);
+
 	ttccs->ttc.clk_rate_change_nb.notifier_call =
 		ttc_rate_change_clocksource_cb;
 	ttccs->ttc.clk_rate_change_nb.next = NULL;
@@ -298,16 +301,14 @@ static void __init ttc_setup_clocksource(struct clk *clk, void __iomem *base)
 	__raw_writel(CNT_CNTRL_RESET,
 		     ttccs->ttc.base_addr + TTC_CNT_CNTRL_OFFSET);
 
-	err = clocksource_register_hz(&ttccs->cs,
-			clk_get_rate(ttccs->ttc.clk) / PRESCALE);
+	err = clocksource_register_hz(&ttccs->cs, ttccs->ttc.freq / PRESCALE);
 	if (WARN_ON(err)) {
 		kfree(ttccs);
 		return;
 	}
 
 	ttc_sched_clock_val_reg = base + TTC_COUNT_VAL_OFFSET;
-	setup_sched_clock(ttc_sched_clock_read, 16,
-			clk_get_rate(ttccs->ttc.clk) / PRESCALE);
+	setup_sched_clock(ttc_sched_clock_read, 16, ttccs->ttc.freq / PRESCALE);
 }
 
 static int ttc_rate_change_clockevent_cb(struct notifier_block *nb,
@@ -334,6 +335,9 @@ static int ttc_rate_change_clockevent_cb(struct notifier_block *nb,
 				ndata->new_rate / PRESCALE);
 		local_irq_restore(flags);
 
+		/* update cached frequency */
+		ttc->freq = ndata->new_rate;
+
 		/* fall through */
 	}
 	case PRE_RATE_CHANGE:
@@ -367,6 +371,7 @@ static void __init ttc_setup_clockevent(struct clk *clk,
 	if (clk_notifier_register(ttcce->ttc.clk,
 				&ttcce->ttc.clk_rate_change_nb))
 		pr_warn("Unable to register clock notifier.\n");
+	ttcce->ttc.freq = clk_get_rate(ttcce->ttc.clk);
 
 	ttcce->ttc.base_addr = base;
 	ttcce->ce.name = "ttc_clockevent";
@@ -396,7 +401,7 @@ static void __init ttc_setup_clockevent(struct clk *clk,
 	}
 
 	clockevents_config_and_register(&ttcce->ce,
-			clk_get_rate(ttcce->ttc.clk) / PRESCALE, 1, 0xfffe);
+			ttcce->ttc.freq / PRESCALE, 1, 0xfffe);
 }
 
 /**

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: cab5e127eef040399902caa8e1510795583fa03a time: Revert to calling clock_was_set_delayed() while in irq context

A late breaking fix from John. (The bug fixed has a hard lockup 
potential, but that was not observed, warnings were.)

 Thanks,

	Ingo

------------------>
John Stultz (1):
      time: Revert to calling clock_was_set_delayed() while in irq context


 kernel/time/timekeeping.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 0aa4ce8..5b40279 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1435,7 +1435,8 @@ void update_wall_time(void)
 out:
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 	if (clock_set)
-		clock_was_set();
+		/* Have to call _delayed version, since in irq context*/
+		clock_was_set_delayed();
 }
 
 /**

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 2d926c15d629a13914ce3e5f26354f6a0ac99e70 hrtimer: Fix incorrect tai offset calculation for non high-res timer systems

A CLOCK_TAI early expiry fix.

 Thanks,

	Ingo

------------------>
John Stultz (1):
      hrtimer: Fix incorrect tai offset calculation for non high-res timer systems


 kernel/time/hrtimer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 37e50aadd471..d8c724cda37b 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -122,7 +122,7 @@ static void hrtimer_get_softirq_time(struct hrtimer_cpu_base *base)
 	mono = ktime_get_update_offsets_tick(&off_real, &off_boot, &off_tai);
 	boot = ktime_add(mono, off_boot);
 	xtim = ktime_add(mono, off_real);
-	tai = ktime_add(xtim, off_tai);
+	tai = ktime_add(mono, off_tai);
 
 	base->clock_base[HRTIMER_BASE_REALTIME].softirq_time = xtim;
 	base->clock_base[HRTIMER_BASE_MONOTONIC].softirq_time = mono;

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 0f44705175347ec96935d60b765b5d14ecc763bb tick: Move the export of tick_broadcast_oneshot_control to the proper place

Fix for a misplaced export that can cause build failures in certain (rare) Kconfig 
situations.

 Thanks,

	Ingo

------------------>
Thomas Gleixner (1):
      tick: Move the export of tick_broadcast_oneshot_control to the proper place


 kernel/time/tick-broadcast.c | 1 -
 kernel/time/tick-common.c    | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 52b9e199b5ac..f6aae7977824 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -839,7 +839,6 @@ int __tick_broadcast_oneshot_control(enum tick_broadcast_state state)
 	raw_spin_unlock(&tick_broadcast_lock);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(tick_broadcast_oneshot_control);
 
 /*
  * Reset the one shot broadcast for a cpu
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 55e13efff1ab..f8bf47571dda 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -363,6 +363,7 @@ int tick_broadcast_oneshot_control(enum tick_broadcast_state state)
 
 	return __tick_broadcast_oneshot_control(state);
 }
+EXPORT_SYMBOL_GPL(tick_broadcast_oneshot_control);
 
 #ifdef CONFIG_HOTPLUG_CPU
 /*

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 54d46b7fbcbd00fe4b20a27208e5909facc714e3 clockevents/drivers/sh_cmt: Only perform clocksource suspend/resume if enabled

A single clocksource driver suspend/resume fix.

 Thanks,

	Ingo

------------------>
Geert Uytterhoeven (1):
      clockevents/drivers/sh_cmt: Only perform clocksource suspend/resume if enabled


 drivers/clocksource/sh_cmt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/clocksource/sh_cmt.c b/drivers/clocksource/sh_cmt.c
index b8ff3c64cc45..c96de14036a0 100644
--- a/drivers/clocksource/sh_cmt.c
+++ b/drivers/clocksource/sh_cmt.c
@@ -661,6 +661,9 @@ static void sh_cmt_clocksource_suspend(struct clocksource *cs)
 {
 	struct sh_cmt_channel *ch = cs_to_sh_cmt(cs);
 
+	if (!ch->cs_enabled)
+		return;
+
 	sh_cmt_stop(ch, FLAG_CLOCKSOURCE);
 	pm_genpd_syscore_poweroff(&ch->cmt->pdev->dev);
 }
@@ -669,6 +672,9 @@ static void sh_cmt_clocksource_resume(struct clocksource *cs)
 {
 	struct sh_cmt_channel *ch = cs_to_sh_cmt(cs);
 
+	if (!ch->cs_enabled)
+		return;
+
 	pm_genpd_syscore_poweron(&ch->cmt->pdev->dev);
 	sh_cmt_start(ch, FLAG_CLOCKSOURCE);
 }

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 16eeed7e5558a3dcf30f75526a896b2632f299f9 clocksource/drivers/tango-xtal: Fix boot hang due to incorrect test

Fix a boot hang in the ARM based Tango SoC clocksource driver.

 Thanks,

	Ingo

------------------>
Daniel Lezcano (1):
      clocksource/drivers/tango-xtal: Fix boot hang due to incorrect test


 drivers/clocksource/tango_xtal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/tango_xtal.c b/drivers/clocksource/tango_xtal.c
index 2bcecafdeaea..c407c47a3232 100644
--- a/drivers/clocksource/tango_xtal.c
+++ b/drivers/clocksource/tango_xtal.c
@@ -42,7 +42,7 @@ static void __init tango_clocksource_init(struct device_node *np)
 
 	ret = clocksource_mmio_init(xtal_in_cnt, "tango-xtal", xtal_freq, 350,
 				    32, clocksource_mmio_readl_up);
-	if (!ret) {
+	if (ret) {
 		pr_err("%s: registration failed\n", np->full_name);
 		return;
 	}

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 2c13ce8f6b2f6fd9ba2f9261b1939fc0f62d1307 posix_cpu_timer: Exit early when process has been reaped

A single fix for a posix CPU timers bug.

 Thanks,

	Ingo

------------------>
Alexey Dobriyan (1):
      posix_cpu_timer: Exit early when process has been reaped


 kernel/time/posix-cpu-timers.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 1cafba860b08..39008d78927a 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -777,6 +777,7 @@ static void posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec *itp)
 			timer->it.cpu.expires = 0;
 			sample_to_timespec(timer->it_clock, timer->it.cpu.expires,
 					   &itp->it_value);
+			return;
 		} else {
 			cpu_timer_sample_group(timer->it_clock, p, &now);
 			unlock_task_sighand(p, &flags);

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 54e23845e965898f65f76aba79fa9db76d830fa9 alarmtimer: Remove unused but set variable

Remove an unused variable.

 Thanks,

	Ingo

------------------>
Tobias Klauser (1):
      alarmtimer: Remove unused but set variable


 kernel/time/alarmtimer.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index c3aad685bbc0..12dd190634ab 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -542,7 +542,6 @@ static int alarm_clock_get(clockid_t which_clock, struct timespec *tp)
 static int alarm_timer_create(struct k_itimer *new_timer)
 {
 	enum  alarmtimer_type type;
-	struct alarm_base *base;
 
 	if (!alarmtimer_get_rtcdev())
 		return -ENOTSUPP;
@@ -551,7 +550,6 @@ static int alarm_timer_create(struct k_itimer *new_timer)
 		return -EPERM;
 
 	type = clock2alarm(new_timer->it_clock);
-	base = &alarm_bases[type];
 	alarm_init(&new_timer->it.alarm.alarmtimer, type, alarm_handle_timer);
 	return 0;
 }

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: c9435f35ae64ee162555a82b6a3586b160093957 clocksource/drivers/moxart: Plug memory and mapping leaks

ARM/MOXA SoC clocksource driver fixes.

 Thanks,

	Ingo

------------------>
Sudip Mukherjee (1):
      clocksource/drivers/moxart: Plug memory and mapping leaks


 drivers/clocksource/moxart_timer.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/clocksource/moxart_timer.c b/drivers/clocksource/moxart_timer.c
index 2a8f4705c734..7f3430654fbd 100644
--- a/drivers/clocksource/moxart_timer.c
+++ b/drivers/clocksource/moxart_timer.c
@@ -161,19 +161,22 @@ static int __init moxart_timer_init(struct device_node *node)
 	timer->base = of_iomap(node, 0);
 	if (!timer->base) {
 		pr_err("%s: of_iomap failed\n", node->full_name);
-		return -ENXIO;
+		ret = -ENXIO;
+		goto out_free;
 	}
 
 	irq = irq_of_parse_and_map(node, 0);
 	if (irq <= 0) {
 		pr_err("%s: irq_of_parse_and_map failed\n", node->full_name);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_unmap;
 	}
 
 	clk = of_clk_get(node, 0);
 	if (IS_ERR(clk))  {
 		pr_err("%s: of_clk_get failed\n", node->full_name);
-		return PTR_ERR(clk);
+		ret = PTR_ERR(clk);
+		goto out_unmap;
 	}
 
 	pclk = clk_get_rate(clk);
@@ -186,7 +189,8 @@ static int __init moxart_timer_init(struct device_node *node)
 		timer->t1_disable_val = ASPEED_TIMER1_DISABLE;
 	} else {
 		pr_err("%s: unknown platform\n", node->full_name);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_unmap;
 	}
 
 	timer->count_per_tick = DIV_ROUND_CLOSEST(pclk, HZ);
@@ -208,14 +212,14 @@ static int __init moxart_timer_init(struct device_node *node)
 				    clocksource_mmio_readl_down);
 	if (ret) {
 		pr_err("%s: clocksource_mmio_init failed\n", node->full_name);
-		return ret;
+		goto out_unmap;
 	}
 
 	ret = request_irq(irq, moxart_timer_interrupt, IRQF_TIMER,
 			  node->name, &timer->clkevt);
 	if (ret) {
 		pr_err("%s: setup_irq failed\n", node->full_name);
-		return ret;
+		goto out_unmap;
 	}
 
 	/* Clear match registers */
@@ -241,6 +245,12 @@ static int __init moxart_timer_init(struct device_node *node)
 	clockevents_config_and_register(&timer->clkevt, pclk, 0x4, 0xfffffffe);
 
 	return 0;
+
+out_unmap:
+	iounmap(timer->base);
+out_free:
+	kfree(timer);
+	return ret;
 }
 CLOCKSOURCE_OF_DECLARE(moxart, "moxa,moxart-timer", moxart_timer_init);
 CLOCKSOURCE_OF_DECLARE(aspeed, "aspeed,ast2400-timer", moxart_timer_init);

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: bc7c36eedb0c7004aa06c2afc3c5385adada8fa3 clocksource/exynos_mct: Clear interrupt when cpu is shut down

Fix a crash in the ARM-Exynos clocksource driver, triggered by CPU hotplug 
operations.

 Thanks,

	Ingo

------------------>
Joonyoung Shim (1):
      clocksource/exynos_mct: Clear interrupt when cpu is shut down


 drivers/clocksource/exynos_mct.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 4da1dc2278bd..670ff0f25b67 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -495,6 +495,7 @@ static int exynos4_mct_dying_cpu(unsigned int cpu)
 	if (mct_int_type == MCT_INT_SPI) {
 		if (evt->irq != -1)
 			disable_irq_nosync(evt->irq);
+		exynos4_mct_write(0x1, mevt->base + MCT_L_INT_CSTAT_OFFSET);
 	} else {
 		disable_percpu_irq(mct_irqs[MCT_L0_IRQ]);
 	}

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: f63d947c1673930bfc5f2f9bd1073a02c179a890 clocksource/arm_arch_timer: Fix arch_timer_mem_find_best_frame()

A single ARM Juno clocksource driver fix.

 Thanks,

	Ingo

------------------>
Sudeep Holla (1):
      clocksource/arm_arch_timer: Fix arch_timer_mem_find_best_frame()


 drivers/clocksource/arm_arch_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index a1fb918b8021..4bed671e490e 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -1268,7 +1268,7 @@ arch_timer_mem_find_best_frame(struct arch_timer_mem *timer_mem)
 		pr_err("Unable to find a suitable frame in timer @ %pa\n",
 			&timer_mem->cntctlbase);
 
-	return frame;
+	return best_frame;
 }
 
 static int __init

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 32f2fea6e77e64cd4045ec2d5deb879aada3b476 clocksource/drivers/timer-of: Handle of_irq_get_byname() result correctly

A timer_irq_init() clocksource API robustness fix.

 Thanks,

	Ingo

------------------>
Sergei Shtylyov (1):
      clocksource/drivers/timer-of: Handle of_irq_get_byname() result correctly


 drivers/clocksource/timer-of.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
index f6e7491c873c..d509b500a7b5 100644
--- a/drivers/clocksource/timer-of.c
+++ b/drivers/clocksource/timer-of.c
@@ -41,8 +41,16 @@ static __init int timer_irq_init(struct device_node *np,
 	struct timer_of *to = container_of(of_irq, struct timer_of, of_irq);
 	struct clock_event_device *clkevt = &to->clkevt;
 
-	of_irq->irq = of_irq->name ? of_irq_get_byname(np, of_irq->name):
-		irq_of_parse_and_map(np, of_irq->index);
+	if (of_irq->name) {
+		of_irq->irq = ret = of_irq_get_byname(np, of_irq->name);
+		if (ret < 0) {
+			pr_err("Failed to get interrupt %s for %s\n",
+			       of_irq->name, np->full_name);
+			return ret;
+		}
+	} else	{
+		of_irq->irq = irq_of_parse_and_map(np, of_irq->index);
+	}
 	if (!of_irq->irq) {
 		pr_err("Failed to map interrupt for %s\n", np->full_name);
 		return -EINVAL;

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 2fe59f507a65dbd734b990a11ebc7488f6f87a24 timers: Fix excessive granularity of new timers after a nohz idle

Fix a timer granularity handling race+bug, which would manifest itself by 
spuriously increasing timeouts of some timers (from 1 jiffy to ~500 jiffies
in the worst case measured) in certain nohz states.

 Thanks,

	Ingo

------------------>
Nicholas Piggin (1):
      timers: Fix excessive granularity of new timers after a nohz idle


 kernel/time/timer.c | 50 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 9 deletions(-)

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 8f5d1bf18854..f2674a056c26 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -203,6 +203,7 @@ struct timer_base {
 	bool			migration_enabled;
 	bool			nohz_active;
 	bool			is_idle;
+	bool			must_forward_clk;
 	DECLARE_BITMAP(pending_map, WHEEL_SIZE);
 	struct hlist_head	vectors[WHEEL_SIZE];
 } ____cacheline_aligned;
@@ -856,13 +857,19 @@ get_target_base(struct timer_base *base, unsigned tflags)
 
 static inline void forward_timer_base(struct timer_base *base)
 {
-	unsigned long jnow = READ_ONCE(jiffies);
+	unsigned long jnow;
 
 	/*
-	 * We only forward the base when it's idle and we have a delta between
-	 * base clock and jiffies.
+	 * We only forward the base when we are idle or have just come out of
+	 * idle (must_forward_clk logic), and have a delta between base clock
+	 * and jiffies. In the common case, run_timers will take care of it.
 	 */
-	if (!base->is_idle || (long) (jnow - base->clk) < 2)
+	if (likely(!base->must_forward_clk))
+		return;
+
+	jnow = READ_ONCE(jiffies);
+	base->must_forward_clk = base->is_idle;
+	if ((long)(jnow - base->clk) < 2)
 		return;
 
 	/*
@@ -938,6 +945,11 @@ __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only)
 	 * same array bucket then just return:
 	 */
 	if (timer_pending(timer)) {
+		/*
+		 * The downside of this optimization is that it can result in
+		 * larger granularity than you would get from adding a new
+		 * timer with this expiry.
+		 */
 		if (timer->expires == expires)
 			return 1;
 
@@ -948,6 +960,7 @@ __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only)
 		 * dequeue/enqueue dance.
 		 */
 		base = lock_timer_base(timer, &flags);
+		forward_timer_base(base);
 
 		clk = base->clk;
 		idx = calc_wheel_index(expires, clk);
@@ -964,6 +977,7 @@ __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only)
 		}
 	} else {
 		base = lock_timer_base(timer, &flags);
+		forward_timer_base(base);
 	}
 
 	ret = detach_if_pending(timer, base, false);
@@ -991,12 +1005,10 @@ __mod_timer(struct timer_list *timer, unsigned long expires, bool pending_only)
 			raw_spin_lock(&base->lock);
 			WRITE_ONCE(timer->flags,
 				   (timer->flags & ~TIMER_BASEMASK) | base->cpu);
+			forward_timer_base(base);
 		}
 	}
 
-	/* Try to forward a stale timer base clock */
-	forward_timer_base(base);
-
 	timer->expires = expires;
 	/*
 	 * If 'idx' was calculated above and the base time did not advance
@@ -1112,6 +1124,7 @@ void add_timer_on(struct timer_list *timer, int cpu)
 		WRITE_ONCE(timer->flags,
 			   (timer->flags & ~TIMER_BASEMASK) | cpu);
 	}
+	forward_timer_base(base);
 
 	debug_activate(timer, timer->expires);
 	internal_add_timer(base, timer);
@@ -1497,10 +1510,16 @@ u64 get_next_timer_interrupt(unsigned long basej, u64 basem)
 		if (!is_max_delta)
 			expires = basem + (u64)(nextevt - basej) * TICK_NSEC;
 		/*
-		 * If we expect to sleep more than a tick, mark the base idle:
+		 * If we expect to sleep more than a tick, mark the base idle.
+		 * Also the tick is stopped so any added timer must forward
+		 * the base clk itself to keep granularity small. This idle
+		 * logic is only maintained for the BASE_STD base, deferrable
+		 * timers may still see large granularity skew (by design).
 		 */
-		if ((expires - basem) > TICK_NSEC)
+		if ((expires - basem) > TICK_NSEC) {
+			base->must_forward_clk = true;
 			base->is_idle = true;
+		}
 	}
 	raw_spin_unlock(&base->lock);
 
@@ -1611,6 +1630,19 @@ static __latent_entropy void run_timer_softirq(struct softirq_action *h)
 {
 	struct timer_base *base = this_cpu_ptr(&timer_bases[BASE_STD]);
 
+	/*
+	 * must_forward_clk must be cleared before running timers so that any
+	 * timer functions that call mod_timer will not try to forward the
+	 * base. idle trcking / clock forwarding logic is only used with
+	 * BASE_STD timers.
+	 *
+	 * The deferrable base does not do idle tracking at all, so we do
+	 * not forward it. This can result in very large variations in
+	 * granularity for deferrable timers, but they can be deferred for
+	 * long periods due to idle.
+	 */
+	base->must_forward_clk = false;
+
 	__run_timers(base);
 	if (IS_ENABLED(CONFIG_NO_HZ_COMMON) && base->nohz_active)
 		__run_timers(this_cpu_ptr(&timer_bases[BASE_DEF]));

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 8fce3dc5c5d6f6301f67311fa79f333902b58cea clocksource/integrator: Fix section mismatch warning

A clocksource driver section mismatch fix.

 Thanks,

	Ingo

------------------>
Arnd Bergmann (1):
      clocksource/integrator: Fix section mismatch warning


 drivers/clocksource/timer-integrator-ap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/timer-integrator-ap.c b/drivers/clocksource/timer-integrator-ap.c
index 2ff64d9d4fb3..62d24690ba02 100644
--- a/drivers/clocksource/timer-integrator-ap.c
+++ b/drivers/clocksource/timer-integrator-ap.c
@@ -36,8 +36,8 @@ static u64 notrace integrator_read_sched_clock(void)
 	return -readl(sched_clk_base + TIMER_VALUE);
 }
 
-static int integrator_clocksource_init(unsigned long inrate,
-				       void __iomem *base)
+static int __init integrator_clocksource_init(unsigned long inrate,
+					      void __iomem *base)
 {
 	u32 ctrl = TIMER_CTRL_ENABLE | TIMER_CTRL_PERIODIC;
 	unsigned long rate = inrate;

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 19b558db12f9f4e45a22012bae7b4783e62224da posix-timers: Protect posix clock array access against speculation

Make posix clock ID usage Spectre-safe.

 Thanks,

	Ingo

------------------>
Thomas Gleixner (1):
      posix-timers: Protect posix clock array access against speculation


 kernel/time/posix-timers.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index 75043046914e..10b7186d0638 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -50,6 +50,7 @@
 #include <linux/export.h>
 #include <linux/hashtable.h>
 #include <linux/compat.h>
+#include <linux/nospec.h>
 
 #include "timekeeping.h"
 #include "posix-timers.h"
@@ -1346,11 +1347,15 @@ static const struct k_clock * const posix_clocks[] = {
 
 static const struct k_clock *clockid_to_kclock(const clockid_t id)
 {
-	if (id < 0)
+	clockid_t idx = id;
+
+	if (id < 0) {
 		return (id & CLOCKFD_MASK) == CLOCKFD ?
 			&clock_posix_dynamic : &clock_posix_cpu;
+	}
 
-	if (id >= ARRAY_SIZE(posix_clocks) || !posix_clocks[id])
+	if (id >= ARRAY_SIZE(posix_clocks))
 		return NULL;
-	return posix_clocks[id];
+
+	return posix_clocks[array_index_nospec(idx, ARRAY_SIZE(posix_clocks))];
 }

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 0e334db6bb4b1fd1e2d72c1f3d8f004313cd9f94 posix-timers: Fix division by zero bug

Fix a division by zero crash in the posix-timers code.

 Thanks,

	Ingo

------------------>
Thomas Gleixner (1):
      posix-timers: Fix division by zero bug


 kernel/time/posix-timers.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index bd62b5eeb5a0..31f49ae80f43 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -289,9 +289,6 @@ static void common_hrtimer_rearm(struct k_itimer *timr)
 {
 	struct hrtimer *timer = &timr->it.real.timer;
 
-	if (!timr->it_interval)
-		return;
-
 	timr->it_overrun += hrtimer_forward(timer, timer->base->get_time(),
 					    timr->it_interval);
 	hrtimer_restart(timer);
@@ -317,7 +314,7 @@ void posixtimer_rearm(struct kernel_siginfo *info)
 	if (!timr)
 		return;
 
-	if (timr->it_requeue_pending == info->si_sys_private) {
+	if (timr->it_interval && timr->it_requeue_pending == info->si_sys_private) {
 		timr->kclock->timer_rearm(timr);
 
 		timr->it_active = 1;

Re: [GIT PULL] timer fix

[pr-tracker-bot]

The pull request you sent on Fri, 21 Dec 2018 13:34:53 +0100:

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/e572fa0e840154d33a69622af030dda551eee606

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

Re: [GIT PULL] timer fix

[Heiko Carstens]

On Fri, Dec 21, 2018 at 01:34:53PM +0100, Ingo Molnar wrote:
> Linus,
> 
> Please pull the latest timers-urgent-for-linus git tree from:
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus
> 
>    # HEAD: 0e334db6bb4b1fd1e2d72c1f3d8f004313cd9f94 posix-timers: Fix division by zero bug
> 
> Fix a division by zero crash in the posix-timers code.
> 
>  Thanks,
> 
> 	Ingo
> 
> ------------------>
> Thomas Gleixner (1):
>       posix-timers: Fix division by zero bug
> 
> 
>  kernel/time/posix-timers.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
> index bd62b5eeb5a0..31f49ae80f43 100644
> --- a/kernel/time/posix-timers.c
> +++ b/kernel/time/posix-timers.c
> @@ -289,9 +289,6 @@ static void common_hrtimer_rearm(struct k_itimer *timr)
>  {
>  	struct hrtimer *timer = &timr->it.real.timer;
>  
> -	if (!timr->it_interval)
> -		return;
> -
>  	timr->it_overrun += hrtimer_forward(timer, timer->base->get_time(),
>  					    timr->it_interval);
>  	hrtimer_restart(timer);
> @@ -317,7 +314,7 @@ void posixtimer_rearm(struct kernel_siginfo *info)
>  	if (!timr)
>  		return;
>  
> -	if (timr->it_requeue_pending == info->si_sys_private) {
> +	if (timr->it_interval && timr->it_requeue_pending == info->si_sys_private) {
>  		timr->kclock->timer_rearm(timr);

FWIW, with this patch the vanilla glibc 2.28 self tests
rt/tst-cputimer1, rt/tst-cputimer2, and rt/tst-cputimer3
start to fail on s390:

rt/tst-cputimer1.out:
clock_gettime returned timespec = { 0, 117181 }
clock_getres returned timespec = { 0, 1 }
Timed out: killed the child process
rt/tst-cputimer1.test-result:
FAIL: rt/tst-cputimer1
original exit status 1

rt/tst-cputimer2.out:
clock_gettime returned timespec = { 0, 9686 }
clock_getres returned timespec = { 0, 1 }
Timed out: killed the child process
rt/tst-cputimer2.test-result:
FAIL: rt/tst-cputimer2
original exit status 1

rt/tst-cputimer3.out:
clock_gettime returned timespec = { 0, 0 }
clock_getres returned timespec = { 0, 1 }
Timed out: killed the child process
rt/tst-cputimer3.test-result:
FAIL: rt/tst-cputimer3
original exit status 1

I haven't looked any further into this, just reporting.. otherwise the
test systems seem to be healthy.

Re: [GIT PULL] timer fix

[Ingo Molnar]

* Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> > -	if (timr->it_requeue_pending == info->si_sys_private) {
> > +	if (timr->it_interval && timr->it_requeue_pending == info->si_sys_private) {
> >  		timr->kclock->timer_rearm(timr);
> 
> FWIW, with this patch the vanilla glibc 2.28 self tests
> rt/tst-cputimer1, rt/tst-cputimer2, and rt/tst-cputimer3
> start to fail on s390:
> 
> rt/tst-cputimer1.out:
> clock_gettime returned timespec = { 0, 117181 }
> clock_getres returned timespec = { 0, 1 }
> Timed out: killed the child process
> rt/tst-cputimer1.test-result:
> FAIL: rt/tst-cputimer1
> original exit status 1
> 
> rt/tst-cputimer2.out:
> clock_gettime returned timespec = { 0, 9686 }
> clock_getres returned timespec = { 0, 1 }
> Timed out: killed the child process
> rt/tst-cputimer2.test-result:
> FAIL: rt/tst-cputimer2
> original exit status 1
> 
> rt/tst-cputimer3.out:
> clock_gettime returned timespec = { 0, 0 }
> clock_getres returned timespec = { 0, 1 }
> Timed out: killed the child process
> rt/tst-cputimer3.test-result:
> FAIL: rt/tst-cputimer3
> original exit status 1
> 
> I haven't looked any further into this, just reporting.. otherwise the
> test systems seem to be healthy.

Could you please check whether the top commit in tip:timers/urgent fixes 
it:

  93ad0fc088c5: posix-cpu-timers: Unbreak timer rearming

?

It's in tip:master as well and should show up in linux-next tomorrow.

Thanks,

	Ingo

Re: [GIT PULL] timer fix

[Heiko Carstens]

On Thu, Jan 17, 2019 at 10:51:02AM +0100, Ingo Molnar wrote:
> 
> * Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> 
> > > -	if (timr->it_requeue_pending == info->si_sys_private) {
> > > +	if (timr->it_interval && timr->it_requeue_pending == info->si_sys_private) {
> > >  		timr->kclock->timer_rearm(timr);
> > 
> > FWIW, with this patch the vanilla glibc 2.28 self tests
> > rt/tst-cputimer1, rt/tst-cputimer2, and rt/tst-cputimer3
> > start to fail on s390:
...
> > I haven't looked any further into this, just reporting.. otherwise the
> > test systems seem to be healthy.
> 
> Could you please check whether the top commit in tip:timers/urgent fixes 
> it:
>   93ad0fc088c5: posix-cpu-timers: Unbreak timer rearming

Yes, the test cases don't fail anymore. Thanks!

A general question: since I reported this already last year, was the
bug report not usable? I understand that x-mas holidays were in
between, just wondering if new "glibc test case" fails are worth to be
reported like I did.

Re: [GIT PULL] timer fix

[Thomas Gleixner]

On Thu, 17 Jan 2019, Heiko Carstens wrote:
> On Thu, Jan 17, 2019 at 10:51:02AM +0100, Ingo Molnar wrote:
> > 
> > * Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> > 
> > > > -	if (timr->it_requeue_pending == info->si_sys_private) {
> > > > +	if (timr->it_interval && timr->it_requeue_pending == info->si_sys_private) {
> > > >  		timr->kclock->timer_rearm(timr);
> > > 
> > > FWIW, with this patch the vanilla glibc 2.28 self tests
> > > rt/tst-cputimer1, rt/tst-cputimer2, and rt/tst-cputimer3
> > > start to fail on s390:
> ...
> > > I haven't looked any further into this, just reporting.. otherwise the
> > > test systems seem to be healthy.
> > 
> > Could you please check whether the top commit in tip:timers/urgent fixes 
> > it:
> >   93ad0fc088c5: posix-cpu-timers: Unbreak timer rearming
> 
> Yes, the test cases don't fail anymore. Thanks!
> 
> A general question: since I reported this already last year, was the
> bug report not usable? I understand that x-mas holidays were in
> between, just wondering if new "glibc test case" fails are worth to be
> reported like I did.

I was on a 3 weeks vacation and I tend to clean out my inbox when I return
as it turned out in the past that playing catch up is hopeless. The
important stuff comes back by itself :)

Thanks,

	tglx

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 07d7e12091f4ab869cc6a4bb276399057e73b0b3 alarmtimer: Return correct remaining time

Fix the alarm_timer_remaining() return value.

 Thanks,

	Ingo

------------------>
Andrei Vagin (1):
      alarmtimer: Return correct remaining time


 kernel/time/alarmtimer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index 2c97e8c2d29f..0519a8805aab 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -594,7 +594,7 @@ static ktime_t alarm_timer_remaining(struct k_itimer *timr, ktime_t now)
 {
 	struct alarm *alarm = &timr->it.alarm.alarmtimer;
 
-	return ktime_sub(now, alarm->node.expires);
+	return ktime_sub(alarm->node.expires, now);
 }
 
 /**

Re: [GIT PULL] timer fix

[pr-tracker-bot]

The pull request you sent on Fri, 12 Apr 2019 15:09:19 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/122c215bfae884f10a189e6754d9603a06b981c3

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: e430d802d6a3aaf61bd3ed03d9404888a29b9bf9 timer: Read jiffies once when forwarding base clk

Fixes a timer expiry bug that would cause spurious delay of timers.

 Thanks,

	Ingo

------------------>
Li RongQing (1):
      timer: Read jiffies once when forwarding base clk


 kernel/time/timer.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 0e315a2e77ae..4820823515e9 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1678,24 +1678,26 @@ void timer_clear_idle(void)
 static int collect_expired_timers(struct timer_base *base,
 				  struct hlist_head *heads)
 {
+	unsigned long now = READ_ONCE(jiffies);
+
 	/*
 	 * NOHZ optimization. After a long idle sleep we need to forward the
 	 * base to current jiffies. Avoid a loop by searching the bitfield for
 	 * the next expiring timer.
 	 */
-	if ((long)(jiffies - base->clk) > 2) {
+	if ((long)(now - base->clk) > 2) {
 		unsigned long next = __next_timer_interrupt(base);
 
 		/*
 		 * If the next timer is ahead of time forward to current
 		 * jiffies, otherwise forward to the next expiry time:
 		 */
-		if (time_after(next, jiffies)) {
+		if (time_after(next, now)) {
 			/*
 			 * The call site will increment base->clk and then
 			 * terminate the expiry loop immediately.
 			 */
-			base->clk = jiffies;
+			base->clk = now;
 			return 0;
 		}
 		base->clk = next;

Re: [GIT PULL] timer fix

[pr-tracker-bot]

The pull request you sent on Thu, 26 Sep 2019 22:18:25 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/da05b5ea12c1e50b2988a63470d6b69434796f8b

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: b9023b91dd020ad7e093baa5122b6968c48cc9e0 tick: broadcast-hrtimer: Fix a race in bc_set_next

Fix a broadcast-timer handling race that can result in spuriously and 
indefinitely delayed hrtimers and even RCU stalls if the system is 
otherwise quiet.

 Thanks,

	Ingo

------------------>
Balasubramani Vivekanandan (1):
      tick: broadcast-hrtimer: Fix a race in bc_set_next


 kernel/time/tick-broadcast-hrtimer.c | 62 +++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 33 deletions(-)

diff --git a/kernel/time/tick-broadcast-hrtimer.c b/kernel/time/tick-broadcast-hrtimer.c
index c1f5bb590b5e..b5a65e212df2 100644
--- a/kernel/time/tick-broadcast-hrtimer.c
+++ b/kernel/time/tick-broadcast-hrtimer.c
@@ -42,39 +42,39 @@ static int bc_shutdown(struct clock_event_device *evt)
  */
 static int bc_set_next(ktime_t expires, struct clock_event_device *bc)
 {
-	int bc_moved;
 	/*
-	 * We try to cancel the timer first. If the callback is on
-	 * flight on some other cpu then we let it handle it. If we
-	 * were able to cancel the timer nothing can rearm it as we
-	 * own broadcast_lock.
+	 * This is called either from enter/exit idle code or from the
+	 * broadcast handler. In all cases tick_broadcast_lock is held.
 	 *
-	 * However we can also be called from the event handler of
-	 * ce_broadcast_hrtimer itself when it expires. We cannot
-	 * restart the timer because we are in the callback, but we
-	 * can set the expiry time and let the callback return
-	 * HRTIMER_RESTART.
+	 * hrtimer_cancel() cannot be called here neither from the
+	 * broadcast handler nor from the enter/exit idle code. The idle
+	 * code can run into the problem described in bc_shutdown() and the
+	 * broadcast handler cannot wait for itself to complete for obvious
+	 * reasons.
 	 *
-	 * Since we are in the idle loop at this point and because
-	 * hrtimer_{start/cancel} functions call into tracing,
-	 * calls to these functions must be bound within RCU_NONIDLE.
+	 * Each caller tries to arm the hrtimer on its own CPU, but if the
+	 * hrtimer callbback function is currently running, then
+	 * hrtimer_start() cannot move it and the timer stays on the CPU on
+	 * which it is assigned at the moment.
+	 *
+	 * As this can be called from idle code, the hrtimer_start()
+	 * invocation has to be wrapped with RCU_NONIDLE() as
+	 * hrtimer_start() can call into tracing.
 	 */
-	RCU_NONIDLE(
-		{
-			bc_moved = hrtimer_try_to_cancel(&bctimer) >= 0;
-			if (bc_moved) {
-				hrtimer_start(&bctimer, expires,
-					      HRTIMER_MODE_ABS_PINNED_HARD);
-			}
-		}
-	);
-
-	if (bc_moved) {
-		/* Bind the "device" to the cpu */
-		bc->bound_on = smp_processor_id();
-	} else if (bc->bound_on == smp_processor_id()) {
-		hrtimer_set_expires(&bctimer, expires);
-	}
+	RCU_NONIDLE( {
+		hrtimer_start(&bctimer, expires, HRTIMER_MODE_ABS_PINNED_HARD);
+		/*
+		 * The core tick broadcast mode expects bc->bound_on to be set
+		 * correctly to prevent a CPU which has the broadcast hrtimer
+		 * armed from going deep idle.
+		 *
+		 * As tick_broadcast_lock is held, nothing can change the cpu
+		 * base which was just established in hrtimer_start() above. So
+		 * the below access is safe even without holding the hrtimer
+		 * base lock.
+		 */
+		bc->bound_on = bctimer.base->cpu_base->cpu;
+	} );
 	return 0;
 }
 
@@ -100,10 +100,6 @@ static enum hrtimer_restart bc_handler(struct hrtimer *t)
 {
 	ce_broadcast_hrtimer.event_handler(&ce_broadcast_hrtimer);
 
-	if (clockevent_state_oneshot(&ce_broadcast_hrtimer))
-		if (ce_broadcast_hrtimer.next_event != KTIME_MAX)
-			return HRTIMER_RESTART;
-
 	return HRTIMER_NORESTART;
 }
 

Re: [GIT PULL] timer fix

[pr-tracker-bot]

The pull request you sent on Thu, 3 Oct 2019 00:06:07 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/5021b9182ee805603e3b180220a929af7bd4b960

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers-urgent-for-linus git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

   # HEAD: 2f5841349df281ecf8f81cc82d869b8476f0db0b ntp/y2038: Remove incorrect time_t truncation

Fix integer truncation bug in __do_adjtimex().

 Thanks,

	Ingo

------------------>
Arnd Bergmann (1):
      ntp/y2038: Remove incorrect time_t truncation


 kernel/time/ntp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 65eb796610dc..069ca78fb0bf 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -771,7 +771,7 @@ int __do_adjtimex(struct __kernel_timex *txc, const struct timespec64 *ts,
 	/* fill PPS status fields */
 	pps_fill_timex(txc);
 
-	txc->time.tv_sec = (time_t)ts->tv_sec;
+	txc->time.tv_sec = ts->tv_sec;
 	txc->time.tv_usec = ts->tv_nsec;
 	if (!(time_status & STA_NANO))
 		txc->time.tv_usec = ts->tv_nsec / NSEC_PER_USEC;

Re: [GIT PULL] timer fix

[pr-tracker-bot]

The pull request you sent on Sat, 16 Nov 2019 22:38:54 +0100:

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-for-linus

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/3278b3b6782c562079a3e0af0979968fd94d141c

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers/urgent git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-2020-04-25

   # HEAD: ac84bac4062e7fc24f5e2c61c6a414b2a00a29ad vdso/datapage: Use correct clock mode name in comment

A single fix for a comment that may show up in DocBook output.

 Thanks,

	Ingo

------------------>
Christian Brauner (1):
      vdso/datapage: Use correct clock mode name in comment


 include/vdso/datapage.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
index 5cbc9fcbfd45..7955c56d6b3c 100644
--- a/include/vdso/datapage.h
+++ b/include/vdso/datapage.h
@@ -73,8 +73,8 @@ struct vdso_timestamp {
  *
  * @offset is used by the special time namespace VVAR pages which are
  * installed instead of the real VVAR page. These namespace pages must set
- * @seq to 1 and @clock_mode to VLOCK_TIMENS to force the code into the
- * time namespace slow path. The namespace aware functions retrieve the
+ * @seq to 1 and @clock_mode to VDSO_CLOCKMODE_TIMENS to force the code into
+ * the time namespace slow path. The namespace aware functions retrieve the
  * real system wide VVAR page, read host time and add the per clock offset.
  * For clocks which are not affected by time namespace adjustment the
  * offset must be zero.

Re: [GIT PULL] timer fix

[pr-tracker-bot]

The pull request you sent on Sat, 25 Apr 2020 12:16:11 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-2020-04-25

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/acd629446804617a8fe4700fc4ca16eb44aa4efd

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

[GIT PULL] timer fix

[Ingo Molnar]

Linus,

Please pull the latest timers/urgent git tree from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-2020-06-28

   # HEAD: f097eb38f71391ff2cf078788bad5a00eb3bd96a timekeeping: Fix kerneldoc system_device_crosststamp & al

A single DocBook fix.

 Thanks,

	Ingo

------------------>
Kurt Kanzenbach (1):
      timekeeping: Fix kerneldoc system_device_crosststamp & al


 include/linux/timekeeping.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index b27e2ffa96c1..d5471d6fa778 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -222,9 +222,9 @@ extern bool timekeeping_rtc_skipresume(void);
 
 extern void timekeeping_inject_sleeptime64(const struct timespec64 *delta);
 
-/*
+/**
  * struct system_time_snapshot - simultaneous raw/real time capture with
- *	counter value
+ *				 counter value
  * @cycles:	Clocksource counter value to produce the system times
  * @real:	Realtime system time
  * @raw:	Monotonic raw system time
@@ -239,9 +239,9 @@ struct system_time_snapshot {
 	u8		cs_was_changed_seq;
 };
 
-/*
+/**
  * struct system_device_crosststamp - system/device cross-timestamp
- *	(syncronized capture)
+ *				      (synchronized capture)
  * @device:		Device time
  * @sys_realtime:	Realtime simultaneous with device time
  * @sys_monoraw:	Monotonic raw simultaneous with device time
@@ -252,12 +252,12 @@ struct system_device_crosststamp {
 	ktime_t sys_monoraw;
 };
 
-/*
+/**
  * struct system_counterval_t - system counter value with the pointer to the
- *	corresponding clocksource
+ *				corresponding clocksource
  * @cycles:	System counter value
  * @cs:		Clocksource corresponding to system counter value. Used by
- *	timekeeping code to verify comparibility of two cycle values
+ *		timekeeping code to verify comparibility of two cycle values
  */
 struct system_counterval_t {
 	u64			cycles;

Re: [GIT PULL] timer fix

[pr-tracker-bot]

The pull request you sent on Sun, 28 Jun 2020 20:39:25 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers-urgent-2020-06-28

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/668f532da4808688f5162cec6a38875390e1a91d

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker