adapt clockevents frequencies to mono clock
v4 cover adapt clockevents frequencies to mono clock
Previous v3 of this series can be found here:
http://lkml.kernel.org/r/[email protected]
First of all, apologies for sending such a huge series.
My intent is to give you a good idea of what would IMO be necessary to fix the "known issues" I listed in v3.
In case you think that this many changes are in no way justified by the final goal, namely to make the clockevent core NTP correction aware and thus avoiding the too short timer interrupts with NOHZ_FULL, please just drop me a note and I'll shut up.
The "known issues" mentioned in v3 were:
Quoted 3 lines · Nicolai Stange
- The patchset assumes that a clockevent device's ->mult is changed after registration only through calls to
clockevents_update_freq(). For a handful of non-x86 drivers this isn't the case.
Addressed by
[1/22] clocksource: sh_cmt: compute rate before registration again [2/22] clocksource: sh_tmu: compute rate before registration again [3/22] clocksource: em_sti: split clock prepare and enable steps [4/22] clocksource: em_sti: compute rate before registration [5/22] clocksource: h8300_timer8: don't reset rate in ->set_state_oneshot()
Quoted 15 lines
- ->min_delta_ns and ->max_delta_ns vs ->mult_mono:
Inclockevents_program_event(), we haddelta = min(delta, (int64_t) dev->max_delta_ns); delta = max(delta, (int64_t) dev->min_delta_ns); clc = ((unsigned long long) delta * dev->mult) >> dev->shift;The
dev->multis replaced with the dynamically adjusteddev->mult_monoby this series. That's problematic since as I understand it, especially ->max_delta_ns is a hard limit preventing the clockevent devices counter to be programmed with values larger than its width allows for. If ->mult_mono happens to be only slightly larger than ->mult, the comparison of delta against the ->mult based ->max_delta_ns can pass although the final clc might actually be larger than allowed. I think what we really want to have at this place is a check of clc against the already present ->min_delta_ticks and ->max_delta_ticks.
Quoted 4 lines
The problem with this approach is that many drivers (~40) initialize ->min_delta_ns and ->max_delta_ns (typically with
clockevent_delta2ns()) but not the ->*_delta_ticks members. My suggestion at this point would be to convert them.
This subseries
[7/22] many clockevent drivers: set ->min_delta_ticks and ->max_delta_ticks
[8/22] arch/s390/kernel/time: set ->min_delta_ticks and ->max_delta_ticks
[9/22] arch/x86/platform/uv/uv_time: set ->min_delta_ticks and
->max_delta_ticks
[10/22] arch/tile/kernel/time: set ->min_delta_ticks and ->max_delta_ticks
[11/22] clockevents: always initialize ->min_delta_ns and ->max_delta_ns
[12/22] many clockevent drivers: don't set ->min_delta_ns and ->max_delta_nsconverts all drivers to set ->*_delta_ticks rather than ->*_delta_ns.
Quoted 5 lines
This makes ->max_delta_ns obsolete right away. ->min_delta_ns is still needed in order to set the ->next_event in
clockevents_program_min_delta()though. My claim is that ->min_delta_ns can be safely replaced with 0 inclockevents_program_min_delta()
This subseries
[13/22] clockevents: check a programmed delta's bounds in terms of cycles [14/22] clockevents: clockevents_program_event(): turn clc into unsigned long [15/22] clockevents: clockevents_program_min_delta(): don't set ->next_event [16/22] clockevents: use ->min_delta_ticks_adjusted to program minimum delta [17/22] clockevents: min delta increment: calculate min_delta_ns from ticks [18/22] timer_list: print_tickdevice(): calculate ->*_delta_ns dynamically [19/22] clockevents: purge ->min_delta_ns and ->max_delta_ns
gets rid of the ->*_delta_ns alltogether.
The remaining patches, i.e. [20-22/22], correspond to the former [1-3/3] from v3.
Tested on next-20160816. Applies to next-20160822.
In case that you want me to proceed with this, I'd really appreciate some hints on how to send this in non-RFC mode, i.e. with everybody CC'd. Split this into smaller series and send one after another? Possibly spread out over several releases?
Changes to v3:
[20/22] ("clockevents: initial support for mono to raw time conversion")
- Following J. Stultz' suggestion, I renamed ->mult_mono to
->mult_adjusted
- J. Stultz pointed out that locking is necessary in the timekeeping_get_mono_mult() helper. Added.
[21/22] ("clockevents: make setting of ->mult and ->mult_adjusted atomic")
- I got the locking wrong here: updates to the bc device should be serialized as well. Extend the lock to the bc case.
- Adapt to the ->mult_mono => ->mult_adjusted renaming.
[22/22] ("timekeeping: inform clockevents about freq adjustments")
- Adapt to the ->mult_mono => ->mult_adjusted renaming.
Nicolai Stange (22):
clocksource: sh_cmt: compute rate before registration again
clocksource: sh_tmu: compute rate before registration again
clocksource: em_sti: split clock prepare and enable steps
clocksource: em_sti: compute rate before registration
clocksource: h8300_timer8: don't reset rate in ->set_state_oneshot()
clockevents: make clockevents_config() static
many clockevent drivers: set ->min_delta_ticks and ->max_delta_ticks arch/s390/kernel/time: set ->min_delta_ticks and ->max_delta_ticks arch/x86/platform/uv/uv_time: set ->min_delta_ticks and
->max_delta_ticks
arch/tile/kernel/time: set ->min_delta_ticks and ->max_delta_ticks
clockevents: always initialize ->min_delta_ns and ->max_delta_ns many clockevent drivers: don't set ->min_delta_ns and ->max_delta_ns
clockevents: check a programmed delta's bounds in terms of cycles
clockevents: clockevents_program_event(): turn clc into unsigned long
clockevents: clockevents_program_min_delta(): don't set ->next_event
clockevents: use ->min_delta_ticks_adjusted to program minimum delta
clockevents: min delta increment: calculate min_delta_ns from ticks
timer_list: print_tickdevice(): calculate ->*_delta_ns dynamically
clockevents: purge ->min_delta_ns and ->max_delta_ns
clockevents: initial support for mono to raw time conversion
clockevents: make setting of ->mult and ->mult_adjusted atomic
timekeeping: inform clockevents about freq adjustments
arch/avr32/kernel/time.c | 4 +- arch/blackfin/kernel/time-ts.c | 8 +- arch/c6x/platforms/timer64.c | 4 +- arch/hexagon/kernel/time.c | 4 +- arch/m68k/coldfire/pit.c | 6 +- arch/microblaze/kernel/timer.c | 6 +- arch/mips/alchemy/common/time.c | 4 +- arch/mips/jz4740/time.c | 4 +- arch/mips/kernel/cevt-bcm1480.c | 4 +- arch/mips/kernel/cevt-ds1287.c | 4 +- arch/mips/kernel/cevt-gt641xx.c | 4 +- arch/mips/kernel/cevt-sb1250.c | 4 +- arch/mips/kernel/cevt-txx9.c | 5 +- arch/mips/loongson32/common/time.c | 4 +- arch/mips/loongson64/common/cs5536/cs5536_mfgpt.c | 4 +- arch/mips/loongson64/loongson-3/hpet.c | 4 +- arch/mips/ralink/cevt-rt3352.c | 4 +- arch/mips/sgi-ip27/ip27-timer.c | 4 +- arch/mn10300/kernel/cevt-mn10300.c | 4 +- arch/powerpc/kernel/time.c | 6 +- arch/s390/kernel/time.c | 4 +- arch/score/kernel/time.c | 6 +- arch/sparc/kernel/time_32.c | 4 +- arch/sparc/kernel/time_64.c | 6 +- arch/tile/kernel/time.c | 4 +- arch/um/kernel/time.c | 4 +- arch/unicore32/kernel/time.c | 6 +- arch/x86/kernel/apic/apic.c | 12 +- arch/x86/lguest/boot.c | 4 +- arch/x86/platform/uv/uv_time.c | 6 +- arch/x86/xen/time.c | 8 +- drivers/clocksource/dw_apb_timer.c | 5 +- drivers/clocksource/em_sti.c | 49 ++++--- drivers/clocksource/h8300_timer8.c | 8 -- drivers/clocksource/metag_generic.c | 4 +- drivers/clocksource/numachip.c | 4 +- drivers/clocksource/sh_cmt.c | 50 +++---- drivers/clocksource/sh_tmu.c | 26 ++-- drivers/clocksource/timer-atlas7.c | 4 +- include/linux/clockchips.h | 17 ++- kernel/time/clockevents.c | 153 ++++++++++++++++------ kernel/time/tick-broadcast-hrtimer.c | 2 - kernel/time/tick-internal.h | 2 + kernel/time/timekeeping.c | 17 +++ kernel/time/timer_list.c | 10 +- 45 files changed, 295 insertions(+), 211 deletions(-)
Signature · 1 line
2.9.2
RFC v4 1/22 clocksource: sh_cmt: compute rate before registration again
With the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration.
Currently, sh_cmt violates this requirement in that it registers its clockevent device with a dummy rate and sets its final ->mult and ->shift values from its ->set_state_oneshot() and ->set_state_periodic() functions respectively.
This patch moves the setting of the clockevent device's ->mult and ->shift values to before its registration.
Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver:
commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before
registration")
moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later
commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz()
update")
basically reverts this, saying
"Without this patch the old code uses clocksource_register() together
with a hack that assumes a never changing clock rate."
However, I checked all current sh_cmt users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_cmt after their respective time_init(). Since all sh_cmt instances are created after time_init(), none of them should ever observe any clock rate changes.
What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here.
- Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device:
it's a property of the underlying clock which is in turn specific to the sh_cmt_device.
- Determine the ->rate value in sh_cmt_setup() at device probing rather than at first usage.
- Set the clockevent device's ->mult and ->shift values right before its registration.
- Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency.
---
Notes:
For a detailed analysis of the current sh_cmt users, please see
https://nicst.de/ced-clk-rate-change-analysis/sh_cmt-cgitted.html
Compile-only tested on ARCH=sh and ARCH=arm.
drivers/clocksource/sh_cmt.c | 50 ++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 23 deletions(-)
drivers/clocksource/sh_cmt.c +27 -23
@@ -103,7 +103,6 @@ struct sh_cmt_channel {unsigned long match_value;unsigned long next_match_value;unsigned long max_match_value; -unsigned long rate;raw_spinlock_t lock;struct clock_event_device ced;struct clocksource cs;
@@ -118,6 +117,7 @@ struct sh_cmt_device {void __iomem *mapbase;struct clk *clk; +unsigned long rate;raw_spinlock_t lock;/* Protect the shared start/stop register */
@@ -320,7 +320,7 @@ static void sh_cmt_start_stop_ch(struct sh_cmt_channel *ch, int start)raw_spin_unlock_irqrestore (&ch->cmt->lock, flags); } -static int sh_cmt_enable (struct sh_cmt_channel *ch,unsigned long *rate) +static int sh_cmt_enable (struct sh_cmt_channel *ch) {int k, ret;
@@ -339,17 +339,14 @@ static int sh_cmt_enable(struct sh_cmt_channel *ch, unsigned long *rate)sh_cmt_start_stop_ch (ch,0 );/* configure channel, periodic mode and maximum timeout */ -if (ch->cmt->info->width ==16 ) { - *rate =clk_get_rate (ch->cmt->clk) /512 ; +if (ch->cmt->info->width ==16 )sh_cmt_write_cmcsr (ch,SH_CMT16_CMCSR_CMIE |SH_CMT16_CMCSR_CKS512 ); - }else { - *rate =clk_get_rate (ch->cmt->clk) /8 ; +else sh_cmt_write_cmcsr (ch,SH_CMT32_CMCSR_CMM |SH_CMT32_CMCSR_CMTOUT_IE |SH_CMT32_CMCSR_CMR_IRQ |SH_CMT32_CMCSR_CKS_RCLK8 ); - }sh_cmt_write_cmcor (ch,0xffffffff );sh_cmt_write_cmcnt (ch,0 );
@@ -572,7 +569,7 @@ static int sh_cmt_start(struct sh_cmt_channel *ch, unsigned long flag)raw_spin_lock_irqsave (&ch->lock, flags);if (!(ch->flags & (FLAG_CLOCKEVENT |FLAG_CLOCKSOURCE ))) - ret =sh_cmt_enable (ch, &ch->rate); + ret =sh_cmt_enable (ch);if (ret) goto out;
@@ -640,10 +637,9 @@ static int sh_cmt_clocksource_enable(struct clocksource *cs) ch->total_cycles =0 ; ret =sh_cmt_start (ch,FLAG_CLOCKSOURCE ); -if (!ret) { -__clocksource_update_freq_hz (cs, ch->rate); +if (!ret) ch->cs_enabled = true; - } +return ret; }
@@ -697,8 +693,7 @@ static int sh_cmt_register_clocksource(struct sh_cmt_channel *ch,dev_info (&ch->cmt->pdev->dev,"ch%u: used as clock source\n" , ch->index); -/* Register with dummy 1 Hz value, gets updated in ->enable() */ -clocksource_register_hz (cs,1 ); +clocksource_register_hz (cs, ch->cmt->rate);return 0 ; }
@@ -709,19 +704,10 @@ static struct sh_cmt_channel *ced_to_sh_cmt(struct clock_event_device *ced)static void sh_cmt_clock_event_start (struct sh_cmt_channel *ch,int periodic) { -struct clock_event_device *ced = &ch->ced; -sh_cmt_start (ch,FLAG_CLOCKEVENT ); -/* TODO: calculate good shift from rate and counter bit width */ - - ced->shift =32 ; - ced->mult =div_sc (ch->rate,NSEC_PER_SEC , ced->shift); - ced->max_delta_ns =clockevent_delta2ns (ch->max_match_value, ced); - ced->min_delta_ns =clockevent_delta2ns (0x1f , ced); -if (periodic) -sh_cmt_set_next (ch, ((ch->rate +HZ /2 ) /HZ ) -1 ); +sh_cmt_set_next (ch, ((ch->cmt->rate +HZ /2 ) /HZ ) -1 );else sh_cmt_set_next (ch, ch->max_match_value); }
@@ -824,6 +810,12 @@ static int sh_cmt_register_clockevent(struct sh_cmt_channel *ch, ced->suspend = sh_cmt_clock_event_suspend; ced->resume = sh_cmt_clock_event_resume; +/* TODO: calculate good shift from rate and counter bit width */ + ced->shift =32 ; + ced->mult =div_sc (ch->cmt->rate,NSEC_PER_SEC , ced->shift); + ced->max_delta_ns =clockevent_delta2ns (ch->max_match_value, ced); + ced->min_delta_ns =clockevent_delta2ns (0x1f , ced); +dev_info (&ch->cmt->pdev->dev,"ch%u: used for clock events\n" , ch->index);clockevents_register_device (ced);
@@ -996,6 +988,18 @@ static int sh_cmt_setup(struct sh_cmt_device *cmt, struct platform_device *pdev)if (ret <0 ) goto err_clk_put; +/* Determine clock rate. */ + ret =clk_enable (cmt->clk); +if (ret <0 ) + goto err_clk_unprepare; + +if (cmt->info->width ==16 ) + cmt->rate =clk_get_rate (cmt->clk) /512 ; +else + cmt->rate =clk_get_rate (cmt->clk) /8 ; + +clk_disable (cmt->clk); +/* Map the memory resource(s). */ ret =sh_cmt_map_memory (cmt);if (ret <0 )
Signature · 1 line
2.9.2
RFC v4 2/22 clocksource: sh_tmu: compute rate before registration again
In reply to Nicolai StangeWith the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration.
Currently, sh_tmu violates this requirement in that it registers its clockevent device with a dummy rate and sets its final rate through clockevents_config() called from its ->set_state_oneshot() and ->set_state_periodic() functions respectively.
This patch moves the setting of the clockevent device's rate to its registration.
Note that there has been some back and forth regarding this question with respect to the clocksource also provided by this driver:
commit 66f49121ffa4 ("clocksource: sh_tmu: compute mult and shift before
registration")
moves the rate determination from the clocksource's ->enable() function to before its registration. OTOH, the later
commit 0aeac458d9eb ("clocksource: sh_tmu: __clocksource_updatefreq_hz()
update")
basically reverts this, saying
"Without this patch the old code uses clocksource_register() together
with a hack that assumes a never changing clock rate."
However, I checked all current sh_tmu users in arch/sh as well as in arch/arm/mach-shmobile carefully and right now, none of them changes any rate in any clock tree relevant to sh_tmu after their respective time_init(). Since all sh_tmu instances are created after time_init(), none of them should ever observe any clock rate changes.
What's more, both, a clocksource as well as a clockevent device, can immediately get selected for use at their registration and thus, enabled at this point already. So it's probably safer to assume a "never changing clock rate" here.
- Move the struct sh_tmu_channel's ->rate member to struct sh_tmu_device:
it's a property of the underlying clock which is in turn specific to the sh_tmu_device.
- Determine the ->rate value in sh_tmu_setup() at device probing rather than at first usage.
- Set the clockevent device's rate at its registration.
- Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency.
---
Notes:
For a detailed analysis of the current sh_tmu users, please see
https://nicst.de/ced-clk-rate-change-analysis/sh_tmu-cgitted.html
Compile-only tested on ARCH=sh and ARCH=arm.
drivers/clocksource/sh_tmu.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
drivers/clocksource/sh_tmu.c +13 -13
@@ -46,7 +46,6 @@ struct sh_tmu_channel {void __iomem *base;int irq; -unsigned long rate;unsigned long periodic;struct clock_event_device ced;struct clocksource cs;
@@ -59,6 +58,7 @@ struct sh_tmu_device {void __iomem *mapbase;struct clk *clk; +unsigned long rate;enum sh_tmu_model model;
@@ -165,7 +165,6 @@ static int __sh_tmu_enable(struct sh_tmu_channel *ch)sh_tmu_write (ch,TCNT ,0xffffffff );/* configure channel to parent clock / 4, irq off */ - ch->rate =clk_get_rate (ch->tmu->clk) /4 ;sh_tmu_write (ch,TCR ,TCR_TPSC_CLK4 );/* enable channel */
@@ -271,10 +270,8 @@ static int sh_tmu_clocksource_enable(struct clocksource *cs)return 0 ; ret =sh_tmu_enable (ch); -if (!ret) { -__clocksource_update_freq_hz (cs, ch->rate); +if (!ret) ch->cs_enabled = true; - }return ret; }
@@ -334,8 +331,7 @@ static int sh_tmu_register_clocksource(struct sh_tmu_channel *ch,dev_info (&ch->tmu->pdev->dev,"ch%u: used as clock source\n" , ch->index); -/* Register with dummy 1 Hz value, gets updated in ->enable() */ -clocksource_register_hz (cs,1 ); +clocksource_register_hz (cs, ch->tmu->rate);return 0 ; }
@@ -346,14 +342,10 @@ static struct sh_tmu_channel *ced_to_sh_tmu(struct clock_event_device *ced)static void sh_tmu_clock_event_start (struct sh_tmu_channel *ch,int periodic) { -struct clock_event_device *ced = &ch->ced; -sh_tmu_enable (ch); -clockevents_config (ced, ch->rate); -if (periodic) { - ch->periodic = (ch->rate +HZ /2 ) /HZ ; + ch->periodic = (ch->tmu->rate +HZ /2 ) /HZ ;sh_tmu_set_next (ch, ch->periodic,1 ); } }
@@ -435,7 +427,7 @@ static void sh_tmu_register_clockevent(struct sh_tmu_channel *ch,dev_info (&ch->tmu->pdev->dev,"ch%u: used for clock events\n" , ch->index); -clockevents_config_and_register (ced,1 ,0x300 ,0xffffffff ); +clockevents_config_and_register (ced, ch->tmu->rate,0x300 ,0xffffffff ); ret = request_irq(ch->irq, sh_tmu_interrupt, IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
@@ -561,6 +553,14 @@ static int sh_tmu_setup(struct sh_tmu_device *tmu, struct platform_device *pdev)if (ret <0 ) goto err_clk_put; +/* Determine clock rate. */ + ret =clk_enable (tmu->clk); +if (ret <0 ) + goto err_clk_unprepare; + + tmu->rate =clk_get_rate (tmu->clk) /4 ; +clk_disable (tmu->clk); +/* Map the memory resource. */ ret =sh_tmu_map_memory (tmu);if (ret <0 ) {
Signature · 1 line
2.9.2
RFC v4 3/22 clocksource: em_sti: split clock prepare and enable steps
In reply to Nicolai StangeCurrently, the em_sti driver prepares and enables the needed clock in em_sti_enable(), potentially called through its clockevent device's ->set_state_oneshot().
However, the clk_prepare() step may sleep whereas tick_program_event() and thus, ->set_state_oneshot(), can be called in atomic context.
Split the clk_prepare_enable() in em_sti_enable() into two steps:
- prepare the clock at device probing via clk_prepare()
- and enable it in em_sti_enable() via clk_enable(). Slightly reorder resource initialization in em_sti_probe() in order to facilitate error handling.
---
Notes:
Compile-only tested on ARCH=arm.
drivers/clocksource/em_sti.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)
drivers/clocksource/em_sti.c +18 -7
@@ -78,7 +78,7 @@ static int em_sti_enable(struct em_sti_priv *p)int ret;/* enable clock */ - ret =clk_prepare_enable (p->clk); + ret =clk_enable (p->clk);if (ret) {dev_err (&p->pdev->dev,"cannot enable clock\n" );return ret;
@@ -107,7 +107,7 @@ static void em_sti_disable(struct em_sti_priv *p)em_sti_write (p,STI_INTENCLR ,3 );/* stop clock */ -clk_disable_unprepare (p->clk); +clk_disable (p->clk); }static cycle_t em_sti_count (struct em_sti_priv *p)
@@ -303,6 +303,7 @@ static int em_sti_probe(struct platform_device *pdev)struct em_sti_priv *p;struct resource *res;int irq; +int ret; p =devm_kzalloc (&pdev->dev,sizeof (*p),GFP_KERNEL );if (p ==NULL )
@@ -323,6 +324,13 @@ static int em_sti_probe(struct platform_device *pdev)if (IS_ERR (p->base))return PTR_ERR (p->base); +if (devm_request_irq (&pdev->dev, irq, em_sti_interrupt, +IRQF_TIMER |IRQF_IRQPOLL |IRQF_NOBALANCING , +dev_name (&pdev->dev), p)) { +dev_err (&pdev->dev,"failed to request low IRQ\n" ); +return -ENOENT ; + } +/* get hold of clock */ p->clk =devm_clk_get (&pdev->dev,"sclk" );if (IS_ERR (p->clk)) {
@@ -330,17 +338,20 @@ static int em_sti_probe(struct platform_device *pdev)return PTR_ERR (p->clk); } -if (devm_request_irq (&pdev->dev, irq, em_sti_interrupt, -IRQF_TIMER |IRQF_IRQPOLL |IRQF_NOBALANCING , -dev_name (&pdev->dev), p)) { -dev_err (&pdev->dev,"failed to request low IRQ\n" ); -return -ENOENT ; + ret =clk_prepare (p->clk); +if (ret <0 ) { +dev_err (&pdev->dev,"cannot prepare clock\n" ); + goto err_clk_put; }raw_spin_lock_init (&p->lock);em_sti_register_clockevent (p);em_sti_register_clocksource (p);return 0 ; + +err_clk_put: +clk_put (p->clk); +return ret; }static int em_sti_remove (struct platform_device *pdev)
Signature · 1 line
2.9.2
RFC v4 4/22 clocksource: em_sti: compute rate before registration
In reply to Nicolai StangeWith the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration.
Currently, em_sti violates this requirement in that it registers its clockevent device with a dummy rate and sets its final rate through clockevents_config() called from its ->set_state_oneshot().
This patch moves the setting of the clockevent device's rate to its registration.
I checked all current em_sti users in arch/arm/mach-shmobile and right now, none of them changes any rate in any clock tree relevant to em_sti after their respective time_init(). Since all em_sti instances are created after time_init(), none of them should ever observe any clock rate changes.
- Determine the ->rate value in em_sti_probe() at device probing rather than at first usage.
- Set the clockevent device's rate at its registration.
- Although not strictly necessary for the upcoming clockevent core changes, set the clocksource's rate at its registration for consistency.
---
Notes:
For a detailed analysis of the current em_sti users, please see
https://nicst.de/ced-clk-rate-change-analysis/em_sti-cgitted.html
Compile-only tested on ARCH=arm.
drivers/clocksource/em_sti.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)
drivers/clocksource/em_sti.c +14 -10
@@ -205,13 +205,9 @@ static cycle_t em_sti_clocksource_read(struct clocksource *cs)static int em_sti_clocksource_enable (struct clocksource *cs) { -int ret;struct em_sti_priv *p =cs_to_em_sti (cs); - ret =em_sti_start (p,USER_CLOCKSOURCE ); -if (!ret) -__clocksource_update_freq_hz (cs, p->rate); -return ret; +return em_sti_start (p,USER_CLOCKSOURCE ); }static void em_sti_clocksource_disable (struct clocksource *cs)
@@ -240,8 +236,7 @@ static int em_sti_register_clocksource(struct em_sti_priv *p)dev_info (&p->pdev->dev,"used as clock source\n" ); -/* Register with dummy 1 Hz value, gets updated in ->enable() */ -clocksource_register_hz (cs,1 ); +clocksource_register_hz (cs, p->rate);return 0 ; }
@@ -263,7 +258,6 @@ static int em_sti_clock_event_set_oneshot(struct clock_event_device *ced)dev_info (&p->pdev->dev,"used for oneshot clock events\n" );em_sti_start (p,USER_CLOCKEVENT ); -clockevents_config (&p->ced, p->rate);return 0 ; }
@@ -294,8 +288,7 @@ static void em_sti_register_clockevent(struct em_sti_priv *p)dev_info (&p->pdev->dev,"used for clock events\n" ); -/* Register with dummy 1 Hz value, gets updated in ->set_state_oneshot() */ -clockevents_config_and_register (ced,1 ,2 ,0xffffffff ); +clockevents_config_and_register (ced, p->rate,2 ,0xffffffff ); }static int em_sti_probe (struct platform_device *pdev)
@@ -344,11 +337,22 @@ static int em_sti_probe(struct platform_device *pdev) goto err_clk_put; } + ret =clk_enable (p->clk); +if (ret <0 ) { +dev_err (&p->pdev->dev,"cannot enable clock\n" ); + goto err_clk_unprepare; + } + p->rate =clk_get_rate (p->clk); +clk_disable (p->clk); +raw_spin_lock_init (&p->lock);em_sti_register_clockevent (p);em_sti_register_clocksource (p);return 0 ; +err_clk_unprepare: +clk_unprepare (p->clk); + err_clk_put:clk_put (p->clk);return ret;
Signature · 1 line
2.9.2
RFC v4 5/22 clocksource: h8300_timer8: don't reset rate in ->set_state_oneshot()
In reply to Nicolai StangeWith the upcoming NTP correction related rate adjustments to be implemented in the clockevents core, the latter needs to get informed about every rate change of a clockevent device made after its registration.
Currently, h8300_timer8 violates this requirement in that it registers its clockevent device with the correct rate, but resets its ->mult and ->rate values in timer8_clock_event_start(), called from its ->set_state_oneshot() function.
It seems like
commit 4633f4cac85a ("clocksource/drivers/h8300: Cleanup startup and
remove module code."),
which introduced the rate initialization at registration, missed to remove the manual setting of ->mult and ->shift from timer8_clock_event_start().
Purge the setting of ->mult, ->shift, ->min_delta_ns and ->max_delta_ns from timer8_clock_event_start().
---
Notes:
Compile-only tested on ARCH=h8300.
drivers/clocksource/h8300_timer8.c | 8 -------- 1 file changed, 8 deletions(-)
drivers/clocksource/h8300_timer8.c +0 -8
@@ -101,15 +101,7 @@ static inline struct timer8_priv *ced_to_priv(struct clock_event_device *ced)static void timer8_clock_event_start (struct timer8_priv *p,unsigned long delta) { -struct clock_event_device *ced = &p->ced; -timer8_start (p); - - ced->shift =32 ; - ced->mult =div_sc (p->rate,NSEC_PER_SEC , ced->shift); - ced->max_delta_ns =clockevent_delta2ns (0xffff , ced); - ced->min_delta_ns =clockevent_delta2ns (0x0001 , ced); -timer8_set_next (p, delta); }
Signature · 1 line
2.9.2
RFC v4 6/22 clockevents: make clockevents_config() static
In reply to Nicolai StangeA clockevent device's rate should be configured before or at registration and changed afterwards through clockevents_update_freq() only.
For the configuration at registration, we already have clockevents_config_and_register().
Right now, there are no clockevents_config() users outside of the clockevents core.
To mitigiate the risk of drivers errorneously reconfiguring their rates through clockevents_config() *after* device registration, make clockevents_config() static.
--- include/linux/clockchips.h | 1 - kernel/time/clockevents.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-)
include/linux/clockchips.h +0 -1
@@ -182,7 +182,6 @@ extern u64 clockevent_delta2ns(unsigned long latch, struct clock_event_device *eextern void clockevents_register_device (struct clock_event_device *dev);extern int clockevents_unbind_device (struct clock_event_device *ced,int cpu); -extern void clockevents_config (struct clock_event_device *dev,u32 freq);extern void clockevents_config_and_register (struct clock_event_device *dev,u32 freq,unsigned long min_delta,unsigned long max_delta);
kernel/time/clockevents.c +1 -1
@@ -468,7 +468,7 @@ void clockevents_register_device(struct clock_event_device *dev) }EXPORT_SYMBOL_GPL (clockevents_register_device); -void clockevents_config (struct clock_event_device *dev,u32 freq) +static void clockevents_config (struct clock_event_device *dev,u32 freq) {u64 sec;
Signature · 1 line
2.9.2
RFC v4 7/22 many clockevent drivers: set ->min_delta_ticks and ->max_delta_ticks
In reply to Nicolai StangeThe struct clock_event_device has got the ->min_delta_ns, ->max_delta_ns, ->min_delta_ticks and ->max_delta_ticks members for setting the bounds of valid deltas to be programmed.
During operation, the clockevent core uses the ->*_delta_ns versions only. OTOH, the ->*_delta_ticks are currently used solely for the calculations of the ->*_delta_ns values in clockevents_config().
Quoted 1 line
From the hardware's point of view, giving the valid ranges in terms of
cycles, i.e. the ->*_delta_ticks values, is the natural choice.
Right now, as the ratio of ns/cycles is fixed, it doesn't matter much which unit of measurement is actually used by the clockevents core. (Well, apart from the fact that nearly all drivers which set the ->*_delta_ns manually don't round towards the correct direction).
This situation is going to change though: as NTP time correction awareness will be introduced to the clockevents core, the ratio ns/cycles will cease to stay fixed at all times.
In conclusion, every driver should hand its valid range in terms of an invariant unit, i.e. cycles, to the clockevent core.
Those drivers which configure their clockevent devices via clockevents_config_and_register() already do this.
This patch makes the vast majority of the remaining drivers do that, too. For the sake of bisectability, the manual settings of the ->*delta_ns by drivers will be removed by a later patch.
This patch was created with the help of the Coccinelle script below. It is noteworthy that for constructs like
ced.min_delta_ns = clockevent_delta2ns(<cycles>, &ced) + 1;
it is assumed that the +1 addition is done in order to compensate for any rounding introduced by the conversion of cycles to ns and thus, is irrelevant for ->min_delta_cycles.
@@
expression ced, cycles;
@@ced->min_delta_ns = clockevent_delta2ns(cycles, ced);
+ ced->min_delta_ticks = cycles;
@min@
expression ced, cycles;
@@
ced.min_delta_ns = clockevent_delta2ns(cycles, &ced);
+ ced.min_delta_ticks = cycles;
@min_round_up depends on !min@
expression ced, cycles;
@@
ced.min_delta_ns = clockevent_delta2ns(cycles, &ced) + 1;
+ ced.min_delta_ticks = cycles;
@@
identifier ced;
expression ns;
@@
struct clock_event_device ced = {
.mult = 1,
.shift = 0,
.min_delta_ns = ns,+ .min_delta_ticks = ns,
};
@p_max_neg_const@
expression ced;
constant int cycles;
@@ced->max_delta_ns = clockevent_delta2ns(-cycles, ced);
+ ced->max_delta_ticks = (unsigned long)-cycles;
@p_max depends on !p_max_neg_const@ expression ced, cycles;
@@ced->max_delta_ns = clockevent_delta2ns(cycles, ced);
+ ced->max_delta_ticks = cycles;
@@
expression ced, cycles;
@@
ced.max_delta_ns = clockevent_delta2ns(cycles, &ced);
+ ced.max_delta_ticks = cycles;
@@
identifier ced;
expression ns;
@@
struct clock_event_device ced = {
.mult = 1,
.shift = 0,
.max_delta_ns = ns,+ .max_delta_ticks = ns,
};
This patch differs from what spatch created only in some whitespace adjustments and an explicit cast added in arch/sparc/kernel/time_32.c.
--- arch/avr32/kernel/time.c | 2 ++ arch/blackfin/kernel/time-ts.c | 4 ++++ arch/c6x/platforms/timer64.c | 2 ++ arch/hexagon/kernel/time.c | 2 ++ arch/m68k/coldfire/pit.c | 2 ++ arch/microblaze/kernel/timer.c | 2 ++ arch/mips/alchemy/common/time.c | 4 +++- arch/mips/jz4740/time.c | 2 ++ arch/mips/kernel/cevt-bcm1480.c | 2 ++ arch/mips/kernel/cevt-ds1287.c | 2 ++ arch/mips/kernel/cevt-gt641xx.c | 2 ++ arch/mips/kernel/cevt-sb1250.c | 2 ++ arch/mips/kernel/cevt-txx9.c | 2 ++ arch/mips/loongson32/common/time.c | 2 ++ arch/mips/loongson64/common/cs5536/cs5536_mfgpt.c | 2 ++ arch/mips/loongson64/loongson-3/hpet.c | 2 ++ arch/mips/ralink/cevt-rt3352.c | 2 ++ arch/mips/sgi-ip27/ip27-timer.c | 2 ++ arch/mn10300/kernel/cevt-mn10300.c | 2 ++ arch/powerpc/kernel/time.c | 2 ++ arch/score/kernel/time.c | 2 ++ arch/sparc/kernel/time_32.c | 2 ++ arch/sparc/kernel/time_64.c | 2 ++ arch/um/kernel/time.c | 4 +++- arch/unicore32/kernel/time.c | 2 ++ arch/x86/kernel/apic/apic.c | 4 ++++ arch/x86/lguest/boot.c | 2 ++ arch/x86/xen/time.c | 4 ++++ drivers/clocksource/dw_apb_timer.c | 2 ++ drivers/clocksource/metag_generic.c | 2 ++ drivers/clocksource/numachip.c | 2 ++ drivers/clocksource/sh_cmt.c | 2 ++ drivers/clocksource/timer-atlas7.c | 2 ++ 33 files changed, 74 insertions(+), 2 deletions(-)
arch/avr32/kernel/time.c +2 -0
@@ -142,7 +142,9 @@ void __init time_init(void)/* setup COMPARE clockevent */ comparator.mult =div_sc (counter_hz,NSEC_PER_SEC , comparator.shift); comparator.max_delta_ns =clockevent_delta2ns ((u32 )~0 , &comparator); + comparator.max_delta_ticks = (u32 )~0 ; comparator.min_delta_ns =clockevent_delta2ns (50 , &comparator) +1 ; + comparator.min_delta_ticks =50 ; comparator.cpumask =cpumask_of (0 );sysreg_write (COMPARE ,0 );
arch/blackfin/kernel/time-ts.c +4 -0
@@ -230,7 +230,9 @@ static void __init bfin_gptmr0_clockevent_init(struct clock_event_device *evt) clock_tick =get_sclk (); evt->mult =div_sc (clock_tick,NSEC_PER_SEC , evt->shift); evt->max_delta_ns =clockevent_delta2ns (-1 , evt); + evt->max_delta_ticks = (unsigned long )-1 ; evt->min_delta_ns =clockevent_delta2ns (100 , evt); + evt->min_delta_ticks =100 ; evt->cpumask =cpumask_of (0 );
@@ -344,7 +346,9 @@ void bfin_coretmr_clockevent_init(void) clock_tick =get_cclk () /TIME_SCALE ; evt->mult =div_sc (clock_tick,NSEC_PER_SEC , evt->shift); evt->max_delta_ns =clockevent_delta2ns (-1 , evt); + evt->max_delta_ticks = (unsigned long )-1 ; evt->min_delta_ns =clockevent_delta2ns (100 , evt); + evt->min_delta_ticks =100 ; evt->cpumask =cpumask_of (cpu);
arch/c6x/platforms/timer64.c +2 -0
@@ -234,7 +234,9 @@ void __init timer64_init(void)clockevents_calc_mult_shift (cd, c6x_core_freq /TIMER_DIVISOR ,5 ); cd->max_delta_ns =clockevent_delta2ns (0x7fffffff , cd); + cd->max_delta_ticks =0x7fffffff ; cd->min_delta_ns =clockevent_delta2ns (250 , cd); + cd->min_delta_ticks =250 ; cd->cpumask =cpumask_of (smp_processor_id ());
arch/hexagon/kernel/time.c +2 -0
@@ -199,7 +199,9 @@ void __init time_init_deferred(void)clockevents_calc_mult_shift (ce_dev, sleep_clk_freq,4 ); ce_dev->max_delta_ns =clockevent_delta2ns (0x7fffffff , ce_dev); + ce_dev->max_delta_ticks =0x7fffffff ; ce_dev->min_delta_ns =clockevent_delta2ns (0xf , ce_dev); + ce_dev->min_delta_ticks =0xf ;#ifdef CONFIG_SMP setup_percpu_clockdev ();
arch/m68k/coldfire/pit.c +2 -0
@@ -149,8 +149,10 @@ void hw_timer_init(irq_handler_t handler) cf_pit_clockevent.mult =div_sc (FREQ ,NSEC_PER_SEC ,32 ); cf_pit_clockevent.max_delta_ns =clockevent_delta2ns (0xFFFF , &cf_pit_clockevent); + cf_pit_clockevent.max_delta_ticks =0xFFFF ; cf_pit_clockevent.min_delta_ns =clockevent_delta2ns (0x3f , &cf_pit_clockevent); + cf_pit_clockevent.min_delta_ticks =0x3f ;clockevents_register_device (&cf_pit_clockevent);setup_irq (MCF_IRQ_PIT1 , &pit_irq);
arch/microblaze/kernel/timer.c +2 -0
@@ -177,8 +177,10 @@ static __init int xilinx_clockevent_init(void) clockevent_xilinx_timer.shift); clockevent_xilinx_timer.max_delta_ns =clockevent_delta2ns ((u32 )~0 , &clockevent_xilinx_timer); + clockevent_xilinx_timer.max_delta_ticks = (u32 )~0 ; clockevent_xilinx_timer.min_delta_ns =clockevent_delta2ns (1 , &clockevent_xilinx_timer); + clockevent_xilinx_timer.min_delta_ticks =1 ; clockevent_xilinx_timer.cpumask =cpumask_of (0 );clockevents_register_device (&clockevent_xilinx_timer);
arch/mips/alchemy/common/time.c +3 -1
@@ -138,7 +138,9 @@ static int __init alchemy_time_init(unsigned int m2int) cd->shift =32 ; cd->mult =div_sc (32768 ,NSEC_PER_SEC , cd->shift); cd->max_delta_ns =clockevent_delta2ns (0xffffffff , cd); - cd->min_delta_ns =clockevent_delta2ns (9 , cd);/* ~0.28ms */ + cd->max_delta_ticks =0xffffffff ; + cd->min_delta_ns =clockevent_delta2ns (9 , cd); + cd->min_delta_ticks =9 ;/* ~0.28ms */ clockevents_register_device (cd);setup_irq (m2int, &au1x_rtcmatch2_irqaction);
arch/mips/jz4740/time.c +2 -0
@@ -145,7 +145,9 @@ void __init plat_time_init(void)clockevent_set_clock (&jz4740_clockevent, clk_rate); jz4740_clockevent.min_delta_ns =clockevent_delta2ns (100 , &jz4740_clockevent); + jz4740_clockevent.min_delta_ticks =100 ; jz4740_clockevent.max_delta_ns =clockevent_delta2ns (0xffff , &jz4740_clockevent); + jz4740_clockevent.max_delta_ticks =0xffff ; jz4740_clockevent.cpumask =cpumask_of (0 );clockevents_register_device (&jz4740_clockevent);
arch/mips/kernel/cevt-bcm1480.c +2 -0
@@ -123,7 +123,9 @@ void sb1480_clockevent_init(void)CLOCK_EVT_FEAT_ONESHOT ;clockevent_set_clock (cd,V_SCD_TIMER_FREQ ); cd->max_delta_ns =clockevent_delta2ns (0x7fffff , cd); + cd->max_delta_ticks =0x7fffff ; cd->min_delta_ns =clockevent_delta2ns (2 , cd); + cd->min_delta_ticks =2 ; cd->rating =200 ; cd->irq = irq; cd->cpumask =cpumask_of (cpu);
arch/mips/kernel/cevt-ds1287.c +2 -0
@@ -128,7 +128,9 @@ int __init ds1287_clockevent_init(int irq) cd->irq = irq;clockevent_set_clock (cd,32768 ); cd->max_delta_ns =clockevent_delta2ns (0x7fffffff , cd); + cd->max_delta_ticks =0x7fffffff ; cd->min_delta_ns =clockevent_delta2ns (0x300 , cd); + cd->min_delta_ticks =0x300 ; cd->cpumask =cpumask_of (0 );clockevents_register_device (&ds1287_clockevent);
arch/mips/kernel/cevt-gt641xx.c +2 -0
@@ -152,7 +152,9 @@ static int __init gt641xx_timer0_clockevent_init(void) cd->rating =200 + gt641xx_base_clock /10000000 ;clockevent_set_clock (cd, gt641xx_base_clock); cd->max_delta_ns =clockevent_delta2ns (0x7fffffff , cd); + cd->max_delta_ticks =0x7fffffff ; cd->min_delta_ns =clockevent_delta2ns (0x300 , cd); + cd->min_delta_ticks =0x300 ; cd->cpumask =cpumask_of (0 );clockevents_register_device (>641xx_timer0_clockevent);
arch/mips/kernel/cevt-sb1250.c +2 -0
@@ -123,7 +123,9 @@ void sb1250_clockevent_init(void)CLOCK_EVT_FEAT_ONESHOT ;clockevent_set_clock (cd,V_SCD_TIMER_FREQ ); cd->max_delta_ns =clockevent_delta2ns (0x7fffff , cd); + cd->max_delta_ticks =0x7fffff ; cd->min_delta_ns =clockevent_delta2ns (2 , cd); + cd->min_delta_ticks =2 ; cd->rating =200 ; cd->irq = irq; cd->cpumask =cpumask_of (cpu);
arch/mips/kernel/cevt-txx9.c +2 -0
@@ -196,7 +196,9 @@ void __init txx9_clockevent_init(unsigned long baseaddr, int irq,clockevent_set_clock (cd,TIMER_CLK (imbusclk)); cd->max_delta_ns =clockevent_delta2ns (0xffffffff >> (32 -TXX9_TIMER_BITS ), cd); + cd->max_delta_ticks =0xffffffff >> (32 -TXX9_TIMER_BITS ); cd->min_delta_ns =clockevent_delta2ns (0xf , cd); + cd->min_delta_ticks =0xf ; cd->irq = irq; cd->cpumask =cpumask_of (0 ),clockevents_register_device (cd);
arch/mips/loongson32/common/time.c +2 -0
@@ -199,7 +199,9 @@ static void __init ls1x_time_init(void)clockevent_set_clock (cd, mips_hpt_frequency); cd->max_delta_ns =clockevent_delta2ns (0xffffff , cd); + cd->max_delta_ticks =0xffffff ; cd->min_delta_ns =clockevent_delta2ns (0x000300 , cd); + cd->min_delta_ticks =0x000300 ; cd->cpumask =cpumask_of (smp_processor_id ());clockevents_register_device (cd);
arch/mips/loongson64/common/cs5536/cs5536_mfgpt.c +2 -0
@@ -123,7 +123,9 @@ void __init setup_mfgpt0_timer(void) cd->cpumask =cpumask_of (cpu);clockevent_set_clock (cd,MFGPT_TICK_RATE ); cd->max_delta_ns =clockevent_delta2ns (0xffff , cd); + cd->max_delta_ticks =0xffff ; cd->min_delta_ns =clockevent_delta2ns (0xf , cd); + cd->min_delta_ticks =0xf ;/* Enable MFGPT0 Comparator 2 Output to the Interrupt Mapper */ _wrmsr (DIVIL_MSR_REG (MFGPT_IRQ ),0 ,0x100 );
arch/mips/loongson64/loongson-3/hpet.c +2 -0
@@ -241,7 +241,9 @@ void __init setup_hpet_timer(void) cd->cpumask =cpumask_of (cpu);clockevent_set_clock (cd,HPET_FREQ ); cd->max_delta_ns =clockevent_delta2ns (0x7fffffff , cd); + cd->max_delta_ticks =0x7fffffff ; cd->min_delta_ns =clockevent_delta2ns (HPET_MIN_PROG_DELTA , cd); + cd->min_delta_ticks =HPET_MIN_PROG_DELTA ;clockevents_register_device (cd);setup_irq (HPET_T0_IRQ , &hpet_irq);
arch/mips/ralink/cevt-rt3352.c +2 -0
@@ -129,7 +129,9 @@ static int __init ralink_systick_init(struct device_node *np) systick.dev.name = np->name;clockevents_calc_mult_shift (&systick.dev,SYSTICK_FREQ ,60 ); systick.dev.max_delta_ns =clockevent_delta2ns (0x7fff , &systick.dev); + systick.dev.max_delta_ticks =0x7fff ; systick.dev.min_delta_ns =clockevent_delta2ns (0x3 , &systick.dev); + systick.dev.min_delta_ticks =0x3 ; systick.dev.irq =irq_of_parse_and_map (np,0 );if (!systick.dev.irq) {pr_err ("%s: request_irq failed" , np->name);
arch/mips/sgi-ip27/ip27-timer.c +2 -0
@@ -113,7 +113,9 @@ void hub_rt_clock_event_init(void) cd->features =CLOCK_EVT_FEAT_ONESHOT ;clockevent_set_clock (cd,CYCLES_PER_SEC ); cd->max_delta_ns =clockevent_delta2ns (0xfffffffffffff , cd); + cd->max_delta_ticks =0xfffffffffffff ; cd->min_delta_ns =clockevent_delta2ns (0x300 , cd); + cd->min_delta_ticks =0x300 ; cd->rating =200 ; cd->irq = irq; cd->cpumask =cpumask_of (cpu);
arch/mn10300/kernel/cevt-mn10300.c +2 -0
@@ -98,7 +98,9 @@ int __init init_clockevents(void)/* Calculate the min / max delta */ cd->max_delta_ns =clockevent_delta2ns (TMJCBR_MAX , cd); + cd->max_delta_ticks =TMJCBR_MAX ; cd->min_delta_ns =clockevent_delta2ns (100 , cd); + cd->min_delta_ticks =100 ; cd->rating =200 ; cd->cpumask =cpumask_of (smp_processor_id ());
arch/powerpc/kernel/time.c +2 -0
@@ -972,8 +972,10 @@ static void __init init_decrementer_clockevent(void) decrementer_clockevent.max_delta_ns =clockevent_delta2ns (decrementer_max, &decrementer_clockevent); + decrementer_clockevent.max_delta_ticks = decrementer_max; decrementer_clockevent.min_delta_ns =clockevent_delta2ns (2 , &decrementer_clockevent); + decrementer_clockevent.min_delta_ticks =2 ;register_decrementer_clockevent (cpu); }
arch/score/kernel/time.c +2 -0
@@ -81,8 +81,10 @@ void __init time_init(void) score_clockevent.shift); score_clockevent.max_delta_ns =clockevent_delta2ns ((u32 )~0 , &score_clockevent); + score_clockevent.max_delta_ticks = (u32 )~0 ; score_clockevent.min_delta_ns =clockevent_delta2ns (50 , &score_clockevent) +1 ; + score_clockevent.min_delta_ticks =50 ; score_clockevent.cpumask =cpumask_of (0 );clockevents_register_device (&score_clockevent); }
arch/sparc/kernel/time_32.c +2 -0
@@ -228,7 +228,9 @@ void register_percpu_ce(int cpu) ce->mult =div_sc (sparc_config.clock_rate,NSEC_PER_SEC , ce->shift); ce->max_delta_ns =clockevent_delta2ns (sparc_config.clock_rate, ce); + ce->max_delta_ticks = (unsigned long )sparc_config.clock_rate; ce->min_delta_ns =clockevent_delta2ns (100 , ce); + ce->min_delta_ticks =100 ;clockevents_register_device (ce); }
arch/sparc/kernel/time_64.c +2 -0
@@ -796,8 +796,10 @@ void __init time_init(void) sparc64_clockevent.max_delta_ns =clockevent_delta2ns (0x7fffffffffffffffUL , &sparc64_clockevent); + sparc64_clockevent.max_delta_ticks =0x7fffffffffffffffUL ; sparc64_clockevent.min_delta_ns =clockevent_delta2ns (0xF , &sparc64_clockevent); + sparc64_clockevent.min_delta_ticks =0xF ;printk ("clockevent: mult[%x] shift[%d]\n" , sparc64_clockevent.mult, sparc64_clockevent.shift);
arch/um/kernel/time.c +3 -1
@@ -65,7 +65,9 @@ static struct clock_event_device timer_clockevent = { .set_next_event = itimer_next_event, .shift = 0, .max_delta_ns = 0xffffffff, - .min_delta_ns = TIMER_MIN_DELTA, //microsecond resolution should be enough for anyone, same as 640K RAM + .max_delta_ticks = 0xffffffff, + .min_delta_ns = TIMER_MIN_DELTA, + .min_delta_ticks = TIMER_MIN_DELTA, // microsecond resolution should be enough for anyone, same as 640K RAM .irq = 0, .mult = 1, };
arch/unicore32/kernel/time.c +2 -0
@@ -91,8 +91,10 @@ void __init time_init(void) ckevt_puv3_osmr0.max_delta_ns =clockevent_delta2ns (0x7fffffff , &ckevt_puv3_osmr0); + ckevt_puv3_osmr0.max_delta_ticks =0x7fffffff ; ckevt_puv3_osmr0.min_delta_ns =clockevent_delta2ns (MIN_OSCR_DELTA *2 , &ckevt_puv3_osmr0) +1 ; + ckevt_puv3_osmr0.min_delta_ticks =MIN_OSCR_DELTA *2 ; ckevt_puv3_osmr0.cpumask =cpumask_of (0 );setup_irq (IRQ_TIMER0 , &puv3_timer_irq);
arch/x86/kernel/apic/apic.c +4 -0
@@ -729,8 +729,10 @@ static int __init calibrate_APIC_clock(void) TICK_NSEC, lapic_clockevent.shift); lapic_clockevent.max_delta_ns =clockevent_delta2ns (0x7FFFFF , &lapic_clockevent); + lapic_clockevent.max_delta_ticks =0x7FFFFF ; lapic_clockevent.min_delta_ns =clockevent_delta2ns (0xF , &lapic_clockevent); + lapic_clockevent.min_delta_ticks =0xF ; lapic_clockevent.features &= ~CLOCK_EVT_FEAT_DUMMY ;return 0 ; }
@@ -776,8 +778,10 @@ static int __init calibrate_APIC_clock(void) lapic_clockevent.shift); lapic_clockevent.max_delta_ns =clockevent_delta2ns (0x7FFFFFFF , &lapic_clockevent); + lapic_clockevent.max_delta_ticks =0x7FFFFFFF ; lapic_clockevent.min_delta_ns =clockevent_delta2ns (0xF , &lapic_clockevent); + lapic_clockevent.min_delta_ticks =0xF ; lapic_timer_frequency = (delta *APIC_DIVISOR ) /LAPIC_CAL_LOOPS ;
arch/x86/lguest/boot.c +2 -0
@@ -1008,7 +1008,9 @@ static struct clock_event_device lguest_clockevent = { .mult = 1, .shift = 0, .min_delta_ns = LG_CLOCK_MIN_DELTA, + .min_delta_ticks = LG_CLOCK_MIN_DELTA, .max_delta_ns = LG_CLOCK_MAX_DELTA, + .max_delta_ticks = LG_CLOCK_MAX_DELTA, };/*
arch/x86/xen/time.c +4 -0
@@ -209,7 +209,9 @@ static const struct clock_event_device xen_timerop_clockevent = { .features = CLOCK_EVT_FEAT_ONESHOT, .max_delta_ns = 0xffffffff, + .max_delta_ticks = 0xffffffff, .min_delta_ns = TIMER_SLOP, + .min_delta_ticks = TIMER_SLOP, .mult = 1, .shift = 0,
@@ -268,7 +270,9 @@ static const struct clock_event_device xen_vcpuop_clockevent = { .features = CLOCK_EVT_FEAT_ONESHOT, .max_delta_ns = 0xffffffff, + .max_delta_ticks = 0xffffffff, .min_delta_ns = TIMER_SLOP, + .min_delta_ticks = TIMER_SLOP, .mult = 1, .shift = 0,
drivers/clocksource/dw_apb_timer.c +2 -0
@@ -257,7 +257,9 @@ dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,clockevents_calc_mult_shift (&dw_ced->ced, freq,APBT_MIN_PERIOD ); dw_ced->ced.max_delta_ns =clockevent_delta2ns (0x7fffffff , &dw_ced->ced); + dw_ced->ced.max_delta_ticks =0x7fffffff ; dw_ced->ced.min_delta_ns =clockevent_delta2ns (5000 , &dw_ced->ced); + dw_ced->ced.min_delta_ticks =5000 ; dw_ced->ced.cpumask =cpumask_of (cpu); dw_ced->ced.features =CLOCK_EVT_FEAT_PERIODIC |CLOCK_EVT_FEAT_ONESHOT |CLOCK_EVT_FEAT_DYNIRQ ;
drivers/clocksource/metag_generic.c +2 -0
@@ -114,7 +114,9 @@ static int arch_timer_starting_cpu(unsigned int cpu) clk->mult =div_sc (hwtimer_freq,NSEC_PER_SEC , clk->shift); clk->max_delta_ns =clockevent_delta2ns (0x7fffffff , clk); + clk->max_delta_ticks =0x7fffffff ; clk->min_delta_ns =clockevent_delta2ns (0xf , clk); + clk->min_delta_ticks =0xf ; clk->cpumask =cpumask_of (cpu);clockevents_register_device (clk);
drivers/clocksource/numachip.c +2 -0
@@ -51,7 +51,9 @@ static struct clock_event_device numachip2_clockevent = { .mult = 1, .shift = 0, .min_delta_ns = 1250, + .min_delta_ticks = 1250, .max_delta_ns = LONG_MAX, + .max_delta_ticks = LONG_MAX, };static void numachip_timer_interrupt (void )
drivers/clocksource/sh_cmt.c +2 -0
@@ -814,7 +814,9 @@ static int sh_cmt_register_clockevent(struct sh_cmt_channel *ch, ced->shift =32 ; ced->mult =div_sc (ch->cmt->rate,NSEC_PER_SEC , ced->shift); ced->max_delta_ns =clockevent_delta2ns (ch->max_match_value, ced); + ced->max_delta_ticks = ch->max_match_value; ced->min_delta_ns =clockevent_delta2ns (0x1f , ced); + ced->min_delta_ticks =0x1f ;dev_info (&ch->cmt->pdev->dev,"ch%u: used for clock events\n" , ch->index);
drivers/clocksource/timer-atlas7.c +2 -0
@@ -192,7 +192,9 @@ static int sirfsoc_local_timer_starting_cpu(unsigned int cpu) ce->set_next_event = sirfsoc_timer_set_next_event;clockevents_calc_mult_shift (ce, atlas7_timer_rate,60 ); ce->max_delta_ns =clockevent_delta2ns (-2 , ce); + ce->max_delta_ticks = (unsigned long )-2 ; ce->min_delta_ns =clockevent_delta2ns (2 , ce); + ce->min_delta_ticks =2 ; ce->cpumask =cpumask_of (cpu); action->dev_id = ce;
Signature · 1 line
2.9.2
RFC v4 8/22 arch/s390/kernel/time: set ->min_delta_ticks and ->max_delta_ticks
In reply to Nicolai StangeWith the yet to come introduction of NTP correction awareness to the clockevent core, drivers should report their valid ranges in units of cycles to the latter.
Currently, the s390's CPU timer clockevent device is initialized as follows:
cd->min_delta_ns = 1; cd->max_delta_ns = LONG_MAX;
Note that the device's time to cycle conversion factor, i.e. cd->mult / (2^cd->shift), is approx. equal to 4.
Hence, this would translate to
cd->min_delta_ticks = 4; cd->max_delta_ticks = 4 * LONG_MAX;
However, a minimum value of 1ns is in the range of noise anyway and the clockevent core will take care of this by increasing it to 1us or so. Furthermore, 4*LONG_MAX will overflow the unsigned long argument the clockevent devices gets programmed with.
Thus, initialize ->min_delta_ticks with 1 and ->max_delta_ticks with ULONG_MAX.
--- arch/s390/kernel/time.c | 2 ++ 1 file changed, 2 insertions(+)
arch/s390/kernel/time.c +2 -0
@@ -166,7 +166,9 @@ void init_cpu_timer(void) cd->mult =16777 ; cd->shift =12 ; cd->min_delta_ns =1 ; + cd->min_delta_ticks =1 ; cd->max_delta_ns =LONG_MAX ; + cd->max_delta_ticks =ULONG_MAX ; cd->rating =400 ; cd->cpumask =cpumask_of (cpu); cd->set_next_event = s390_next_event;
Signature · 1 line
2.9.2
RFC v4 9/22 arch/x86/platform/uv/uv_time: set ->min_delta_ticks and ->max_delta_ticks
In reply to Nicolai StangeWith the yet to come introduction of NTP correction awareness to the clockevent core, drivers should report their valid ranges in units of cycles to the latter.
Currently, the x86's uv rtc clockevent device is initialized as follows:
clock_event_device_uv.min_delta_ns = NSEC_PER_SEC /
sn_rtc_cycles_per_second;
clock_event_device_uv.max_delta_ns = clocksource_uv.mask *
(NSEC_PER_SEC / sn_rtc_cycles_per_second);This translates to a ->min_delta_ticks value of 1 and a ->max_delta_ticks value of clocksource_uv.mask.
Initialize ->min_delta_ticks and ->max_delta_ticks with these values respectively.
---
Notes:
Compile-only tested through an allmodconfig build on ARCH=x86_64.
arch/x86/platform/uv/uv_time.c | 2 ++ 1 file changed, 2 insertions(+)
arch/x86/platform/uv/uv_time.c +2 -0
@@ -390,9 +390,11 @@ static __init int uv_rtc_setup_clock(void) clock_event_device_uv.min_delta_ns =NSEC_PER_SEC / sn_rtc_cycles_per_second; + clock_event_device_uv.min_delta_ticks =1 ; clock_event_device_uv.max_delta_ns = clocksource_uv.mask * (NSEC_PER_SEC / sn_rtc_cycles_per_second); + clock_event_device_uv.max_delta_ticks = clocksource_uv.mask; rc =schedule_on_each_cpu (uv_rtc_register_clockevents);if (rc) {
Signature · 1 line
2.9.2
RFC v4 10/22 arch/tile/kernel/time: set ->min_delta_ticks and ->max_delta_ticks
In reply to Nicolai StangeWith the yet to come introduction of NTP correction awareness to the clockevent core, drivers should report their valid ranges in units of cycles to the latter.
Currently, the tile's timer clockevent device is initialized as follows:
evt->max_delta_ns = clockevent_delta2ns(MAX_TICK, evt);
and
.min_delta_ns = 1000,
The first one translates to a ->max_delta_ticks value of MAX_TICK. For the latter, note that the clockevent core will superimpose a minimum of 1us by itself -- setting ->min_delta_ticks to 1 is safe here.
Initialize ->min_delta_ticks and ->max_delta_ticks with these values.
--- arch/tile/kernel/time.c | 2 ++ 1 file changed, 2 insertions(+)
arch/tile/kernel/time.c +2 -0
@@ -154,6 +154,7 @@ static DEFINE_PER_CPU(struct clock_event_device, tile_timer) = { .name = "tile timer", .features = CLOCK_EVT_FEAT_ONESHOT, .min_delta_ns = 1000, + .min_delta_ticks = 1, .rating = 100, .irq = -1, .set_next_event = tile_timer_set_next_event,
@@ -169,6 +170,7 @@ void setup_tile_timer(void)/* Fill in fields that are speed-specific. */ clockevents_calc_mult_shift (evt, cycles_per_sec,TILE_MINSEC ); evt->max_delta_ns =clockevent_delta2ns (MAX_TICK , evt); + evt->max_delta_ticks =MAX_TICK ;/* Mark as being for this cpu only. */ evt->cpumask =cpumask_of (smp_processor_id ());
Signature · 1 line
2.9.2
Re: [RFC v4 10/22] arch/tile/kernel/time: set ->min_delta_ticks and ->max_delta_ticks
Quoted 16 lines · Nicolai Stange
With the yet to come introduction of NTP correction awareness to the clockevent core, drivers should report their valid ranges in units of cycles to the latter.
Currently, the tile's timer clockevent device is initialized as follows:
evt->max_delta_ns = clockevent_delta2ns(MAX_TICK, evt);and
.min_delta_ns = 1000,
The first one translates to a ->max_delta_ticks value of
MAX_TICK. For the latter, note that the clockevent core will superimpose a minimum of 1us by itself -- setting ->min_delta_ticks to 1 is safe here.
Quoted 6 lines
Initialize ->min_delta_ticks and ->max_delta_ticks with these values.
Signed-off-by Nicolai Stange <[email protected]>--- arch/tile/kernel/time.c | 2 ++ 1 file changed, 2 insertions(+)
Thanks. Taken into the tile tree.
Signature · 2 lines
Chris Metcalf, Mellanox Technologies http://www.mellanox.com
Re: [RFC v4 10/22] arch/tile/kernel/time: set ->min_delta_ticks and ->max_delta_ticks
Quoted 17 lines · Chris Metcalf
Quoted 16 lines · Nicolai Stange
With the yet to come introduction of NTP correction awareness to the clockevent core, drivers should report their valid ranges in units of cycles to the latter.
Currently, the tile's timer clockevent device is initialized as follows:
evt->max_delta_ns = clockevent_delta2ns(MAX_TICK, evt);and
.min_delta_ns = 1000,
The first one translates to a ->max_delta_ticks value of
MAX_TICK. For the latter, note that the clockevent core will superimpose a minimum of 1us by itself -- setting ->min_delta_ticks to 1 is safe here.
Quoted 8 lines
Quoted 6 lines
Initialize ->min_delta_ticks and ->max_delta_ticks with these values.
Signed-off-by Nicolai Stange <[email protected]>--- arch/tile/kernel/time.c | 2 ++ 1 file changed, 2 insertions(+)Thanks. Taken into the tile tree.
I thank you for caring, but may I ask you to drop this again?
The reasons are twofold:
1.) It isn't clear yet whether this series is worth it and will be
accepted at all (hence the "RFC" tag). This patch by itself would not make any sense.
2.) The patches in this series depend heavily on each other. So I'd
personally prefer if those more or less trivial changes to arch/ could be taken through the same tree as the rest, i.e. through the timers/core tree. I have no idea whether this is feasible and perhaps I'll have to get back to you. But for now, getting this patch removed from your tree would certainly simplify things a lot for me...
Thanks and sorry for the inconvenience,
Nicolai Stange
Re: [RFC v4 10/22] arch/tile/kernel/time: set ->min_delta_ticks and ->max_delta_ticks
Quoted 19 lines · Nicolai Stange
Quoted 17 lines · Chris Metcalf
Quoted 16 lines · Nicolai Stange
With the yet to come introduction of NTP correction awareness to the clockevent core, drivers should report their valid ranges in units of cycles to the latter.
Currently, the tile's timer clockevent device is initialized as follows:
evt->max_delta_ns = clockevent_delta2ns(MAX_TICK, evt);and
.min_delta_ns = 1000,The first one translates to a ->max_delta_ticks value of
MAX_TICK. For the latter, note that the clockevent core will superimpose a minimum of 1us by itself -- setting ->min_delta_ticks to 1 is safe here.
Quoted 8 lines
Quoted 7 lines
Quoted 6 lines
Initialize ->min_delta_ticks and ->max_delta_ticks with these values.
Signed-off-by Nicolai Stange <[email protected]>--- arch/tile/kernel/time.c | 2 ++ 1 file changed, 2 insertions(+)Thanks. Taken into the tile tree.
I thank you for caring, but may I ask you to drop this again?
No problem - I have removed it.
Signature · 2 lines
Chris Metcalf, Mellanox Technologies http://www.mellanox.com
RFC v4 11/22 clockevents: always initialize ->min_delta_ns and ->max_delta_ns
In reply to Nicolai StangeNow that all clockevent drivers set ->min_delta_ticks and ->max_delta_ticks independently of whether they use clockevents_config*() or not, the clockevent core can calculate ->min_delta_ns and ->max_delta_ns from these unconditionally.
The goal is to prepare the clockevent core for introducing NTP rate correction awareness: as the clockevent devices' rates will get adjusted, the ->*_delta_ns won't stay fixed but need to get changed as well.
Thus, make the clockevent core calculate the ->*_delta_ns values from the invariant ->_delta_ticks ones at device registration.
In order to facilitate this, move the corresponding code from clockevents_config() into its own helper function, __clockevents_update_bounds() and invoke this where needed, in particular from clockevents_register_device().
Note that there is a side effect affecting those drivers
- not using clockevents_config*()
- and manually initializing their ->min_delta_ns to values less than 1us. In order to avoid "pointless noise", the clockevent core always sets ->min_delta_ns to values >= 1us, and thus, those drivers' ->min_delta_ns is now increased. I think that this side effect is desired though.
--- kernel/time/clockevents.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
kernel/time/clockevents.c +16 -2
@@ -442,6 +442,19 @@ int clockevents_unbind_device(struct clock_event_device *ced, int cpu) }EXPORT_SYMBOL_GPL (clockevents_unbind_device); +static void __clockevents_update_bounds (struct clock_event_device *dev) +{ +if (!(dev->features &CLOCK_EVT_FEAT_ONESHOT )) +return ; + +/* +* cev_delta2ns() never returns values less than 1us and thus, +* we'll never program any ced with anything less. +*/ + dev->min_delta_ns =cev_delta2ns (dev->min_delta_ticks, dev, false); + dev->max_delta_ns =cev_delta2ns (dev->max_delta_ticks, dev, true); +} +/** * clockevents_register_device - register a clock event device * @dev: device to register
@@ -458,6 +471,8 @@ void clockevents_register_device(struct clock_event_device *dev) dev->cpumask =cpumask_of (smp_processor_id ()); } +__clockevents_update_bounds (dev); +raw_spin_lock_irqsave (&clockevents_lock, flags);list_add (&dev->list, &clockevent_devices);
@@ -488,8 +503,6 @@ static void clockevents_config(struct clock_event_device *dev, u32 freq) sec =600 ;clockevents_calc_mult_shift (dev, freq, sec); - dev->min_delta_ns =cev_delta2ns (dev->min_delta_ticks, dev, false); - dev->max_delta_ns =cev_delta2ns (dev->max_delta_ticks, dev, true); }/**
@@ -515,6 +528,7 @@ EXPORT_SYMBOL_GPL(clockevents_config_and_register);int __clockevents_update_freq (struct clock_event_device *dev,u32 freq) {clockevents_config (dev, freq); +__clockevents_update_bounds (dev);if (clockevent_state_oneshot (dev))return clockevents_program_event (dev, dev->next_event, false);
Signature · 1 line
2.9.2
RFC v4 12/22 many clockevent drivers: don't set ->min_delta_ns and ->max_delta_ns
In reply to Nicolai StangeNow that the clockevent core always initializes the ->*_delta_ns values from their ->_delta_ticks counterparts, there is no point in having the clockevent devices' drivers doing so as well.
Don't initialize ->min_delta_ns and ->max_delta_ns from the clockevent devices' drivers.
This patch was created with the help of the Coccinelle script below. One initialization of ->min_delta_ns in arch/tile/kernel/time.c gets missed by this Cocci script and its removal has been manually added.
@@
expression ced;
expression ns, cycles;
@@
- ced->min_delta_ns = ns;
...ced->min_delta_ticks = cycles;
@@
expression ced;
expression ns, cycles;
@@
- ced.min_delta_ns = ns;
...
ced.min_delta_ticks = cycles;
@@
identifier ced;
expression ns, cycles;
@@
struct clock_event_device ced = {
- .min_delta_ns = ns,
.min_delta_ticks = cycles,
};
@@
expression ced;
expression ns, cycles;
@@
- ced->max_delta_ns = ns;
...ced->max_delta_ticks = cycles;
@@
expression ced;
expression ns, cycles;
@@
- ced.max_delta_ns = ns;
...
ced.max_delta_ticks = cycles;
@@
identifier ced;
expression ns, cycles;
@@
struct clock_event_device ced = {
- .max_delta_ns = ns,
.max_delta_ticks = cycles,
};
--- arch/avr32/kernel/time.c | 2 -- arch/blackfin/kernel/time-ts.c | 4 ---- arch/c6x/platforms/timer64.c | 2 -- arch/hexagon/kernel/time.c | 2 -- arch/m68k/coldfire/pit.c | 4 ---- arch/microblaze/kernel/timer.c | 4 ---- arch/mips/alchemy/common/time.c | 2 -- arch/mips/jz4740/time.c | 2 -- arch/mips/kernel/cevt-bcm1480.c | 2 -- arch/mips/kernel/cevt-ds1287.c | 2 -- arch/mips/kernel/cevt-gt641xx.c | 2 -- arch/mips/kernel/cevt-sb1250.c | 2 -- arch/mips/kernel/cevt-txx9.c | 3 --- arch/mips/loongson32/common/time.c | 2 -- arch/mips/loongson64/common/cs5536/cs5536_mfgpt.c | 2 -- arch/mips/loongson64/loongson-3/hpet.c | 2 -- arch/mips/ralink/cevt-rt3352.c | 2 -- arch/mips/sgi-ip27/ip27-timer.c | 2 -- arch/mn10300/kernel/cevt-mn10300.c | 2 -- arch/powerpc/kernel/time.c | 4 ---- arch/s390/kernel/time.c | 2 -- arch/score/kernel/time.c | 4 ---- arch/sparc/kernel/time_32.c | 2 -- arch/sparc/kernel/time_64.c | 4 ---- arch/tile/kernel/time.c | 2 -- arch/um/kernel/time.c | 2 -- arch/unicore32/kernel/time.c | 4 ---- arch/x86/kernel/apic/apic.c | 8 -------- arch/x86/lguest/boot.c | 2 -- arch/x86/platform/uv/uv_time.c | 4 ---- arch/x86/xen/time.c | 4 ---- drivers/clocksource/dw_apb_timer.c | 3 --- drivers/clocksource/metag_generic.c | 2 -- drivers/clocksource/numachip.c | 2 -- drivers/clocksource/sh_cmt.c | 2 -- drivers/clocksource/timer-atlas7.c | 2 -- kernel/time/tick-broadcast-hrtimer.c | 2 -- 37 files changed, 100 deletions(-)
arch/avr32/kernel/time.c +0 -2
@@ -141,9 +141,7 @@ void __init time_init(void)/* setup COMPARE clockevent */ comparator.mult =div_sc (counter_hz,NSEC_PER_SEC , comparator.shift); - comparator.max_delta_ns =clockevent_delta2ns ((u32 )~0 , &comparator); comparator.max_delta_ticks = (u32 )~0 ; - comparator.min_delta_ns =clockevent_delta2ns (50 , &comparator) +1 ; comparator.min_delta_ticks =50 ; comparator.cpumask =cpumask_of (0 );
arch/blackfin/kernel/time-ts.c +0 -4
@@ -229,9 +229,7 @@ static void __init bfin_gptmr0_clockevent_init(struct clock_event_device *evt) clock_tick =get_sclk (); evt->mult =div_sc (clock_tick,NSEC_PER_SEC , evt->shift); - evt->max_delta_ns =clockevent_delta2ns (-1 , evt); evt->max_delta_ticks = (unsigned long )-1 ; - evt->min_delta_ns =clockevent_delta2ns (100 , evt); evt->min_delta_ticks =100 ; evt->cpumask =cpumask_of (0 );
@@ -345,9 +343,7 @@ void bfin_coretmr_clockevent_init(void) clock_tick =get_cclk () /TIME_SCALE ; evt->mult =div_sc (clock_tick,NSEC_PER_SEC , evt->shift); - evt->max_delta_ns =clockevent_delta2ns (-1 , evt); evt->max_delta_ticks = (unsigned long )-1 ; - evt->min_delta_ns =clockevent_delta2ns (100 , evt); evt->min_delta_ticks =100 ; evt->cpumask =cpumask_of (cpu);
arch/c6x/platforms/timer64.c +0 -2
@@ -233,9 +233,7 @@ void __init timer64_init(void)clockevents_calc_mult_shift (cd, c6x_core_freq /TIMER_DIVISOR ,5 ); - cd->max_delta_ns =clockevent_delta2ns (0x7fffffff , cd); cd->max_delta_ticks =0x7fffffff ; - cd->min_delta_ns =clockevent_delta2ns (250 , cd); cd->min_delta_ticks =250 ; cd->cpumask =cpumask_of (smp_processor_id ());
arch/hexagon/kernel/time.c +0 -2
@@ -198,9 +198,7 @@ void __init time_init_deferred(void)*/ clockevents_calc_mult_shift (ce_dev, sleep_clk_freq,4 ); - ce_dev->max_delta_ns =clockevent_delta2ns (0x7fffffff , ce_dev); ce_dev->max_delta_ticks =0x7fffffff ; - ce_dev->min_delta_ns =clockevent_delta2ns (0xf , ce_dev); ce_dev->min_delta_ticks =0xf ;#ifdef CONFIG_SMP
arch/m68k/coldfire/pit.c +0 -4
@@ -147,11 +147,7 @@ void hw_timer_init(irq_handler_t handler) { cf_pit_clockevent.cpumask =cpumask_of (smp_processor_id ()); cf_pit_clockevent.mult =div_sc (FREQ ,NSEC_PER_SEC ,32 ); - cf_pit_clockevent.max_delta_ns = -clockevent_delta2ns (0xFFFF , &cf_pit_clockevent); cf_pit_clockevent.max_delta_ticks =0xFFFF ; - cf_pit_clockevent.min_delta_ns = -clockevent_delta2ns (0x3f , &cf_pit_clockevent); cf_pit_clockevent.min_delta_ticks =0x3f ;clockevents_register_device (&cf_pit_clockevent);
arch/microblaze/kernel/timer.c +0 -4
@@ -175,11 +175,7 @@ static __init int xilinx_clockevent_init(void) clockevent_xilinx_timer.mult =div_sc (timer_clock_freq,NSEC_PER_SEC , clockevent_xilinx_timer.shift); - clockevent_xilinx_timer.max_delta_ns = -clockevent_delta2ns ((u32 )~0 , &clockevent_xilinx_timer); clockevent_xilinx_timer.max_delta_ticks = (u32 )~0 ; - clockevent_xilinx_timer.min_delta_ns = -clockevent_delta2ns (1 , &clockevent_xilinx_timer); clockevent_xilinx_timer.min_delta_ticks =1 ; clockevent_xilinx_timer.cpumask =cpumask_of (0 );clockevents_register_device (&clockevent_xilinx_timer);
arch/mips/alchemy/common/time.c +0 -2
@@ -137,9 +137,7 @@ static int __init alchemy_time_init(unsigned int m2int) cd->shift =32 ; cd->mult =div_sc (32768 ,NSEC_PER_SEC , cd->shift); - cd->max_delta_ns =clockevent_delta2ns (0xffffffff , cd); cd->max_delta_ticks =0xffffffff ; - cd->min_delta_ns =clockevent_delta2ns (9 , cd); cd->min_delta_ticks =9 ;/* ~0.28ms */ clockevents_register_device (cd);setup_irq (m2int, &au1x_rtcmatch2_irqaction);
arch/mips/jz4740/time.c +0 -2
@@ -144,9 +144,7 @@ void __init plat_time_init(void) jz4740_jiffies_per_tick =DIV_ROUND_CLOSEST (clk_rate,HZ );clockevent_set_clock (&jz4740_clockevent, clk_rate); - jz4740_clockevent.min_delta_ns =clockevent_delta2ns (100 , &jz4740_clockevent); jz4740_clockevent.min_delta_ticks =100 ; - jz4740_clockevent.max_delta_ns =clockevent_delta2ns (0xffff , &jz4740_clockevent); jz4740_clockevent.max_delta_ticks =0xffff ; jz4740_clockevent.cpumask =cpumask_of (0 );
arch/mips/kernel/cevt-bcm1480.c +0 -2
@@ -122,9 +122,7 @@ void sb1480_clockevent_init(void) cd->features =CLOCK_EVT_FEAT_PERIODIC |CLOCK_EVT_FEAT_ONESHOT ;clockevent_set_clock (cd,V_SCD_TIMER_FREQ ); - cd->max_delta_ns =clockevent_delta2ns (0x7fffff , cd); cd->max_delta_ticks =0x7fffff ; - cd->min_delta_ns =clockevent_delta2ns (2 , cd); cd->min_delta_ticks =2 ; cd->rating =200 ; cd->irq = irq;
arch/mips/kernel/cevt-ds1287.c +0 -2
@@ -127,9 +127,7 @@ int __init ds1287_clockevent_init(int irq) cd->rating =100 ; cd->irq = irq;clockevent_set_clock (cd,32768 ); - cd->max_delta_ns =clockevent_delta2ns (0x7fffffff , cd); cd->max_delta_ticks =0x7fffffff ; - cd->min_delta_ns =clockevent_delta2ns (0x300 , cd); cd->min_delta_ticks =0x300 ; cd->cpumask =cpumask_of (0 );
arch/mips/kernel/cevt-gt641xx.c +0 -2
@@ -151,9 +151,7 @@ static int __init gt641xx_timer0_clockevent_init(void) cd = >641xx_timer0_clockevent; cd->rating =200 + gt641xx_base_clock /10000000 ;clockevent_set_clock (cd, gt641xx_base_clock); - cd->max_delta_ns =clockevent_delta2ns (0x7fffffff , cd); cd->max_delta_ticks =0x7fffffff ; - cd->min_delta_ns =clockevent_delta2ns (0x300 , cd); cd->min_delta_ticks =0x300 ; cd->cpumask =cpumask_of (0 );
arch/mips/kernel/cevt-sb1250.c +0 -2
@@ -122,9 +122,7 @@ void sb1250_clockevent_init(void) cd->features =CLOCK_EVT_FEAT_PERIODIC |CLOCK_EVT_FEAT_ONESHOT ;clockevent_set_clock (cd,V_SCD_TIMER_FREQ ); - cd->max_delta_ns =clockevent_delta2ns (0x7fffff , cd); cd->max_delta_ticks =0x7fffff ; - cd->min_delta_ns =clockevent_delta2ns (2 , cd); cd->min_delta_ticks =2 ; cd->rating =200 ; cd->irq = irq;
arch/mips/kernel/cevt-txx9.c +0 -3
@@ -194,10 +194,7 @@ void __init txx9_clockevent_init(unsigned long baseaddr, int irq, txx9_clock_event_device.tmrptr = tmrptr;clockevent_set_clock (cd,TIMER_CLK (imbusclk)); - cd->max_delta_ns = -clockevent_delta2ns (0xffffffff >> (32 -TXX9_TIMER_BITS ), cd); cd->max_delta_ticks =0xffffffff >> (32 -TXX9_TIMER_BITS ); - cd->min_delta_ns =clockevent_delta2ns (0xf , cd); cd->min_delta_ticks =0xf ; cd->irq = irq; cd->cpumask = cpumask_of(0),
arch/mips/loongson32/common/time.c +0 -2
@@ -198,9 +198,7 @@ static void __init ls1x_time_init(void)ls1x_pwmtimer_init ();clockevent_set_clock (cd, mips_hpt_frequency); - cd->max_delta_ns =clockevent_delta2ns (0xffffff , cd); cd->max_delta_ticks =0xffffff ; - cd->min_delta_ns =clockevent_delta2ns (0x000300 , cd); cd->min_delta_ticks =0x000300 ; cd->cpumask =cpumask_of (smp_processor_id ());clockevents_register_device (cd);
arch/mips/loongson64/common/cs5536/cs5536_mfgpt.c +0 -2
@@ -122,9 +122,7 @@ void __init setup_mfgpt0_timer(void) cd->cpumask =cpumask_of (cpu);clockevent_set_clock (cd,MFGPT_TICK_RATE ); - cd->max_delta_ns =clockevent_delta2ns (0xffff , cd); cd->max_delta_ticks =0xffff ; - cd->min_delta_ns =clockevent_delta2ns (0xf , cd); cd->min_delta_ticks =0xf ;/* Enable MFGPT0 Comparator 2 Output to the Interrupt Mapper */
arch/mips/loongson64/loongson-3/hpet.c +0 -2
@@ -240,9 +240,7 @@ void __init setup_hpet_timer(void) cd->irq =HPET_T0_IRQ ; cd->cpumask =cpumask_of (cpu);clockevent_set_clock (cd,HPET_FREQ ); - cd->max_delta_ns =clockevent_delta2ns (0x7fffffff , cd); cd->max_delta_ticks =0x7fffffff ; - cd->min_delta_ns =clockevent_delta2ns (HPET_MIN_PROG_DELTA , cd); cd->min_delta_ticks =HPET_MIN_PROG_DELTA ;clockevents_register_device (cd);
arch/mips/ralink/cevt-rt3352.c +0 -2
@@ -128,9 +128,7 @@ static int __init ralink_systick_init(struct device_node *np) systick_irqaction.name = np->name; systick.dev.name = np->name;clockevents_calc_mult_shift (&systick.dev,SYSTICK_FREQ ,60 ); - systick.dev.max_delta_ns =clockevent_delta2ns (0x7fff , &systick.dev); systick.dev.max_delta_ticks =0x7fff ; - systick.dev.min_delta_ns =clockevent_delta2ns (0x3 , &systick.dev); systick.dev.min_delta_ticks =0x3 ; systick.dev.irq =irq_of_parse_and_map (np,0 );if (!systick.dev.irq) {
arch/mips/sgi-ip27/ip27-timer.c +0 -2
@@ -112,9 +112,7 @@ void hub_rt_clock_event_init(void) cd->name = name; cd->features =CLOCK_EVT_FEAT_ONESHOT ;clockevent_set_clock (cd,CYCLES_PER_SEC ); - cd->max_delta_ns =clockevent_delta2ns (0xfffffffffffff , cd); cd->max_delta_ticks =0xfffffffffffff ; - cd->min_delta_ns =clockevent_delta2ns (0x300 , cd); cd->min_delta_ticks =0x300 ; cd->rating =200 ; cd->irq = irq;
arch/mn10300/kernel/cevt-mn10300.c +0 -2
@@ -97,9 +97,7 @@ int __init init_clockevents(void)clockevents_calc_mult_shift (cd,MN10300_JCCLK ,1 );/* Calculate the min / max delta */ - cd->max_delta_ns =clockevent_delta2ns (TMJCBR_MAX , cd); cd->max_delta_ticks =TMJCBR_MAX ; - cd->min_delta_ns =clockevent_delta2ns (100 , cd); cd->min_delta_ticks =100 ; cd->rating =200 ;
arch/powerpc/kernel/time.c +0 -4
@@ -970,11 +970,7 @@ static void __init init_decrementer_clockevent(void)clockevents_calc_mult_shift (&decrementer_clockevent, ppc_tb_freq,4 ); - decrementer_clockevent.max_delta_ns = -clockevent_delta2ns (decrementer_max, &decrementer_clockevent); decrementer_clockevent.max_delta_ticks = decrementer_max; - decrementer_clockevent.min_delta_ns = -clockevent_delta2ns (2 , &decrementer_clockevent); decrementer_clockevent.min_delta_ticks =2 ;register_decrementer_clockevent (cpu);
arch/s390/kernel/time.c +0 -2
@@ -165,9 +165,7 @@ void init_cpu_timer(void) cd->features =CLOCK_EVT_FEAT_ONESHOT ; cd->mult =16777 ; cd->shift =12 ; - cd->min_delta_ns =1 ; cd->min_delta_ticks =1 ; - cd->max_delta_ns =LONG_MAX ; cd->max_delta_ticks =ULONG_MAX ; cd->rating =400 ; cd->cpumask =cpumask_of (cpu);
arch/score/kernel/time.c +0 -4
@@ -79,11 +79,7 @@ void __init time_init(void)/* setup COMPARE clockevent */ score_clockevent.mult =div_sc (SYSTEM_CLOCK ,NSEC_PER_SEC , score_clockevent.shift); - score_clockevent.max_delta_ns =clockevent_delta2ns ((u32 )~0 , - &score_clockevent); score_clockevent.max_delta_ticks = (u32 )~0 ; - score_clockevent.min_delta_ns =clockevent_delta2ns (50 , - &score_clockevent) +1 ; score_clockevent.min_delta_ticks =50 ; score_clockevent.cpumask =cpumask_of (0 );clockevents_register_device (&score_clockevent);
arch/sparc/kernel/time_32.c +0 -2
@@ -227,9 +227,7 @@ void register_percpu_ce(int cpu) ce->shift =32 ; ce->mult =div_sc (sparc_config.clock_rate,NSEC_PER_SEC , ce->shift); - ce->max_delta_ns =clockevent_delta2ns (sparc_config.clock_rate, ce); ce->max_delta_ticks = (unsigned long )sparc_config.clock_rate; - ce->min_delta_ns =clockevent_delta2ns (100 , ce); ce->min_delta_ticks =100 ;clockevents_register_device (ce);
arch/sparc/kernel/time_64.c +0 -4
@@ -794,11 +794,7 @@ void __init time_init(void) sparc64_clockevent.name = tick_ops->name;clockevents_calc_mult_shift (&sparc64_clockevent, freq,4 ); - sparc64_clockevent.max_delta_ns = -clockevent_delta2ns (0x7fffffffffffffffUL , &sparc64_clockevent); sparc64_clockevent.max_delta_ticks =0x7fffffffffffffffUL ; - sparc64_clockevent.min_delta_ns = -clockevent_delta2ns (0xF , &sparc64_clockevent); sparc64_clockevent.min_delta_ticks =0xF ; printk("clockevent: mult[%x] shift[%d]\n",
arch/tile/kernel/time.c +0 -2
@@ -153,7 +153,6 @@ static int tile_timer_shutdown(struct clock_event_device *evt) static DEFINE_PER_CPU(struct clock_event_device, tile_timer) = { .name ="tile timer" , .features =CLOCK_EVT_FEAT_ONESHOT , - .min_delta_ns =1000 , .min_delta_ticks =1 , .rating =100 , .irq =-1 ,
@@ -169,7 +168,6 @@ void setup_tile_timer(void)/* Fill in fields that are speed-specific. */ clockevents_calc_mult_shift (evt, cycles_per_sec,TILE_MINSEC ); - evt->max_delta_ns =clockevent_delta2ns (MAX_TICK , evt); evt->max_delta_ticks =MAX_TICK ;/* Mark as being for this cpu only. */
arch/um/kernel/time.c +0 -2
@@ -64,9 +64,7 @@ static struct clock_event_device timer_clockevent = { .set_state_oneshot = itimer_one_shot, .set_next_event = itimer_next_event, .shift = 0, - .max_delta_ns = 0xffffffff, .max_delta_ticks = 0xffffffff, - .min_delta_ns = TIMER_MIN_DELTA, .min_delta_ticks = TIMER_MIN_DELTA, // microsecond resolution should be enough for anyone, same as 640K RAM .irq = 0, .mult = 1,
arch/unicore32/kernel/time.c +0 -4
@@ -89,11 +89,7 @@ void __init time_init(void)clockevents_calc_mult_shift (&ckevt_puv3_osmr0,CLOCK_TICK_RATE ,5 ); - ckevt_puv3_osmr0.max_delta_ns = -clockevent_delta2ns (0x7fffffff , &ckevt_puv3_osmr0); ckevt_puv3_osmr0.max_delta_ticks =0x7fffffff ; - ckevt_puv3_osmr0.min_delta_ns = -clockevent_delta2ns (MIN_OSCR_DELTA *2 , &ckevt_puv3_osmr0) +1 ; ckevt_puv3_osmr0.min_delta_ticks =MIN_OSCR_DELTA *2 ; ckevt_puv3_osmr0.cpumask =cpumask_of (0 );
arch/x86/kernel/apic/apic.c +0 -8
@@ -727,11 +727,7 @@ static int __init calibrate_APIC_clock(void) lapic_timer_frequency); lapic_clockevent.mult =div_sc (lapic_timer_frequency/APIC_DIVISOR ,TICK_NSEC , lapic_clockevent.shift); - lapic_clockevent.max_delta_ns = -clockevent_delta2ns (0x7FFFFF , &lapic_clockevent); lapic_clockevent.max_delta_ticks =0x7FFFFF ; - lapic_clockevent.min_delta_ns = -clockevent_delta2ns (0xF , &lapic_clockevent); lapic_clockevent.min_delta_ticks =0xF ; lapic_clockevent.features &= ~CLOCK_EVT_FEAT_DUMMY ;return 0 ;
@@ -776,11 +772,7 @@ static int __init calibrate_APIC_clock(void)/* Calculate the scaled math multiplication factor */ lapic_clockevent.mult =div_sc (delta,TICK_NSEC *LAPIC_CAL_LOOPS , lapic_clockevent.shift); - lapic_clockevent.max_delta_ns = -clockevent_delta2ns (0x7FFFFFFF , &lapic_clockevent); lapic_clockevent.max_delta_ticks =0x7FFFFFFF ; - lapic_clockevent.min_delta_ns = -clockevent_delta2ns (0xF , &lapic_clockevent); lapic_clockevent.min_delta_ticks =0xF ; lapic_timer_frequency = (delta *APIC_DIVISOR ) /LAPIC_CAL_LOOPS ;
arch/x86/lguest/boot.c +0 -2
@@ -1007,9 +1007,7 @@ static struct clock_event_device lguest_clockevent = { .rating = INT_MAX, .mult = 1, .shift = 0, - .min_delta_ns = LG_CLOCK_MIN_DELTA, .min_delta_ticks = LG_CLOCK_MIN_DELTA, - .max_delta_ns = LG_CLOCK_MAX_DELTA, .max_delta_ticks = LG_CLOCK_MAX_DELTA, };
arch/x86/platform/uv/uv_time.c +0 -4
@@ -388,12 +388,8 @@ static __init int uv_rtc_setup_clock(void) clock_event_device_uv.mult =div_sc (sn_rtc_cycles_per_second,NSEC_PER_SEC , clock_event_device_uv.shift); - clock_event_device_uv.min_delta_ns =NSEC_PER_SEC / - sn_rtc_cycles_per_second; clock_event_device_uv.min_delta_ticks =1 ; - clock_event_device_uv.max_delta_ns = clocksource_uv.mask * - (NSEC_PER_SEC / sn_rtc_cycles_per_second); clock_event_device_uv.max_delta_ticks = clocksource_uv.mask; rc =schedule_on_each_cpu (uv_rtc_register_clockevents);
arch/x86/xen/time.c +0 -4
@@ -208,9 +208,7 @@ static const struct clock_event_device xen_timerop_clockevent = { .name = "xen", .features = CLOCK_EVT_FEAT_ONESHOT, - .max_delta_ns = 0xffffffff, .max_delta_ticks = 0xffffffff, - .min_delta_ns = TIMER_SLOP, .min_delta_ticks = TIMER_SLOP, .mult = 1,
@@ -269,9 +267,7 @@ static const struct clock_event_device xen_vcpuop_clockevent = { .name = "xen", .features = CLOCK_EVT_FEAT_ONESHOT, - .max_delta_ns = 0xffffffff, .max_delta_ticks = 0xffffffff, - .min_delta_ns = TIMER_SLOP, .min_delta_ticks = TIMER_SLOP, .mult = 1,
drivers/clocksource/dw_apb_timer.c +0 -3
@@ -255,10 +255,7 @@ dw_apb_clockevent_init(int cpu, const char *name, unsigned rating, dw_ced->timer.freq = freq;clockevents_calc_mult_shift (&dw_ced->ced, freq,APBT_MIN_PERIOD ); - dw_ced->ced.max_delta_ns =clockevent_delta2ns (0x7fffffff , - &dw_ced->ced); dw_ced->ced.max_delta_ticks =0x7fffffff ; - dw_ced->ced.min_delta_ns =clockevent_delta2ns (5000 , &dw_ced->ced); dw_ced->ced.min_delta_ticks =5000 ; dw_ced->ced.cpumask =cpumask_of (cpu); dw_ced->ced.features = CLOCK_EVT_FEAT_PERIODIC |
drivers/clocksource/metag_generic.c +0 -2
@@ -113,9 +113,7 @@ static int arch_timer_starting_cpu(unsigned int cpu) clk->set_next_event = metag_timer_set_next_event, clk->mult =div_sc (hwtimer_freq,NSEC_PER_SEC , clk->shift); - clk->max_delta_ns =clockevent_delta2ns (0x7fffffff , clk); clk->max_delta_ticks =0x7fffffff ; - clk->min_delta_ns =clockevent_delta2ns (0xf , clk); clk->min_delta_ticks =0xf ; clk->cpumask =cpumask_of (cpu);
drivers/clocksource/numachip.c +0 -2
@@ -50,9 +50,7 @@ static struct clock_event_device numachip2_clockevent = { .features = CLOCK_EVT_FEAT_ONESHOT, .mult = 1, .shift = 0, - .min_delta_ns = 1250, .min_delta_ticks = 1250, - .max_delta_ns = LONG_MAX, .max_delta_ticks = LONG_MAX, };
drivers/clocksource/sh_cmt.c +0 -2
@@ -813,9 +813,7 @@ static int sh_cmt_register_clockevent(struct sh_cmt_channel *ch,/* TODO: calculate good shift from rate and counter bit width */ ced->shift =32 ; ced->mult =div_sc (ch->cmt->rate,NSEC_PER_SEC , ced->shift); - ced->max_delta_ns =clockevent_delta2ns (ch->max_match_value, ced); ced->max_delta_ticks = ch->max_match_value; - ced->min_delta_ns =clockevent_delta2ns (0x1f , ced); ced->min_delta_ticks =0x1f ; dev_info(&ch->cmt->pdev->dev, "ch%u: used for clock events\n",
drivers/clocksource/timer-atlas7.c +0 -2
@@ -191,9 +191,7 @@ static int sirfsoc_local_timer_starting_cpu(unsigned int cpu) ce->tick_resume = sirfsoc_timer_shutdown; ce->set_next_event = sirfsoc_timer_set_next_event;clockevents_calc_mult_shift (ce, atlas7_timer_rate,60 ); - ce->max_delta_ns =clockevent_delta2ns (-2 , ce); ce->max_delta_ticks = (unsigned long )-2 ; - ce->min_delta_ns =clockevent_delta2ns (2 , ce); ce->min_delta_ticks =2 ; ce->cpumask =cpumask_of (cpu);
kernel/time/tick-broadcast-hrtimer.c +0 -2
@@ -83,8 +83,6 @@ static struct clock_event_device ce_broadcast_hrtimer = { CLOCK_EVT_FEAT_HRTIMER, .rating = 0, .bound_on = -1, - .min_delta_ns = 1, - .max_delta_ns = KTIME_MAX, .min_delta_ticks = 1, .max_delta_ticks = ULONG_MAX, .mult = 1,
Signature · 1 line
2.9.2
RFC v4 13/22 clockevents: check a programmed delta's bounds in terms of cycles
In reply to Nicolai StangeCurrently, clockevents_program_event()
- compares a given delta against ->min_delta_ns and ->max_delta_ns
- and transforms the given delta value from ns to device cycles via ->mult.
Future changes introducing NTP time awareness into the clockevents core would possibly need to update ->mult, ->min_delta_ns and ->max_delta_ns from a different CPU than clockevents_program_event() runs on: the ->*_delta_ns values depend on ->mult. In order to guarantee atomicity between these updates, a seqlock would have to be introduced and acquired for reading in clockevents_program_event().
In order to avoid this, do the bounds checking in terms of the always invariant ->min_delta_ticks and ->max_delta_ticks values.
The transition from ->max_delta_ns to ->max_delta_ticks is straight forward since they are 1:1.
For ->min_delta_ns, the situation is slightly different as it can get larger than its initial value derived from ->min_delta_ticks for two reason:
- ->min_delta_ns is enforced to be larger than 1us at initialization
- and it can grow over time with CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST, c.f. clockevents_increase_min_delta().
Introduce ->min_delta_ticks_adjusted always resembling the current value of ->min_delta_ns. The original ->min_delta_ticks must be kept for reconfiguration.
The invariant ->min_delta_ticks <= ->min_delta_ticks_adjusted will always be guaranteed to hold. This will allow for non-atomic updates of ->mult and ->min_delta_ticks_adjusted -- as long as we stay within a device's allowed bounds, we don't care for small deviations.
Make clockevents_program_event() use ->min_delta_ticks_adjusted and ->max_delta_ticks for the bounds enforcement on the delta value.
Finally, slightly reorder the members of struct clock_event_device in order to keep the now often used ones close together.
Also, since ->max_delta_ticks is being actively used now, remove the assertion that it is "stored for reconfiguration" from its docstring.
--- include/linux/clockchips.h | 14 ++++++++------ kernel/time/clockevents.c | 15 ++++++++++++--- 2 files changed, 20 insertions(+), 9 deletions(-)
include/linux/clockchips.h +8 -6
@@ -73,8 +73,8 @@ enum clock_event_state {* @set_next_event: set next event function using a clocksource delta * @set_next_ktime: set next event function using a direct ktime value * @next_event: local storage for the next event in oneshot mode -* @max_delta_ns: maximum delta value in ns -* @min_delta_ns: minimum delta value in ns +* @max_delta_ticks: maximum delta value in ticks +* @min_delta_ticks_adjusted: minimum delta value, increased as needed * @mult: nanosecond to cycles multiplier * @shift: nanoseconds to cycles divisor (power of two) * @state_use_accessors:current state of the device, assigned by the core code
@@ -87,7 +87,8 @@ enum clock_event_state {* @tick_resume: resume clkevt device * @broadcast: function to broadcast events * @min_delta_ticks: minimum delta value in ticks stored for reconfiguration -* @max_delta_ticks: maximum delta value in ticks stored for reconfiguration +* @max_delta_ns: maximum delta value in ns +* @min_delta_ns: minimum delta value in ns * @name: ptr to clock event name * @rating: variable to rate clock event devices * @irq: IRQ number (only for non CPU local devices)
@@ -101,8 +102,8 @@ struct clock_event_device {int (*set_next_event)(unsigned long evt,struct clock_event_device *);int (*set_next_ktime)(ktime_t expires,struct clock_event_device *);ktime_t next_event; -u64 max_delta_ns; -u64 min_delta_ns; +unsigned long max_delta_ticks; +unsigned long min_delta_ticks_adjusted;u32 mult;u32 shift;enum clock_event_state state_use_accessors;
@@ -119,7 +120,8 @@ struct clock_event_device {void (*suspend)(struct clock_event_device *);void (*resume)(struct clock_event_device *);unsigned long min_delta_ticks; -unsigned long max_delta_ticks; +u64 max_delta_ns; +u64 min_delta_ns;const char *name;int rating;
kernel/time/clockevents.c +12 -3
@@ -225,6 +225,11 @@ static int clockevents_increase_min_delta(struct clock_event_device *dev)if (dev->min_delta_ns >MIN_DELTA_LIMIT ) dev->min_delta_ns =MIN_DELTA_LIMIT ; + dev->min_delta_ticks_adjusted = (unsigned long )((dev->min_delta_ns * + dev->mult) >> dev->shift); + dev->min_delta_ticks_adjusted =max (dev->min_delta_ticks_adjusted, + dev->min_delta_ticks); + printk_deferred(KERN_WARNING "CE: %s increased min_delta_ns to %llu nsec\n", dev->name ? dev->name : "?",
@@ -332,10 +337,10 @@ int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,if (delta <=0 )return force ?clockevents_program_min_delta (dev) : -ETIME ; - delta =min (delta, (int64_t ) dev->max_delta_ns); - delta =max (delta, (int64_t ) dev->min_delta_ns); - clc = ((unsigned long long ) delta * dev->mult) >> dev->shift; + clc = min_t(unsigned long, clc, dev->max_delta_ticks); + clc = max_t(unsigned long, clc, dev->min_delta_ticks_adjusted); + rc = dev->set_next_event ((unsigned long ) clc, dev);return (rc && force) ?clockevents_program_min_delta (dev) : rc;
@@ -453,6 +458,10 @@ static void __clockevents_update_bounds(struct clock_event_device *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); + dev->min_delta_ticks_adjusted = (unsigned long )((dev->min_delta_ns * + dev->mult) >> dev->shift); + dev->min_delta_ticks_adjusted =max (dev->min_delta_ticks_adjusted, + dev->min_delta_ticks); }/**
Signature · 1 line
2.9.2
Re: [RFC v4 13/22] clockevents: check a programmed delta's bounds in terms of cycles
Quoted 4 lines · Nicolai Stange
kernel/time/clockevents.c +0
-0@@ -332,10 +337,10 @@ int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,if (delta <=0 )return force ?clockevents_program_min_delta (dev) : -ETIME ;
kernel/time/clockevents.c int clockevents_program_event(struct clock_event_device *dev, ktime_t expires, · line 346
Quoted 10 lines
kernel/time/clockevents.c +3
-3- delta =min (delta, (int64_t ) dev->max_delta_ns); - delta =max (delta, (int64_t ) dev->min_delta_ns); - clc = ((unsigned long long ) delta * dev->mult) >> dev->shift; + clc = min_t(unsigned long, clc, dev->max_delta_ticks); + clc = max_t(unsigned long, clc, dev->min_delta_ticks_adjusted); + rc = dev->set_next_event ((unsigned long ) clc, dev);return (rc && force) ?clockevents_program_min_delta (dev) : rc;
This is broken :(
I failed to recognize that ->max_delta_ns serves not only one, but three purposes actually:
1. It prevents the ced to get programmed with too large values. Still works with this patch.
2. It prevents the multiplication by dev->mult from overflowing 64 bits, i.e. it clamps the input delta to a range valid for the given ->mult. Ouch.
3. On 32 bit archs, it prevents the cast of clc to unsigned long from overflowing. Ouch here as well.
The 3.) can be restored by doing
clc = min_t(unsigned long long, clc, dev->max_delta_ticks);
rather than min_t(unsigned long, ...)
because dev->max_delta_ticks is of type unsigned long and thus, <= ULONG_MAX.
Unfortunately, fixing up 2.) is not so straight forward: I'll certainly have to resort to ->max_delta_ns again. But then, there will be the issue with non-atomic updates from timekeeping -- at least if ->max_delta_ns continues to represent ->max_delta_ticks as it did before.
In order to get rid of the requirement to update ->max_delta_ns whenever the ->mult changes, would it be Ok to decouple ->max_delta_ns from ->max_delta_ticks by
a. setting
dev->max_delta_ns = (1 << (64 - ilog2(dev->mult))) - 1
once and for all at device registration (and from clockevents_update_freq()),
b. and introducing an *additional* comparison
delta = min(delta, (int64_t) dev->max_delta_ns); right before the multiplication in clockevents_program_event()?
In this setting, ->max_delta_ns would be a function of the original ->mult only -- more precisely, of ilog2(dev->mult).
Altogether, we'd have
delta = min(delta, (int64_t) dev->max_delta_ns); clc = ((unsigned long long) delta * dev->mult) >> dev->shift; clc = min_t(unsigned long long, clc, dev->max_delta_ticks); clc = max_t(unsigned long long, clc, dev->min_delta_ticks_adjusted); rc = dev->set_next_event((unsigned long) clc, dev);
in clockevents_program_event() then.
So, purposes 1.) and 3.) would get served by the second min() while the first one would make sure that the multiplication will never overflow.
The downside would be the additional comparison + conditional move in the ced programming path. The ->max_delta_ns and ->max_delta_ticks can both be moved to struct clock_event_device's first cacheline simultaneously without affecting any of its remaining hot members though (on 64 bit archs with a cacheline size of 64 bytes).
Now, to quote your objections to [22/22] ("timekeeping: inform clockevents about freq adjustments"):
Quoted 4 lines
What makes sure that the resulting shift/mult pair is still valid after this adjustment? The non adjusted mult/shift pair might be right at the border of potential overflows and the adjustment might just put it over the edge.... We need at least sanity checks here.
The updated ->mult_adjusted could get restricted to never grow beyond
(1 << fls(dev->mult)) - 1
where dev->mult is the never changing, non-adjusted mult value. That is, the mult adjustment would simply stop at the point where it could possibly introduce overflows for some deltas smaller than the now fixed ->max_delta_ns.
I have to admit that checking both, ->max_delta_ticks and ->max_delta_ns from clockevents_program_event() is a little bit messy. As is the cut-off point for the mult adjustments...
Maybe I should just try to schedule the necessary updates from timekeeping on each CPU instead? If this worked out, I could probably recalculate appropriate values of ->*_delta_ns and store these racelessly along with the adjusted mult while not touching clockevents_program_event() at all. That is, I would schedule something similar to clockevents_update_freq() on each CPU.
Thanks,
Nicolai Stange
Re: [RFC v4 13/22] clockevents: check a programmed delta's bounds in terms of cycles
Quoted 93 lines · Nicolai Stange
Quoted 4 lines · Nicolai Stange
kernel/time/clockevents.c +0
-0@@ -332,10 +337,10 @@ int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,if (delta <=0 )return force ?clockevents_program_min_delta (dev) : -ETIME ;kernel/time/clockevents.c int clockevents_program_event(struct clock_event_device *dev, ktime_t expires, · line 346
Quoted 10 lines
kernel/time/clockevents.c +3
-3- delta =min (delta, (int64_t ) dev->max_delta_ns); - delta =max (delta, (int64_t ) dev->min_delta_ns); - clc = ((unsigned long long ) delta * dev->mult) >> dev->shift; + clc = min_t(unsigned long, clc, dev->max_delta_ticks); + clc = max_t(unsigned long, clc, dev->min_delta_ticks_adjusted); + rc = dev->set_next_event ((unsigned long ) clc, dev);return (rc && force) ?clockevents_program_min_delta (dev) : rc;This is broken :(
I failed to recognize that ->max_delta_ns serves not only one, but three purposes actually:
1. It prevents the ced to get programmed with too large values. Still works with this patch.
2. It prevents the multiplication bydev->multfrom overflowing 64 bits, i.e. it clamps the input delta to a range valid for the given ->mult. Ouch.
3. On 32 bit archs, it prevents the cast of clc to unsigned long from overflowing. Ouch here as well.The 3.) can be restored by doing
clc =min_t(unsigned long long, clc, dev->max_delta_ticks);
rather thanmin_t(unsigned long, ...)
becausedev->max_delta_ticksis of type unsigned long and thus, <=ULONG_MAX.Unfortunately, fixing up 2.) is not so straight forward: I'll certainly have to resort to ->max_delta_ns again. But then, there will be the issue with non-atomic updates from timekeeping -- at least if ->max_delta_ns continues to represent ->max_delta_ticks as it did before.
In order to get rid of the requirement to update ->max_delta_ns whenever the ->mult changes, would it be Ok to decouple ->max_delta_ns from ->max_delta_ticks by
a. settingdev->max_delta_ns = (1 << (64 - ilog2(dev->mult))) - 1 once and for all at device registration (and from clockevents_update_freq()), b. and introducing an *additional* comparison delta = min(delta, (int64_t) dev->max_delta_ns);right before the multiplication in
clockevents_program_event()?In this setting, ->max_delta_ns would be a function of the original ->mult only -- more precisely, of
ilog2(dev->mult).Altogether, we'd have
delta = min(delta, (int64_t) dev->max_delta_ns); clc = ((unsigned long long) delta * dev->mult) >> dev->shift; clc = min_t(unsigned long long, clc, dev->max_delta_ticks); clc = max_t(unsigned long long, clc, dev->min_delta_ticks_adjusted); rc = dev->set_next_event((unsigned long) clc, dev);in
clockevents_program_event()then.So, purposes 1.) and 3.) would get served by the second
min()while the first one would make sure that the multiplication will never overflow.The downside would be the additional comparison + conditional move in the ced programming path. The ->max_delta_ns and ->max_delta_ticks can both be moved to struct clock_event_device's first cacheline simultaneously without affecting any of its remaining hot members though (on 64 bit archs with a cacheline size of 64 bytes).
Now, to quote your objections to [22/22] ("timekeeping: inform clockevents about freq adjustments"):
Quoted 4 lines
What makes sure that the resulting shift/mult pair is still valid after this adjustment? The non adjusted mult/shift pair might be right at the border of potential overflows and the adjustment might just put it over the edge.... We need at least sanity checks here.
The updated ->mult_adjusted could get restricted to never grow beyond
(1 <<fls(dev->mult)) - 1
wheredev->multis the never changing, non-adjusted mult value. That is, the mult adjustment would simply stop at the point where it could possibly introduce overflows for some deltas smaller than the now fixed ->max_delta_ns.
Quoted 3 lines
I have to admit that checking both, ->max_delta_ticks and ->max_delta_ns from
clockevents_program_event()is a little bit messy. As is the cut-off point for the mult adjustments...
I thought a little bit more about this: having that cut-off point for the adjusted mult is probably the right thing to do. Reasoning: In order to avoid the overflows, the sum
ilog2(max_delta_ns) + ilog2(mult_adjusted)
should be kept bounded from above by some fixed value (this is how clockevents_config() calculates the proper ->shift value).
Now, if mult_adjusted crossed the cut-off point,
i.e. ilog2(mult_adjusted) increased by one, then I would have no choice other than setting ->max_delta_ns >>= 1.
Given that the whole purpose of this series is to avoid too early timer interrupts for large deltas, this would be a strange thing to do.
Quoted 8 lines
Maybe I should just try to schedule the necessary updates from timekeeping on each CPU instead? If this worked out, I could probably recalculate appropriate values of ->*_delta_ns and store these racelessly along with the adjusted mult while not touching
clockevents_program_event()at all. That is, I would schedule something similar toclockevents_update_freq()on each CPU.
Since the "cut-off point of mult_adjusted is messy" argument is gone now, I won't do this. So, if you don't object in the meanwhile, v5 will have both, ->max_delta_ns *and* ->max_delta_ticks. This trades the reduced complexity of not having to schedule the update everywhere against an extra min() in the ced programming path.
Thanks,
Nicolai Stange
RFC v4 14/22 clockevents: clockevents_program_event(): turn clc into unsigned long
In reply to Nicolai StangeRight now, being of type unsigned long long, the clc local variable in clockevents_program_event() is unnecessarily wide as it gets cast to unsigned long anyway.
Degrade clc's type to unsigned long and remove the now superfluous casts.
--- kernel/time/clockevents.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
kernel/time/clockevents.c +6 -5
@@ -311,7 +311,7 @@ static int clockevents_program_min_delta(struct clock_event_device *dev)int clockevents_program_event (struct clock_event_device *dev,ktime_t expires,bool force) { -unsigned long long clc; +unsigned long clc;int64_t delta;int rc;
@@ -337,11 +337,12 @@ int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,if (delta <=0 )return force ?clockevents_program_min_delta (dev) : -ETIME ; - clc = ((unsigned long long ) delta * dev->mult) >> dev->shift; - clc = min_t(unsigned long, clc, dev->max_delta_ticks); - clc = max_t(unsigned long, clc, dev->min_delta_ticks_adjusted); + clc = (unsigned long )(((unsigned long long )delta * + dev->mult) >> dev->shift); + clc =min (clc, dev->max_delta_ticks); + clc =max (clc, dev->min_delta_ticks_adjusted); - rc = dev->set_next_event ((unsigned long ) clc, dev); + rc = dev->set_next_event (clc, dev);return (rc && force) ?clockevents_program_min_delta (dev) : rc; }
Signature · 1 line
2.9.2
Re: [RFC v4 14/22] clockevents: clockevents_program_event(): turn clc into unsigned long
Quoted 3 lines · Nicolai Stange
Right now, being of type unsigned long long, the clc local variable in
clockevents_program_event()is unnecessarily wide as it gets cast to unsigned long anyway.
Congrats! You just broke 32bit systems. When delta is sufficiently large then the mult overflows....
Thanks,
tglx
Re: [RFC v4 14/22] clockevents: clockevents_program_event(): turn clc into unsigned long
Quoted 8 lines · Thomas Gleixner
Quoted 3 lines · Nicolai Stange
Right now, being of type unsigned long long, the clc local variable in
clockevents_program_event()is unnecessarily wide as it gets cast to unsigned long anyway.Congrats! You just broke 32bit systems. When delta is sufficiently large then the mult overflows....
The 32bit breakage has been introduced in [13/22] ("clockevents: check a programmed delta's bounds in terms of cycles") already -- please see my reply there.
RFC v4 15/22 clockevents: clockevents_program_min_delta(): don't set ->next_event
In reply to Nicolai StangeCurrently, clockevents_program_min_delta() sets a clockevent device's ->next_event to the point in time where the minimum delta would actually expire:
delta = dev->min_delta_ns; dev->next_event = ktime_add_ns(ktime_get(), delta);
For your reference, this is so since the initial advent of clockevents_program_min_delta() with
commit d1748302f70b ("clockevents: Make minimum delay adjustments
configurable").
clockevents_program_min_delta() is called from clockevents_program_event() only. More specifically, it is called if the latter's force argument is set and, neglecting the case of device programming failure for the moment, if the requested expiry is in the past.
On the contrary, if the expiry requested from clockevents_program_event() is in the future, but less than ->min_delta_ns behind, then
- ->next_event gets set to that expiry verbatim
- but the clockevent device gets silently programmed to fire after ->min_delta_ns only.
Thus, in the extreme cases of expires == ktime_get() and expires == ktime_get() + 1, the respective values of ->next_event would differ by ->min_delta_ns while the clockevent device would actually get programmed to fire at (almost) the same times (with force being set, of course).
While this discontinuity of ->next_event at expires == ktime_get() is not a problem by itself, the mere use of ->min_delta_ns in the event programming path hinders upcoming changes making the clockevent core NTP correction aware: both, ->mult and ->min_delta_ns would need to get updated as well as consumed atomically and we'd rather like to avoid any locking here.
Thus, let clockevents_program_event() unconditionally set ->next_event to the expiry time actually requested by its caller, i.e. don't set ->next_event from clockevents_program_min_delta().
A few notes on why this change is safe with the current consumers of ->next_event:
1.
Note that a clockevents_program_event() with a requested expiry in the past and force being set basically means: "fire ASAP". Now, consider this so programmed event getting handed once again to clockevents_program_event(), i.e. that a
clockevents_program_event(dev, dev->next_event, false)
as in __clockevents_update_freq() is done.
With this change applied, clockevents_program_event() would now properly detect the expiry being in the past and, due to the force argument being unset, wouldn't actually do anything.
Before this change OTOH, there would be the (very unlikely) possibility that the requested event is still somewhere in the future and clockevents_program_event() would silently delay the event expiration by another ->min_delta_ns.
2.
The periodic tick handlers on oneshot-only devices use ->next_event to calculate the followup expiry time. tick_handle_periodic() spins on reprogramming the clockevent device until some expiry in the future has been reached:
ktime_t next = dev->next_event;
...
for(;;) {
next = ktime_add(next, tick_period);
if (!clockevents_program_event(dev, next, false))
return;
...
}Thus, tick_handle_periodic() isn't affected by this change. For tick_handle_periodic_broadcast(), the situation is different since
commit 2951d5c031a3 ("tick: broadcast: Prevent livelock from event
handler")
though: a loop similar to the one from tick_handle_periodic() above got replaced by a single
ktime_t next = ktime_add(dev->next_event, tick_period); clockevents_program_event(dev, next, true);
In the case that dev->next_event + tick_period happens to be less than ktime_get() + ->min_delta_ns, without this change applied, ->next_event would get recovered to some point in the future after a single tick_handle_periodic_broadcast() event.
On the contrary, with this patch applied, it could potentially take some number of tick_handle_periodic_broadcast() events, each separated by ->min_delta_ns only, until ->next_event is able to catch up with the current ktime_get(). However, if this turns out to become a problem, the reprogramming loop in tick_handle_periodic_broadcast() can probably be restored easily.
3.
In kernel/time/tick-broadcast.c, the broadcast receiving clockevent devices' ->next_event is read multiple times in order to determine who's next or who must be pinged. These uses all continue to work. Moreover, clockevent devices getting programmed to something less than ktime_get() + ->min_delta_ns
might not be the best candidates for a transition into C3 anyway.
4.
Finally, a "sleep length" is calculated at the very end of tick_nohz_stop_sched_tick() as follows:
ts->sleep_length = ktime_sub(dev->next_event, now);
AFAICS, this can happen to be negative w/o this change applied already: in NOHZ_MODE_HIGHRES mode there can be some overdue hrtimers whose removal is blocked because tick_nohz_stop_sched_tick() gets called with interrupts disabled. Unfortunately, the only user, the menu cpuidle governor, can't cope with negative sleep lengths as it casts the return value of the tick_nohz_get_sleep_length() getter to an unsigned int. This change can very well make things worse here. A followup patch will force this ->sleep_length to >= 0.
--- kernel/time/clockevents.c | 2 -- 1 file changed, 2 deletions(-)
kernel/time/clockevents.c +0 -2
@@ -251,7 +251,6 @@ static int clockevents_program_min_delta(struct clock_event_device *dev)for (i =0 ;;) { delta = dev->min_delta_ns; - dev->next_event =ktime_add_ns (ktime_get (), delta);if (clockevent_state_shutdown (dev))return 0 ;
@@ -288,7 +287,6 @@ static int clockevents_program_min_delta(struct clock_event_device *dev)int64_t delta; delta = dev->min_delta_ns; - dev->next_event =ktime_add_ns (ktime_get (), delta);if (clockevent_state_shutdown (dev))return 0 ;
Signature · 1 line
2.9.2
RFC v4 16/22 clockevents: use ->min_delta_ticks_adjusted to program minimum delta
In reply to Nicolai StangeThe use of a clockevent device's ->min_delta_ns in the event programming path hinders upcoming changes to the clockevent core making it NTP correction aware: both, ->mult and ->min_delta_ns would need to get updated as well as consumed atomically and we'd rather like to avoid any locking here.
We already have got ->min_delta_ticks_adjusted which
- resembles the value of ->min_delta_ns
- and is guaranteed to be always >= the hardware's hard limit ->min_delta_ticks and thus, can be used w/o locking as we don't care for small deviations.
In both implementations of clockevents_program_min_delta(), don't calculate the event's deadline from ->min_delta_ns, but use its drop-in replacement ->min_delta_ticks_adjusted.
--- kernel/time/clockevents.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-)
kernel/time/clockevents.c +2 -13
@@ -245,19 +245,14 @@ static int clockevents_increase_min_delta(struct clock_event_device *dev)*/ static int clockevents_program_min_delta (struct clock_event_device *dev) { -unsigned long long clc; -int64_t delta;int i;for (i =0 ;;) { - delta = dev->min_delta_ns; -if (clockevent_state_shutdown (dev))return 0 ; dev->retries++; - clc = ((unsigned long long ) delta * dev->mult) >> dev->shift; -if (dev->set_next_event ((unsigned long ) clc, dev) ==0 ) +if (!dev->set_next_event (dev->min_delta_ticks_adjusted, dev))return 0 ;if (++i >2 ) {
@@ -283,17 +278,11 @@ static int clockevents_program_min_delta(struct clock_event_device *dev)*/ static int clockevents_program_min_delta (struct clock_event_device *dev) { -unsigned long long clc; -int64_t delta; - - delta = dev->min_delta_ns; -if (clockevent_state_shutdown (dev))return 0 ; dev->retries++; - clc = ((unsigned long long ) delta * dev->mult) >> dev->shift; -return dev->set_next_event ((unsigned long ) clc, dev); +return dev->set_next_event (dev->min_delta_ticks_incr, dev); }#endif /* CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST */
Signature · 1 line
2.9.2
Re: [RFC v4 16/22] clockevents: use ->min_delta_ticks_adjusted to program minimum delta
Quoted 7 lines
static int
clockevents_program_min_delta(struct clock_event_device *dev){kernel/time/clockevents.c +0
-5-unsigned long long clc; -int64_t delta; - - delta = dev->min_delta_ns; -
kernel/time/clockevents.c static int clockevents_program_min_delta(struct clock_event_device *dev) · line 288
Quoted 10 lines
kernel/time/clockevents.c +1
-2if (clockevent_state_shutdown (dev))return 0 ; dev->retries++; - clc = ((unsigned long long ) delta * dev->mult) >> dev->shift; - return dev->set_next_event((unsigned long) clc, dev); + return dev->set_next_event(dev->min_delta_ticks_incr, dev); }#endif /* CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST */
FYI, the kernel build test robot reported a build failure with CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=n. It shoud read as dev->min_delta_ticks_adjusted here. In order to avoid unnecessary traffic, I'll only fix this up in a v5 if you give the whole thing a go.
RFC v4 17/22 clockevents: min delta increment: calculate min_delta_ns from ticks
In reply to Nicolai StangeThe use of a clockevent device's ->min_delta_ns in the event programming path hinders upcoming changes to the clockevent core making it NTP correction aware: both, ->mult and ->min_delta_ns would need to get updated as well as consumed atomically and we'd rather like to avoid any locking here.
We already have got ->min_delta_ticks_adjusted which
- resembles the value of ->min_delta_ns
- and is guaranteed to be always >= the hardware's hard limit ->min_delta_ticks and thus, can be used w/o locking as we don't care for small deviations.
In clockevents_increase_min_delta(), don't use ->min_delta_ns but calculate it dynamically from ->min_delta_ticks_adjusted.
As clockevents_increase_min_delta() gets invoked only rarely, the additional division should not be an issue from a performance standpoint.
--- kernel/time/clockevents.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
kernel/time/clockevents.c +11 -8
@@ -209,23 +209,26 @@ int clockevents_tick_resume(struct clock_event_device *dev)*/ static int clockevents_increase_min_delta (struct clock_event_device *dev) { +u64 min_delta_ns =cev_delta2ns (dev->min_delta_ticks_adjusted, dev, + false); +/* Nothing to do if we already reached the limit */ -if (dev->min_delta_ns >=MIN_DELTA_LIMIT ) { +if (min_delta_ns >=MIN_DELTA_LIMIT ) {printk_deferred (KERN_WARNING "CE: Reprogramming failure. Giving up\n" ); dev->next_event.tv64 =KTIME_MAX ;return -ETIME ; } -if (dev->min_delta_ns <5000 ) - dev->min_delta_ns =5000 ; +if (min_delta_ns <5000 ) + min_delta_ns =5000 ;else - dev->min_delta_ns += dev->min_delta_ns >>1 ; + min_delta_ns += min_delta_ns >>1 ; -if (dev->min_delta_ns >MIN_DELTA_LIMIT ) - dev->min_delta_ns =MIN_DELTA_LIMIT ; +if (min_delta_ns >MIN_DELTA_LIMIT ) + min_delta_ns =MIN_DELTA_LIMIT ; - dev->min_delta_ticks_adjusted = (unsigned long )((dev->min_delta_ns * + dev->min_delta_ticks_adjusted = (unsigned long )((min_delta_ns * dev->mult) >> dev->shift); dev->min_delta_ticks_adjusted =max (dev->min_delta_ticks_adjusted, dev->min_delta_ticks);
@@ -233,7 +236,7 @@ static int clockevents_increase_min_delta(struct clock_event_device *dev)printk_deferred (KERN_WARNING "CE: %s increased min_delta_ns to %llu nsec\n" , dev->name ? dev->name :"?" , - (unsigned long long ) dev->min_delta_ns); + (unsigned long long ) min_delta_ns);return 0 ; }
Signature · 1 line
2.9.2
RFC v4 18/22 timer_list: print_tickdevice(): calculate ->*_delta_ns dynamically
In reply to Nicolai Stangeprint_tickdevice(), assembling the per-tick device sections in /proc/timer_list, is the last user of struct clock_event_device's ->min_delta_ns and ->max_delta_ns members.
In order to make these fully obsolete while retaining userspace ABI, calculate the displayed values of 'min_delta_ns' and 'max_delta_ns' on the fly from ->min_delta_ticks_adjusted and ->max_delta_ticks respectively.
--- kernel/time/timer_list.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
kernel/time/timer_list.c +6 -4
@@ -206,6 +206,10 @@ static void print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu) { struct clock_event_device *dev = td->evtdev; +unsigned long long min_delta_ns = +clockevent_delta2ns (dev->min_delta_ticks_adjusted, dev); +unsigned long long max_delta_ns = +clockevent_delta2ns (dev->max_delta_ticks, dev);SEQ_printf (m,"Tick Device: mode: %d\n" , td->mode);if (cpu <0 )
@@ -219,10 +223,8 @@ print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu)return ; }SEQ_printf (m,"%s\n" , dev->name); -SEQ_printf (m," max_delta_ns: %llu\n" , - (unsigned long long ) dev->max_delta_ns); -SEQ_printf (m," min_delta_ns: %llu\n" , - (unsigned long long ) dev->min_delta_ns); +SEQ_printf (m," max_delta_ns: %llu\n" , max_delta_ns); +SEQ_printf (m," min_delta_ns: %llu\n" , min_delta_ns);SEQ_printf (m," mult: %u\n" , dev->mult);SEQ_printf (m," shift: %u\n" , dev->shift);SEQ_printf (m," mode: %d\n" ,clockevent_get_state (dev));
Signature · 1 line
2.9.2
RFC v4 19/22 clockevents: purge ->min_delta_ns and ->max_delta_ns
In reply to Nicolai StangeThe struct clock_event_device ->min_delta_ns and ->max_delta_ns aren't used anymore.
Purge them.
In __clockevents_update_bounds(), shortcut the
->min_delta_ticks => ->min_delta_ns => ->min_delta_ticks_adjusted
calculation detour -- it had been made solely for the purpose of ensuring that ->min_delta_ticks_adjusted corresponds to something >= 1us.
--- include/linux/clockchips.h | 4 ---- kernel/time/clockevents.c | 13 +++++-------- 2 files changed, 5 insertions(+), 12 deletions(-)
include/linux/clockchips.h +0 -4
@@ -87,8 +87,6 @@ enum clock_event_state {* @tick_resume: resume clkevt device * @broadcast: function to broadcast events * @min_delta_ticks: minimum delta value in ticks stored for reconfiguration -* @max_delta_ns: maximum delta value in ns -* @min_delta_ns: minimum delta value in ns * @name: ptr to clock event name * @rating: variable to rate clock event devices * @irq: IRQ number (only for non CPU local devices)
@@ -120,8 +118,6 @@ struct clock_event_device {void (*suspend)(struct clock_event_device *);void (*resume)(struct clock_event_device *);unsigned long min_delta_ticks; -u64 max_delta_ns; -u64 min_delta_ns;const char *name;int rating;
kernel/time/clockevents.c +5 -8
@@ -444,15 +444,12 @@ static void __clockevents_update_bounds(struct clock_event_device *dev)return ;/* -* cev_delta2ns() never returns values less than 1us and thus, -* we'll never program any ced with anything less. +* Enforce ->min_delta_ticks_adjusted to correspond to a value +* >= 1us. */ - dev->min_delta_ns =cev_delta2ns (dev->min_delta_ticks, dev, false); - dev->max_delta_ns =cev_delta2ns (dev->max_delta_ticks, dev, true); - dev->min_delta_ticks_adjusted = (unsigned long )((dev->min_delta_ns * - dev->mult) >> dev->shift); - dev->min_delta_ticks_adjusted =max (dev->min_delta_ticks_adjusted, - dev->min_delta_ticks); + dev->min_delta_ticks_adjusted = +max (dev->min_delta_ticks, + (unsigned long )((1000ULL * dev->mult) >> dev->shift)); }/**
Signature · 1 line
2.9.2
RFC v4 20/22 clockevents: initial support for mono to raw time conversion
In reply to Nicolai StangeWith NOHZ_FULL and one single well-isolated, CPU consumptive task, one would expect approximately one clockevent interrupt per second. However, on my Intel Haswell where the monotonic clock is the TSC monotonic clock and the clockevent device is the TSC deadline device, it turns out that every second, there are two such interrupts: the first one arrives always approximately ~50us before the scheduled deadline as programmed by tick_nohz_stop_sched_tick() through the hrtimer API. The __hrtimer_run_queues() called in this interrupt detects that the queued tick_sched_timer hasn't expired yet and simply does nothing except reprogramming the clock event device to fire shortly after again.
These too early programmed deadlines are explained as follows:clockevents_program_event() programs the clockevent device to fire after
f_event * delta_t_progr
clockevent device cycles where f_event is the clockevent device's hardware frequency and delta_t_progr is the requested time interval. After that many clockevent device cycles have elapsed, the device underlying the monotonic clock, that is the monotonic raw clock has seen f_raw / f_event as many cycles.
The ktime_get() called from __hrtimer_run_queues() interprets those cycles to run at the frequency of the monotonic clock. Summarizing:
delta_t_perc = 1/f_mono * f_raw/f_event * f_event * delta_t_progr
= f_raw / f_mono * delta_t_progr
with f_mono being the monotonic clock's frequency and delta_t_perc being the elapsed time interval as perceived by __hrtimer_run_queues().
Now, f_mono is not a constant, but is dynamically adjusted in timekeeping_adjust() in order to compensate for the NTP error. With the large values of delta_t_progr of 10^9ns with NOHZ_FULL, the error made becomes significant and results in the double timer interrupts described above.
Compensate for this error by multiplying the clockevent device's f_event by f_mono/f_raw.
Namely:
- Introduce a ->mult_adjusted member to the struct clock_event_device. Its value is supposed to be equal to ->mult * f_mono/f_raw.
- Introduce the timekeeping_get_mono_mult() helper which provides the clockevent core with access to the timekeeping's current f_mono and f_raw.
- Introduce the helper __clockevents_adjust_freq() which
sets a clockevent device's ->mult_adjusted member as appropriate. It is implemented with the help of the new __clockevents_calc_adjust_freq().
- Move the __clockevents_update_bounds() functionality into the new __clockevents_adjust_freq().
- Call __clockevents_adjust_freq() at clockevent device registration time as well as at frequency updates through clockevents_update_freq().
- Use the ->mult_adjusted rather than ->mult in the ns to cycle conversion made in clockevents_program_event() as well as in the cycle to ns conversion in cev_delta2ns().
- Finally, move ->mult out of struct clock_event_device's first cacheline.
Note that future adjustments of the monotonic clock are not taken into account yet. Furthemore, this patch assumes that after a clockevent device's registration, its ->mult changes only through calls to clockevents_update_freq().
--- include/linux/clockchips.h | 6 ++- kernel/time/clockevents.c | 91 ++++++++++++++++++++++++++++++++------------- kernel/time/tick-internal.h | 1 + kernel/time/timekeeping.c | 14 +++++++ 4 files changed, 84 insertions(+), 28 deletions(-)
include/linux/clockchips.h +4 -2
@@ -75,7 +75,7 @@ enum clock_event_state {* @next_event: local storage for the next event in oneshot mode * @max_delta_ticks: maximum delta value in ticks * @min_delta_ticks_adjusted: minimum delta value, increased as needed -* @mult: nanosecond to cycles multiplier +* @mult_adjusted: adjusted multiplier compensating for NTP adjustments * @shift: nanoseconds to cycles divisor (power of two) * @state_use_accessors:current state of the device, assigned by the core code * @features: features
@@ -87,6 +87,7 @@ enum clock_event_state {* @tick_resume: resume clkevt device * @broadcast: function to broadcast events * @min_delta_ticks: minimum delta value in ticks stored for reconfiguration +* @mult: ns to cycles multiplier stored for reconfiguration * @name: ptr to clock event name * @rating: variable to rate clock event devices * @irq: IRQ number (only for non CPU local devices)
@@ -102,7 +103,7 @@ struct clock_event_device {ktime_t next_event;unsigned long max_delta_ticks;unsigned long min_delta_ticks_adjusted; -u32 mult; +u32 mult_adjusted;u32 shift;enum clock_event_state state_use_accessors;unsigned int features;
@@ -118,6 +119,7 @@ struct clock_event_device {void (*suspend)(struct clock_event_device *);void (*resume)(struct clock_event_device *);unsigned long min_delta_ticks; +u32 mult;const char *name;int rating;
kernel/time/clockevents.c +65 -26
152 lines not on this page · raw message
kernel/time/tick-internal.h +1 -0
@@ -56,6 +56,7 @@ extern int clockevents_program_event(struct clock_event_device *dev, ktime_t expires, bool force);extern void clockevents_handle_noop (struct clock_event_device *dev);extern int __clockevents_update_freq (struct clock_event_device *dev,u32 freq); +extern void timekeeping_get_mono_mult (u32 *mult_cs_mono,u32 *mult_cs_raw);extern ssize_t sysfs_get_uname (const char *buf,char *dst,size_t cnt);/* Broadcasting support */
kernel/time/timekeeping.c +14 -0
@@ -329,6 +329,20 @@ static inline s64 timekeeping_cycles_to_ns(struct tk_read_base *tkr,return timekeeping_delta_to_ns (tkr, delta); } +void timekeeping_get_mono_mult (u32 *mult_cs_mono,u32 *mult_cs_raw) +{ +unsigned int seq; +struct tk_read_base *tkr_mono = &tk_core.timekeeper.tkr_mono; + +/* The seqlock protects us from a racing change_clocksource(). */ +do { + seq =read_seqcount_begin (&tk_core.seq); + + *mult_cs_mono = tkr_mono->mult; + *mult_cs_raw = tkr_mono->clock->mult; + }while (read_seqcount_retry (&tk_core.seq, seq)); +} +/** * update_fast_timekeeper - Update the fast and NMI safe monotonic timekeeper. * @tkr: Timekeeping readout base from which we take the update
Signature · 1 line
2.9.2
RFC v4 21/22 clockevents: make setting of ->mult and ->mult_adjusted atomic
In reply to Nicolai StangeIn order to avoid races between setting a struct clock_event_device's ->mult_adjusted in clockevents_update_freq() and yet to be implemented updates triggered from the timekeeping core, the setting of ->mult and ->mult_adjusted should be made atomic.
Protect the update in clockevents_update_freq() by locking the clockevents_lock spinlock. Frequency updates are expected to be done seldomly and thus, taking this subsystem lock should not have any impact on performance.
Note that the call to tick_broadcast_update_freq() is also protected by this clockevents_lock and thus, this patch introduces the locking dependency clockevents_lock -> tick_broadcast_lock.
--- kernel/time/clockevents.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/time/clockevents.c +2 -2
@@ -592,11 +592,11 @@ int clockevents_update_freq(struct clock_event_device *dev, u32 freq)unsigned long flags;int ret; -local_irq_save (flags); +raw_spin_lock_irqsave (&clockevents_lock, flags); ret =tick_broadcast_update_freq (dev, freq);if (ret == -ENODEV ) ret =__clockevents_update_freq (dev, freq); -local_irq_restore (flags); +raw_spin_unlock_irqrestore (&clockevents_lock, flags);return ret; }
Signature · 1 line
2.9.2
RFC v4 22/22 timekeeping: inform clockevents about freq adjustments
In reply to Nicolai StangeUpon adjustments of the monotonic clock's frequencies from the timekeeping core, the clockevents devices' ->mult_adjusted should be changed accordingly, too.
Introduce clockevents_adjust_all_freqs() which traverses all registered clockevent devices and recalculates their ->mult_adjusted based on the monotonic clock's current frequency.
Call clockevents_adjust_all_freqs() from timekeeping_apply_adjustment().
--- kernel/time/clockevents.c | 25 +++++++++++++++++++++++++ kernel/time/tick-internal.h | 1 + kernel/time/timekeeping.c | 3 +++ 3 files changed, 29 insertions(+)
kernel/time/clockevents.c +25 -0
@@ -561,6 +561,31 @@ void __clockevents_adjust_freq(struct clock_event_device *dev) dev->shift)); } +void clockevents_adjust_all_freqs (u32 mult_cs_mono,u32 mult_cs_raw) +{ +u32 last_mult_raw =0 , last_mult_adjusted =0 ; +u32 mult_raw; +unsigned long flags; +struct clock_event_device *dev; + +raw_spin_lock_irqsave (&clockevents_lock, flags); +list_for_each_entry (dev, &clockevent_devices, list) { +if (!(dev->features &CLOCK_EVT_FEAT_ONESHOT )) +continue ; + + mult_raw = dev->mult; +if (mult_raw != last_mult_raw) { + last_mult_raw = mult_raw; + last_mult_adjusted = +__clockevents_calc_adjust_freq (mult_raw, + mult_cs_mono, + mult_cs_raw); + } + dev->mult_adjusted = last_mult_adjusted; + } +raw_spin_unlock_irqrestore (&clockevents_lock, flags); +} +int __clockevents_update_freq (struct clock_event_device *dev,u32 freq) {clockevents_config (dev, freq);
kernel/time/tick-internal.h +1 -0
@@ -56,6 +56,7 @@ extern int clockevents_program_event(struct clock_event_device *dev, ktime_t expires, bool force);extern void clockevents_handle_noop (struct clock_event_device *dev);extern int __clockevents_update_freq (struct clock_event_device *dev,u32 freq); +extern void clockevents_adjust_all_freqs (u32 mult_cs_mono,u32 mult_cs_raw);extern void timekeeping_get_mono_mult (u32 *mult_cs_mono,u32 *mult_cs_raw);extern ssize_t sysfs_get_uname (const char *buf,char *dst,size_t cnt);
kernel/time/timekeeping.c +3 -0
@@ -1845,6 +1845,9 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk, tk->xtime_interval += interval; tk->tkr_mono.xtime_nsec -= offset; tk->ntp_error -= (interval - offset) << tk->ntp_error_shift; + +clockevents_adjust_all_freqs (tk->tkr_mono.mult, + tk->tkr_mono.clock->mult); }/*
Signature · 1 line
2.9.2
Re: [RFC v4 22/22] timekeeping: inform clockevents about freq adjustments
kernel/time/timekeeping.c static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk, · line 1851
Quoted 10 lines · Nicolai Stange
kernel/time/timekeeping.c +3
-0@@ -1845,6 +1845,9 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk, tk->xtime_interval += interval; tk->tkr_mono.xtime_nsec -= offset; tk->ntp_error -= (interval - offset) << tk->ntp_error_shift; + +clockevents_adjust_all_freqs (tk->tkr_mono.mult, + tk->tkr_mono.clock->mult); }
FYI, the kernel build robot has reported a build failure for CONFIG_GENERIC_CLOCKEVENTS=n.
In order to avoid unnecessary traffic, I'll only fix this up in a v5 if you give the whole thing a go.
Re: [RFC v4 22/22] timekeeping: inform clockevents about freq adjustments
In reply to Nicolai StangeQuoted 7 lines · Nicolai Stange
Upon adjustments of the monotonic clock's frequencies from the timekeeping core, the clockevents devices' ->mult_adjusted should be changed accordingly, too.
Introduce
clockevents_adjust_all_freqs()which traverses all registered clockevent devices and recalculates their ->mult_adjusted based on the monotonic clock's current frequency.
I'm not sure whether adjusting all devices is a good idea. We have systems where the clockevent device is operated from a totally different crystal than the clocksource device which is used for timekeeping. At least we want an opt-out flag for this.
Thanks,
tglx
Re: [RFC v4 22/22] timekeeping: inform clockevents about freq adjustments
In reply to Nicolai Stangekernel/time/clockevents.c void __clockevents_adjust_freq(struct clock_event_device *dev) · line 582
Quoted 12 lines · Nicolai Stange
kernel/time/clockevents.c +12
-0+raw_spin_lock_irqsave (&clockevents_lock, flags); +list_for_each_entry (dev, &clockevent_devices, list) { +if (!(dev->features &CLOCK_EVT_FEAT_ONESHOT )) +continue ; + + mult_raw = dev->mult; +if (mult_raw != last_mult_raw) { + last_mult_raw = mult_raw; + last_mult_adjusted = +__clockevents_calc_adjust_freq (mult_raw, + mult_cs_mono, + mult_cs_raw);
What makes sure that the resulting shift/mult pair is still valid after this adjustment? The non adjusted mult/shift pair might be right at the border of potential overflows and the adjustment might just put it over the edge.... We need at least sanity checks here.
Thanks,
tglx