x86/apic: Implement set_state_oneshot_stopped() callback
PATCH 1/1 x86/apic: Implement set_state_oneshot_stopped() callback
When clock_event_device::set_state_oneshot_stopped() is not implemented, hrtimer_cancel() can't stop the clock when there is no more timer in the queue. So the ghost of the freshly cancelled hrtimer haunts us back later with an extra interrupt:
<idle>-0 [002] d..2 2248.557659: hrtimer_cancel: hrtimer=ffff88021fa92d80
<idle>-0 [002] d.h1 2249.303659: local_timer_entry: vector=239So let's implement this missing callback for the lapic clock. This consist in calling its set_state_shutdown() callback. There don't seem to be a lighter way to stop the clock. Simply writing 0 to APIC_TMICT won't be enough to stop the clock and avoid the extra interrupt, as opposed to what is specified in the specs. We must also mask the timer interrupt in the device.
--- arch/x86/kernel/apic/apic.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)
arch/x86/kernel/apic/apic.c +13 -12
@@ -529,18 +529,19 @@ static void lapic_timer_broadcast(const struct cpumask *mask)* The local apic timer can be used for any function which is CPU local. */ static struct clock_event_device lapic_clockevent = { - .name ="lapic" , - .features =CLOCK_EVT_FEAT_PERIODIC | -CLOCK_EVT_FEAT_ONESHOT |CLOCK_EVT_FEAT_C3STOP - |CLOCK_EVT_FEAT_DUMMY , - .shift =32 , - .set_state_shutdown = lapic_timer_shutdown, - .set_state_periodic = lapic_timer_set_periodic, - .set_state_oneshot = lapic_timer_set_oneshot, - .set_next_event = lapic_next_event, - .broadcast = lapic_timer_broadcast, - .rating =100 , - .irq =-1 , + .name ="lapic" , + .features =CLOCK_EVT_FEAT_PERIODIC | +CLOCK_EVT_FEAT_ONESHOT |CLOCK_EVT_FEAT_C3STOP + |CLOCK_EVT_FEAT_DUMMY , + .shift =32 , + .set_state_shutdown = lapic_timer_shutdown, + .set_state_periodic = lapic_timer_set_periodic, + .set_state_oneshot = lapic_timer_set_oneshot, + .set_state_oneshot_stopped = lapic_timer_shutdown, + .set_next_event = lapic_next_event, + .broadcast = lapic_timer_broadcast, + .rating =100 , + .irq =-1 , }; static DEFINE_PER_CPU(struct clock_event_device, lapic_events);
Signature · 1 line
2.7.4
2016-12-30 0:45 GMT+08:00 Frederic Weisbecker <[email protected]>:
Quoted 15 lines
When
clock_event_device::set_state_oneshot_stopped()is not implemented,hrtimer_cancel()can't stop the clock when there is no more timer in the queue. So the ghost of the freshly cancelled hrtimer haunts us back later with an extra interrupt:<idle>-0 [002] d..2 2248.557659: hrtimer_cancel: hrtimer=ffff88021fa92d80 <idle>-0 [002] d.h1 2249.303659: local_timer_entry: vector=239So let's implement this missing callback for the lapic clock. This consist in calling its
set_state_shutdown()callback. There don't seem to be a lighter way to stop the clock. Simply writing 0 toAPIC_TMICTwon't be enough to stop the clock and avoid the extra interrupt, as opposed to what is specified in the specs. We must also mask the timer interrupt in the device.
Quoted 7 lines
Cc Thomas Gleixner <[email protected]>Cc Borislav Petkov <[email protected]>Cc Ingo Molnar <[email protected]>Cc Wanpeng Li <[email protected]>Cc Viresh Kumar <[email protected]>Signed-off-by Frederic Weisbecker <[email protected]>---
Quoted 42 lines
arch/x86/kernel/apic/apic.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)arch/x86/kernel/apic/apic.c +13
-13@@ -529,18 +529,19 @@ static void lapic_timer_broadcast(const struct cpumask *mask)* The local apic timer can be used for any function which is CPU local. */ static struct clock_event_device lapic_clockevent = { - .name ="lapic" , - .features =CLOCK_EVT_FEAT_PERIODIC | -CLOCK_EVT_FEAT_ONESHOT |CLOCK_EVT_FEAT_C3STOP - |CLOCK_EVT_FEAT_DUMMY , - .shift =32 , - .set_state_shutdown = lapic_timer_shutdown, - .set_state_periodic = lapic_timer_set_periodic, - .set_state_oneshot = lapic_timer_set_oneshot, - .set_next_event = lapic_next_event, - .broadcast = lapic_timer_broadcast, - .rating =100 , - .irq =-1 , + .name ="lapic" , + .features =CLOCK_EVT_FEAT_PERIODIC | +CLOCK_EVT_FEAT_ONESHOT |CLOCK_EVT_FEAT_C3STOP + |CLOCK_EVT_FEAT_DUMMY , + .shift =32 , + .set_state_shutdown = lapic_timer_shutdown, + .set_state_periodic = lapic_timer_set_periodic, + .set_state_oneshot = lapic_timer_set_oneshot, + .set_state_oneshot_stopped = lapic_timer_shutdown, + .set_next_event = lapic_next_event, + .broadcast = lapic_timer_broadcast, + .rating =100 , + .irq =-1 , }; static DEFINE_PER_CPU(struct clock_event_device, lapic_events); --2.7.4
Quoted 21 lines · Frederic Weisbecker
When
clock_event_device::set_state_oneshot_stopped()is not implemented,hrtimer_cancel()can't stop the clock when there is no more timer in the queue. So the ghost of the freshly cancelled hrtimer haunts us back later with an extra interrupt:<idle>-0 [002] d..2 2248.557659: hrtimer_cancel: hrtimer=ffff88021fa92d80 <idle>-0 [002] d.h1 2249.303659: local_timer_entry: vector=239So let's implement this missing callback for the lapic clock. This consist in calling its
set_state_shutdown()callback. There don't seem to be a lighter way to stop the clock. Simply writing 0 toAPIC_TMICTwon't be enough to stop the clock and avoid the extra interrupt, as opposed to what is specified in the specs. We must also mask the timer interrupt in the device.Cc Thomas Gleixner <[email protected]>Cc Borislav Petkov <[email protected]>Cc Ingo Molnar <[email protected]>Cc Wanpeng Li <[email protected]>Cc Viresh Kumar <[email protected]>Signed-off-by Frederic Weisbecker <[email protected]>
Quoted 3 lines
--- arch/x86/kernel/apic/apic.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)
I didn't try updating anything other than the ARM subsystem as I was afraid of getting crushed by Thomas :)
Though, I do remember from one of the email (from Thomas), where he mentioned that disabling the timer on some platforms may have bigger penalties. Perhaps its fast enough for apic..
Signature · 1 line
viresh
tip-bot for Frederic Weisbecker
merge botx86/apic: Implement set_state_oneshot_stopped() callback
Commit-ID: 914122c389d091a02f7b5476209af715e77ccb73 Gitweb: http://git.kernel.org/tip/914122c389d091a02f7b5476209af715e77ccb73 Author: Frederic Weisbecker <[email protected]> AuthorDate: Thu, 29 Dec 2016 17:45:49 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Mon, 9 Jan 2017 11:48:42 +0100
x86/apic: Implement set_state_oneshot_stopped() callback
When clock_event_device::set_state_oneshot_stopped() is not implemented, hrtimer_cancel() can't stop the clock when there is no more timer in the queue. So the ghost of the freshly cancelled hrtimer haunts us back later with an extra interrupt:
<idle>-0 [002] d..2 2248.557659: hrtimer_cancel: hrtimer=ffff88021fa92d80
<idle>-0 [002] d.h1 2249.303659: local_timer_entry: vector=239So let's implement this missing callback for the lapic clock. This consist in calling its set_state_shutdown() callback. There don't seem to be a lighter way to stop the clock. Simply writing 0 to APIC_TMICT won't be enough to stop the clock and avoid the extra interrupt, as opposed to what is specified in the specs. We must also mask the timer interrupt in the device.
--- arch/x86/kernel/apic/apic.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)
arch/x86/kernel/apic/apic.c +13 -12
@@ -529,18 +529,19 @@ static void lapic_timer_broadcast(const struct cpumask *mask)* The local apic timer can be used for any function which is CPU local. */ static struct clock_event_device lapic_clockevent = { - .name ="lapic" , - .features =CLOCK_EVT_FEAT_PERIODIC | -CLOCK_EVT_FEAT_ONESHOT |CLOCK_EVT_FEAT_C3STOP - |CLOCK_EVT_FEAT_DUMMY , - .shift =32 , - .set_state_shutdown = lapic_timer_shutdown, - .set_state_periodic = lapic_timer_set_periodic, - .set_state_oneshot = lapic_timer_set_oneshot, - .set_next_event = lapic_next_event, - .broadcast = lapic_timer_broadcast, - .rating =100 , - .irq =-1 , + .name ="lapic" , + .features =CLOCK_EVT_FEAT_PERIODIC | +CLOCK_EVT_FEAT_ONESHOT |CLOCK_EVT_FEAT_C3STOP + |CLOCK_EVT_FEAT_DUMMY , + .shift =32 , + .set_state_shutdown = lapic_timer_shutdown, + .set_state_periodic = lapic_timer_set_periodic, + .set_state_oneshot = lapic_timer_set_oneshot, + .set_state_oneshot_stopped = lapic_timer_shutdown, + .set_next_event = lapic_next_event, + .broadcast = lapic_timer_broadcast, + .rating =100 , + .irq =-1 , }; static DEFINE_PER_CPU(struct clock_event_device, lapic_events);