* [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion @ 2014-02-07 17:29 Preeti U Murthy 2014-02-07 17:57 ` Thomas Gleixner 2014-02-09 7:31 ` [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion David Rientjes 0 siblings, 2 replies; 7+ messages in thread From: Preeti U Murthy @ 2014-02-07 17:29 UTC (permalink / raw) To: tglx, fengguang.wu; +Cc: mingo, linux-kernel The broadcast timer registration has to be done only when GENERIC_CLOCKEVENTS_BROADCAST and TICK_ONESHOT config options are enabled. Also fix max_delta_ticks value for the pseudo clock device. Reported-by: Fengguang Wu <fengguang.wu@intel.com> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@kernel.org> --- kernel/time/tick-broadcast-hrtimer.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kernel/time/tick-broadcast-hrtimer.c b/kernel/time/tick-broadcast-hrtimer.c index 5591aaa..bc383ac 100644 --- a/kernel/time/tick-broadcast-hrtimer.c +++ b/kernel/time/tick-broadcast-hrtimer.c @@ -81,7 +81,7 @@ static struct clock_event_device ce_broadcast_hrtimer = { .min_delta_ns = 1, .max_delta_ns = KTIME_MAX, .min_delta_ticks = 1, - .max_delta_ticks = KTIME_MAX, + .max_delta_ticks = ULONG_MAX, .mult = 1, .shift = 0, .cpumask = cpu_all_mask, @@ -102,9 +102,11 @@ static enum hrtimer_restart bc_handler(struct hrtimer *t) return HRTIMER_RESTART; } +#if defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) && defined(CONFIG_TICK_ONESHOT) void tick_setup_hrtimer_broadcast(void) { hrtimer_init(&bctimer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS); bctimer.function = bc_handler; clockevents_register_device(&ce_broadcast_hrtimer); } +#endif ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion 2014-02-07 17:29 [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion Preeti U Murthy @ 2014-02-07 17:57 ` Thomas Gleixner 2014-02-09 6:02 ` Preeti U Murthy 2014-02-09 7:31 ` [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion David Rientjes 1 sibling, 1 reply; 7+ messages in thread From: Thomas Gleixner @ 2014-02-07 17:57 UTC (permalink / raw) To: Preeti U Murthy; +Cc: fengguang.wu, mingo, linux-kernel On Fri, 7 Feb 2014, Preeti U Murthy wrote: > The broadcast timer registration has to be done only when > GENERIC_CLOCKEVENTS_BROADCAST and TICK_ONESHOT config options are enabled. Then we should compile that file only when those options are enabled. Where is the point to compile that code w/o the registration function? > Also fix max_delta_ticks value for the pseudo clock device. > > Reported-by: Fengguang Wu <fengguang.wu@intel.com> > Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@kernel.org> > --- > > kernel/time/tick-broadcast-hrtimer.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/kernel/time/tick-broadcast-hrtimer.c b/kernel/time/tick-broadcast-hrtimer.c > index 5591aaa..bc383ac 100644 > --- a/kernel/time/tick-broadcast-hrtimer.c > +++ b/kernel/time/tick-broadcast-hrtimer.c > @@ -81,7 +81,7 @@ static struct clock_event_device ce_broadcast_hrtimer = { > .min_delta_ns = 1, > .max_delta_ns = KTIME_MAX, > .min_delta_ticks = 1, > - .max_delta_ticks = KTIME_MAX, > + .max_delta_ticks = ULONG_MAX, > .mult = 1, > .shift = 0, > .cpumask = cpu_all_mask, > @@ -102,9 +102,11 @@ static enum hrtimer_restart bc_handler(struct hrtimer *t) > return HRTIMER_RESTART; > } > > +#if defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) && defined(CONFIG_TICK_ONESHOT) > void tick_setup_hrtimer_broadcast(void) > { > hrtimer_init(&bctimer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS); > bctimer.function = bc_handler; > clockevents_register_device(&ce_broadcast_hrtimer); > } > +#endif > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion 2014-02-07 17:57 ` Thomas Gleixner @ 2014-02-09 6:02 ` Preeti U Murthy 2014-02-09 14:16 ` [tip:timers/core] tick: Fixup more fallout from hrtimer broadcast mode tip-bot for Preeti U Murthy 0 siblings, 1 reply; 7+ messages in thread From: Preeti U Murthy @ 2014-02-09 6:02 UTC (permalink / raw) To: Thomas Gleixner; +Cc: fengguang.wu, mingo, linux-kernel Hi Thomas, On 02/07/2014 11:27 PM, Thomas Gleixner wrote: > On Fri, 7 Feb 2014, Preeti U Murthy wrote: > >> The broadcast timer registration has to be done only when >> GENERIC_CLOCKEVENTS_BROADCAST and TICK_ONESHOT config options are enabled. > > Then we should compile that file only when those options are > enabled. Where is the point to compile that code w/o the registration > function? Hmm of course. The delta patch is at the end. Another concern I have is with regard to the periodic mode of broadcast . We currently do not support the hrtimer mode of broadcast in periodic mode. The BROADCAST_ON/OFF calls which take effect in periodic mode has not yet been modified by this patchset to disable one CPU from going into deep idle, since we expect the deep idle states to never be chosen by the cpuidle governor in this mode. Do you think we should bother to modify this piece of code at all? On the same note, my understanding is that BROADCAST_ON/OFF takes effect only in periodic mode, in oneshot mode it is a nop. But why do we expect the CPUs to avail broadcast in periodic mode when they are not supposed to be entering deep idle states? Am I missing something here? IOW what is the point of periodic mode of broadcast? Is it for malfunctioning local clock devices? The delta patch below for fixing the compile time errors. This is based on tip/timers/core branch. time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion From: Preeti U Murthy <preeti@linux.vnet.ibm.com> The hrtimer mode of broadcast is supported only when GENERIC_CLOCKEVENTS_BROADCAST and TICK_ONESHOT config options are enabled. Hence compile in the functions for hrtimer mode of broadcast only when these options are selected. Also fix max_delta_ticks value for the pseudo clock device. Reported-by: Fengguang Wu <fengguang.wu@intel.com> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@kernel.org> --- include/linux/clockchips.h | 1 + kernel/time/Makefile | 5 ++++- kernel/time/tick-broadcast-hrtimer.c | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h index 20a7183..2e4cb67 100644 --- a/include/linux/clockchips.h +++ b/include/linux/clockchips.h @@ -207,6 +207,7 @@ static inline void clockevents_resume(void) {} static inline int clockevents_notify(unsigned long reason, void *arg) { return 0; } static inline int tick_check_broadcast_expired(void) { return 0; } +static inline void tick_setup_hrtimer_broadcast(void) {}; #endif diff --git a/kernel/time/Makefile b/kernel/time/Makefile index 06151ef..57a413f 100644 --- a/kernel/time/Makefile +++ b/kernel/time/Makefile @@ -3,7 +3,10 @@ obj-y += timeconv.o posix-clock.o alarmtimer.o obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD) += clockevents.o obj-$(CONFIG_GENERIC_CLOCKEVENTS) += tick-common.o -obj-$(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) += tick-broadcast.o tick-broadcast-hrtimer.o +ifeq ($(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST),y) + obj-y += tick-broadcast.o + obj-$(CONFIG_TICK_ONESHOT) += tick-broadcast-hrtimer.o +endif obj-$(CONFIG_GENERIC_SCHED_CLOCK) += sched_clock.o obj-$(CONFIG_TICK_ONESHOT) += tick-oneshot.o obj-$(CONFIG_TICK_ONESHOT) += tick-sched.o diff --git a/kernel/time/tick-broadcast-hrtimer.c b/kernel/time/tick-broadcast-hrtimer.c index 9242527..eb682d5 100644 --- a/kernel/time/tick-broadcast-hrtimer.c +++ b/kernel/time/tick-broadcast-hrtimer.c @@ -82,7 +82,7 @@ static struct clock_event_device ce_broadcast_hrtimer = { .min_delta_ns = 1, .max_delta_ns = KTIME_MAX, .min_delta_ticks = 1, - .max_delta_ticks = KTIME_MAX, + .max_delta_ticks = ULONG_MAX, .mult = 1, .shift = 0, .cpumask = cpu_all_mask, Thanks Regards Preeti U Murthy > >> Also fix max_delta_ticks value for the pseudo clock device. >> >> Reported-by: Fengguang Wu <fengguang.wu@intel.com> >> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Ingo Molnar <mingo@kernel.org> >> --- >> >> kernel/time/tick-broadcast-hrtimer.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/time/tick-broadcast-hrtimer.c b/kernel/time/tick-broadcast-hrtimer.c >> index 5591aaa..bc383ac 100644 >> --- a/kernel/time/tick-broadcast-hrtimer.c >> +++ b/kernel/time/tick-broadcast-hrtimer.c >> @@ -81,7 +81,7 @@ static struct clock_event_device ce_broadcast_hrtimer = { >> .min_delta_ns = 1, >> .max_delta_ns = KTIME_MAX, >> .min_delta_ticks = 1, >> - .max_delta_ticks = KTIME_MAX, >> + .max_delta_ticks = ULONG_MAX, >> .mult = 1, >> .shift = 0, >> .cpumask = cpu_all_mask, >> @@ -102,9 +102,11 @@ static enum hrtimer_restart bc_handler(struct hrtimer *t) >> return HRTIMER_RESTART; >> } >> >> +#if defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) && defined(CONFIG_TICK_ONESHOT) >> void tick_setup_hrtimer_broadcast(void) >> { >> hrtimer_init(&bctimer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS); >> bctimer.function = bc_handler; >> clockevents_register_device(&ce_broadcast_hrtimer); >> } >> +#endif >> >> > ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:timers/core] tick: Fixup more fallout from hrtimer broadcast mode 2014-02-09 6:02 ` Preeti U Murthy @ 2014-02-09 14:16 ` tip-bot for Preeti U Murthy 0 siblings, 0 replies; 7+ messages in thread From: tip-bot for Preeti U Murthy @ 2014-02-09 14:16 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, preeti, tglx, fengguang.wu Commit-ID: 849401b66d305f3feb75b6db7459b95ad190552a Gitweb: http://git.kernel.org/tip/849401b66d305f3feb75b6db7459b95ad190552a Author: Preeti U Murthy <preeti@linux.vnet.ibm.com> AuthorDate: Sun, 9 Feb 2014 11:32:22 +0530 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Sun, 9 Feb 2014 15:11:47 +0100 tick: Fixup more fallout from hrtimer broadcast mode The hrtimer mode of broadcast is supported only when GENERIC_CLOCKEVENTS_BROADCAST and TICK_ONESHOT config options are enabled. Hence compile in the functions for hrtimer mode of broadcast only when these options are selected. Also fix max_delta_ticks value for the pseudo clock device. Reported-by: Fengguang Wu <fengguang.wu@intel.com> Reported-by: Ingo Molnar <mingo@kernel.org> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com> Link: http://lkml.kernel.org/r/52F719EE.9010304@linux.vnet.ibm.com Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- include/linux/clockchips.h | 1 + kernel/time/Makefile | 5 ++++- kernel/time/tick-broadcast-hrtimer.c | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h index 20a7183..2e4cb67 100644 --- a/include/linux/clockchips.h +++ b/include/linux/clockchips.h @@ -207,6 +207,7 @@ static inline void clockevents_resume(void) {} static inline int clockevents_notify(unsigned long reason, void *arg) { return 0; } static inline int tick_check_broadcast_expired(void) { return 0; } +static inline void tick_setup_hrtimer_broadcast(void) {}; #endif diff --git a/kernel/time/Makefile b/kernel/time/Makefile index 06151ef..57a413f 100644 --- a/kernel/time/Makefile +++ b/kernel/time/Makefile @@ -3,7 +3,10 @@ obj-y += timeconv.o posix-clock.o alarmtimer.o obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD) += clockevents.o obj-$(CONFIG_GENERIC_CLOCKEVENTS) += tick-common.o -obj-$(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) += tick-broadcast.o tick-broadcast-hrtimer.o +ifeq ($(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST),y) + obj-y += tick-broadcast.o + obj-$(CONFIG_TICK_ONESHOT) += tick-broadcast-hrtimer.o +endif obj-$(CONFIG_GENERIC_SCHED_CLOCK) += sched_clock.o obj-$(CONFIG_TICK_ONESHOT) += tick-oneshot.o obj-$(CONFIG_TICK_ONESHOT) += tick-sched.o diff --git a/kernel/time/tick-broadcast-hrtimer.c b/kernel/time/tick-broadcast-hrtimer.c index 9242527..eb682d5 100644 --- a/kernel/time/tick-broadcast-hrtimer.c +++ b/kernel/time/tick-broadcast-hrtimer.c @@ -82,7 +82,7 @@ static struct clock_event_device ce_broadcast_hrtimer = { .min_delta_ns = 1, .max_delta_ns = KTIME_MAX, .min_delta_ticks = 1, - .max_delta_ticks = KTIME_MAX, + .max_delta_ticks = ULONG_MAX, .mult = 1, .shift = 0, .cpumask = cpu_all_mask, ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion 2014-02-07 17:29 [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion Preeti U Murthy 2014-02-07 17:57 ` Thomas Gleixner @ 2014-02-09 7:31 ` David Rientjes 2014-02-09 10:04 ` Preeti U Murthy 1 sibling, 1 reply; 7+ messages in thread From: David Rientjes @ 2014-02-09 7:31 UTC (permalink / raw) To: Preeti U Murthy; +Cc: tglx, fengguang.wu, mingo, linux-kernel [-- Attachment #1: Type: TEXT/PLAIN, Size: 1973 bytes --] On Fri, 7 Feb 2014, Preeti U Murthy wrote: > The broadcast timer registration has to be done only when > GENERIC_CLOCKEVENTS_BROADCAST and TICK_ONESHOT config options are enabled. > Also fix max_delta_ticks value for the pseudo clock device. > > Reported-by: Fengguang Wu <fengguang.wu@intel.com> > Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@kernel.org> > --- > > kernel/time/tick-broadcast-hrtimer.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/kernel/time/tick-broadcast-hrtimer.c b/kernel/time/tick-broadcast-hrtimer.c > index 5591aaa..bc383ac 100644 > --- a/kernel/time/tick-broadcast-hrtimer.c > +++ b/kernel/time/tick-broadcast-hrtimer.c > @@ -81,7 +81,7 @@ static struct clock_event_device ce_broadcast_hrtimer = { > .min_delta_ns = 1, > .max_delta_ns = KTIME_MAX, > .min_delta_ticks = 1, > - .max_delta_ticks = KTIME_MAX, > + .max_delta_ticks = ULONG_MAX, > .mult = 1, > .shift = 0, > .cpumask = cpu_all_mask, > @@ -102,9 +102,11 @@ static enum hrtimer_restart bc_handler(struct hrtimer *t) > return HRTIMER_RESTART; > } > > +#if defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) && defined(CONFIG_TICK_ONESHOT) > void tick_setup_hrtimer_broadcast(void) > { > hrtimer_init(&bctimer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS); > bctimer.function = bc_handler; > clockevents_register_device(&ce_broadcast_hrtimer); > } > +#endif I see a build error in timers/core today: kernel/time/tick-broadcast-hrtimer.c:101:6: error: redefinition of 'tick_setup_hrtimer_broadcast' include/linux/clockchips.h:194:20: note: previous definition of 'tick_setup_hrtimer_broadcast' was here and I assume this is the intended fix for that, although it isn't mentioned in the changelog. After it's applied, this is left over: kernel/time/tick-broadcast-hrtimer.c:91:29: warning: ‘bc_handler’ defined but not used [-Wunused-function] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion 2014-02-09 7:31 ` [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion David Rientjes @ 2014-02-09 10:04 ` Preeti U Murthy 2014-02-09 21:35 ` David Rientjes 0 siblings, 1 reply; 7+ messages in thread From: Preeti U Murthy @ 2014-02-09 10:04 UTC (permalink / raw) To: David Rientjes; +Cc: tglx, fengguang.wu, mingo, linux-kernel Hi David, I have sent out a revised patch on https://lkml.org/lkml/2014/2/9/2. Can you let me know if this works for you? Thanks Regards Preeti U Murthy On 02/09/2014 01:01 PM, David Rientjes wrote: > On Fri, 7 Feb 2014, Preeti U Murthy wrote: > >> The broadcast timer registration has to be done only when >> GENERIC_CLOCKEVENTS_BROADCAST and TICK_ONESHOT config options are enabled. >> Also fix max_delta_ticks value for the pseudo clock device. >> >> Reported-by: Fengguang Wu <fengguang.wu@intel.com> >> Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Ingo Molnar <mingo@kernel.org> >> --- >> >> kernel/time/tick-broadcast-hrtimer.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/time/tick-broadcast-hrtimer.c b/kernel/time/tick-broadcast-hrtimer.c >> index 5591aaa..bc383ac 100644 >> --- a/kernel/time/tick-broadcast-hrtimer.c >> +++ b/kernel/time/tick-broadcast-hrtimer.c >> @@ -81,7 +81,7 @@ static struct clock_event_device ce_broadcast_hrtimer = { >> .min_delta_ns = 1, >> .max_delta_ns = KTIME_MAX, >> .min_delta_ticks = 1, >> - .max_delta_ticks = KTIME_MAX, >> + .max_delta_ticks = ULONG_MAX, >> .mult = 1, >> .shift = 0, >> .cpumask = cpu_all_mask, >> @@ -102,9 +102,11 @@ static enum hrtimer_restart bc_handler(struct hrtimer *t) >> return HRTIMER_RESTART; >> } >> >> +#if defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST) && defined(CONFIG_TICK_ONESHOT) >> void tick_setup_hrtimer_broadcast(void) >> { >> hrtimer_init(&bctimer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS); >> bctimer.function = bc_handler; >> clockevents_register_device(&ce_broadcast_hrtimer); >> } >> +#endif > > I see a build error in timers/core today: > > kernel/time/tick-broadcast-hrtimer.c:101:6: error: redefinition of 'tick_setup_hrtimer_broadcast' > include/linux/clockchips.h:194:20: note: previous definition of 'tick_setup_hrtimer_broadcast' was here > > and I assume this is the intended fix for that, although it isn't > mentioned in the changelog. > > After it's applied, this is left over: > > kernel/time/tick-broadcast-hrtimer.c:91:29: warning: ‘bc_handler’ defined but not used [-Wunused-function] > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion 2014-02-09 10:04 ` Preeti U Murthy @ 2014-02-09 21:35 ` David Rientjes 0 siblings, 0 replies; 7+ messages in thread From: David Rientjes @ 2014-02-09 21:35 UTC (permalink / raw) To: Preeti U Murthy; +Cc: tglx, fengguang.wu, mingo, linux-kernel On Sun, 9 Feb 2014, Preeti U Murthy wrote: > Hi David, > > I have sent out a revised patch on > https://lkml.org/lkml/2014/2/9/2. Can you let me > know if this works for you? > This is commit 849401b66d30 ("tick: Fixup more fallout from hrtimer broadcast mode") in timers/core and it fixes it for my config. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-02-09 21:35 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-02-07 17:29 [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion Preeti U Murthy 2014-02-07 17:57 ` Thomas Gleixner 2014-02-09 6:02 ` Preeti U Murthy 2014-02-09 14:16 ` [tip:timers/core] tick: Fixup more fallout from hrtimer broadcast mode tip-bot for Preeti U Murthy 2014-02-09 7:31 ` [PATCH] time/cpuidle:Fixup fallout from hrtimer broadcast mode inclusion David Rientjes 2014-02-09 10:04 ` Preeti U Murthy 2014-02-09 21:35 ` David Rientjes
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox