From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755066AbcIIUCF (ORCPT ); Fri, 9 Sep 2016 16:02:05 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:34310 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753554AbcIIUB0 (ORCPT ); Fri, 9 Sep 2016 16:01:26 -0400 From: Nicolai Stange To: Thomas Gleixner Cc: John Stultz , linux-kernel@vger.kernel.org, Nicolai Stange Subject: [RFC v6 14/23] clockevents: decouple ->max_delta_ns from ->max_delta_ticks Date: Fri, 9 Sep 2016 22:00:24 +0200 Message-Id: <20160909200033.32103-15-nicstange@gmail.com> X-Mailer: git-send-email 2.9.3 In-Reply-To: <20160909200033.32103-1-nicstange@gmail.com> References: <20160909200033.32103-1-nicstange@gmail.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Before converting the given delta from ns to cycles by means of the mult/shift pair, clockevents_program_event() enforces it to be less or equal than ->max_delta_ns. Simplified, this reads as delta = min(delta, dev->max_delta_ns); clc = (delta * dev->mult) >> dev->shift; A device's ->max_delta_ns is chosen such that 1.) The multiplication does not overflow 64 bits. 2.) clc fits into an unsigned long. 3.) clc <= the ->max_delta_ticks allowed by hardware. Item 3.) is responsible for making ->max_delta_ns depend tightly on ->max_delta_ticks and the device's frequency, i.e. its mult/shift pair. As it stands, any update to ->mult would require a corresponding change of ->max_delta_ns as well and these two had to get consumed in the clockevent programming path atomically. This would have a negative performance impact as soon as we start adjusting ->mult from a different CPU with upcoming changes making the clockevents core NTP correction aware: some kind of locking would have been needed in the event programming path. In order to circumvent this necessity, ->max_delta_ns could be made small enough to cover the whole range of possible adjustments to ->mult, eliminating the need to update it at all. The timekeeping core never adjusts its mono clock's inverse frequency by more than 11% (c.f. timekeeping_adjust()). This means that a clockevent device's adjusted mult will never grow beyond 1 / (1-11%) = 112.4% of its initial value. Thus, reducing ->max_delta_ns unconditionally to 1/112.4% = 89% would do the trick. (Actually, setting it to 8/9 = 88.89% thereof allows for mult increases of up to 12.5% = 1/8 which is good for binary arithmetic.) However, such a decrease of ->max_delta_ns could become problematic for devices whose ->max_delta_ticks is small, i.e. those for whom item 3.) prevails. In order to work around this, I chose to make ->max_delta_ns completely independent of ->max_delta_ticks. It is set to 8/9 of the maximum value satisfying conditions 1.) and 2.) for the given ->mult. Item 3.) is made explicit by introducing an extra min(clc, ->max_delta_ticks) after the ns to cycles conversion in clockevents_program_event(). This way, the maximum programmable delta is reduced only for those devices which have got a large ->max_delta_ticks anyway. This comes at the cost of an extra min() in the event programming path though. So, - In the case of CLOCK_EVT_FEAT_NO_ADJUST not being enabled, set ->max_delta_ns to 8/9 of the maximum possible value such that items 1.) and 2.) hold for the given ->mult. OTOH, if CLOCK_EVT_FEAT_NO_ADJUST is not set, set ->max_delta_ns to the full such value. - Add a clc = min(clc, dev->max_delta_ticks) after the ns to cycle conversion in clockevents_program_event(). - Move the ->max_delta_ticks member into struct clock_event_device's first 64 bytes in order to optimize for cacheline usage. - Finally, preserve the semantics of /proc/timer_list by letting it display the minimum of ->max_delta_ns and ->max_delta_ticks converted to ns at the 'max_delta_ns' field. Signed-off-by: Nicolai Stange --- include/linux/clockchips.h | 4 ++-- kernel/time/clockevents.c | 50 +++++++++++++++++++++++++++++++++++++++++++++- kernel/time/timer_list.c | 6 +++++- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h index 9a25185..be5f222 100644 --- a/include/linux/clockchips.h +++ b/include/linux/clockchips.h @@ -81,6 +81,7 @@ enum clock_event_state { * @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 * @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 @@ -93,7 +94,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_ticks: maximum delta value in ticks 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) @@ -109,6 +109,7 @@ struct clock_event_device { ktime_t next_event; u64 max_delta_ns; u64 min_delta_ns; + unsigned long max_delta_ticks; u32 mult; u32 shift; enum clock_event_state state_use_accessors; @@ -125,7 +126,6 @@ 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; const char *name; int rating; diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c index f352f54..472fcc2 100644 --- a/kernel/time/clockevents.c +++ b/kernel/time/clockevents.c @@ -17,6 +17,7 @@ #include #include #include +#include #include "tick-internal.h" @@ -336,6 +337,9 @@ int clockevents_program_event(struct clock_event_device *dev, ktime_t expires, 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); + rc = dev->set_next_event((unsigned long) clc, dev); return (rc && force) ? clockevents_program_min_delta(dev) : rc; @@ -444,15 +448,59 @@ EXPORT_SYMBOL_GPL(clockevents_unbind_device); static void __clockevents_update_bounds(struct clock_event_device *dev) { + u64 max_delta_ns; + if (!(dev->features & CLOCK_EVT_FEAT_ONESHOT)) return; /* + * max_delta_ns is <= the maximum value such that the ns to + * cycles conversion in clockevents_program_event() doesn't + * overflow. It follows that is has to fulfill the following + * constraints: + * 1.) mult * max_delta_ns <= ULLONG_MAX + * 2.) (mult * max_delta_ns) >> shift <= ULONG_MAX + * On 32 bit archs, 2.) is stricter because shift <= 32 always + * holds, otherwise 1.) is. + * + * If CLOCK_EVT_FEAT_NO_ADJUST is not set, the actual + * ->max_delta_ns is set to a value equal to 8/9 of the + * maximum one possible. This gives some room for increasing + * mult by up to 12.5% which is just enough to compensate for + * the up to 11% mono clock frequency adjustments made by the + * timekeeping core, c.f. timekeeping_adjust(). + * + */ +#if BITS_PER_LONG == 32 + /* + * Set the lower BITS_PER_LONG + dev->shift bits. + * Note: dev->shift can be 32, so avoid undefined behaviour. + * The ^= below is really a |= here. However the former is the + * common idiom and recognizable by gcc. + */ + max_delta_ns = (u64)1 << (BITS_PER_LONG + dev->shift - 1); + max_delta_ns ^= (dev->max_delta_ns - 1); +#else + max_delta_ns = U64_MAX; +#endif + if (unlikely(!dev->mult)) { + dev->mult = 1; + WARN_ON(1); + } + do_div(max_delta_ns, dev->mult); + dev->max_delta_ns = max_delta_ns; + if (!(dev->features & CLOCK_EVT_FEAT_NO_ADJUST)) { + /* reduce by 1/9 */ + do_div(max_delta_ns, 9); + ++max_delta_ns; /* round up */ + dev->max_delta_ns -= max_delta_ns; + } + + /* * 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); } /** diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c index ba7d8b2..ac20d4c 100644 --- a/kernel/time/timer_list.c +++ b/kernel/time/timer_list.c @@ -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; + u64 max_delta_ns; + + max_delta_ns = clockevent_delta2ns(dev->max_delta_ticks, dev); + max_delta_ns = min(max_delta_ns, dev->max_delta_ns); SEQ_printf(m, "Tick Device: mode: %d\n", td->mode); if (cpu < 0) @@ -220,7 +224,7 @@ print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu) } SEQ_printf(m, "%s\n", dev->name); SEQ_printf(m, " max_delta_ns: %llu\n", - (unsigned long long) dev->max_delta_ns); + (unsigned long long) max_delta_ns); SEQ_printf(m, " min_delta_ns: %llu\n", (unsigned long long) dev->min_delta_ns); SEQ_printf(m, " mult: %u\n", dev->mult); -- 2.9.3