The field has no value because all clock bases have the same
resolution. The resolution only changes when we switch to high
resolution timer mode. We can evaluate that from a single static
variable as well. In the !HIGHRES case its simply a constant.
Export the variable, so we can simplify the usage sites.
Signed-off-by: Thomas Gleixner <[email protected]>
---
include/linux/hrtimer.h | 6 ++++--
kernel/time/hrtimer.c | 26 +++++++++-----------------
kernel/time/timer_list.c | 8 ++++----
3 files changed, 17 insertions(+), 23 deletions(-)
Index: tip/include/linux/hrtimer.h
===================================================================
--- tip.orig/include/linux/hrtimer.h
+++ tip/include/linux/hrtimer.h
@@ -137,7 +137,6 @@ struct hrtimer_sleeper {
* timer to a base on another cpu.
* @clockid: clock id for per_cpu support
* @active: red black tree root node for the active timers
- * @resolution: the resolution of the clock, in nanoseconds
* @get_time: function to retrieve the current time of the clock
* @softirq_time: the time when running the hrtimer queue in the softirq
* @offset: offset of this clock to the monotonic base
@@ -147,7 +146,6 @@ struct hrtimer_clock_base {
int index;
clockid_t clockid;
struct timerqueue_head active;
- ktime_t resolution;
ktime_t (*get_time)(void);
ktime_t softirq_time;
ktime_t offset;
@@ -295,11 +293,15 @@ extern void hrtimer_peek_ahead_timers(vo
extern void clock_was_set_delayed(void);
+extern unsigned int hrtimer_resolution;
+
#else
# define MONOTONIC_RES_NSEC LOW_RES_NSEC
# define KTIME_MONOTONIC_RES KTIME_LOW_RES
+#define hrtimer_resolution LOW_RES_NSEC
+
static inline void hrtimer_peek_ahead_timers(void) { }
/*
Index: tip/kernel/time/hrtimer.c
===================================================================
--- tip.orig/kernel/time/hrtimer.c
+++ tip/kernel/time/hrtimer.c
@@ -66,7 +66,6 @@
*/
DEFINE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases) =
{
-
.lock = __RAW_SPIN_LOCK_UNLOCKED(hrtimer_bases.lock),
.clock_base =
{
@@ -74,25 +73,21 @@ DEFINE_PER_CPU(struct hrtimer_cpu_base,
.index = HRTIMER_BASE_MONOTONIC,
.clockid = CLOCK_MONOTONIC,
.get_time = &ktime_get,
- .resolution = KTIME_LOW_RES,
},
{
.index = HRTIMER_BASE_REALTIME,
.clockid = CLOCK_REALTIME,
.get_time = &ktime_get_real,
- .resolution = KTIME_LOW_RES,
},
{
.index = HRTIMER_BASE_BOOTTIME,
.clockid = CLOCK_BOOTTIME,
.get_time = &ktime_get_boottime,
- .resolution = KTIME_LOW_RES,
},
{
.index = HRTIMER_BASE_TAI,
.clockid = CLOCK_TAI,
.get_time = &ktime_get_clocktai,
- .resolution = KTIME_LOW_RES,
},
}
};
@@ -478,6 +473,8 @@ static ktime_t __hrtimer_get_next_event(
* High resolution timer enabled ?
*/
static int hrtimer_hres_enabled __read_mostly = 1;
+unsigned int hrtimer_resolution __read_mostly = LOW_RES_NSEC;
+EXPORT_SYMBOL_GPL(hrtimer_resolution);
/*
* Enable / Disable high resolution mode
@@ -660,7 +657,7 @@ static void retrigger_next_event(void *a
*/
static int hrtimer_switch_to_hres(void)
{
- int i, cpu = smp_processor_id();
+ int cpu = smp_processor_id();
struct hrtimer_cpu_base *base = &per_cpu(hrtimer_bases, cpu);
unsigned long flags;
@@ -676,8 +673,7 @@ static int hrtimer_switch_to_hres(void)
return 0;
}
base->hres_active = 1;
- for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++)
- base->clock_base[i].resolution = KTIME_HIGH_RES;
+ hrtimer_resolution = HIGH_RES_NSEC;
tick_setup_sched_timer();
/* "Retrigger" the interrupt to get things going */
@@ -812,8 +808,8 @@ u64 hrtimer_forward(struct hrtimer *time
if (delta.tv64 < 0)
return 0;
- if (interval.tv64 < timer->base->resolution.tv64)
- interval.tv64 = timer->base->resolution.tv64;
+ if (interval.tv64 < hrtimer_resolution)
+ interval.tv64 = hrtimer_resolution;
if (unlikely(delta.tv64 >= interval.tv64)) {
s64 incr = ktime_to_ns(interval);
@@ -955,7 +951,7 @@ int __hrtimer_start_range_ns(struct hrti
* timeouts. This will go away with the GTOD framework.
*/
#ifdef CONFIG_TIME_LOW_RES
- tim = ktime_add_safe(tim, base->resolution);
+ tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
#endif
}
@@ -1185,12 +1181,8 @@ EXPORT_SYMBOL_GPL(hrtimer_init);
*/
int hrtimer_get_res(const clockid_t which_clock, struct timespec *tp)
{
- struct hrtimer_cpu_base *cpu_base;
- int base = hrtimer_clockid_to_base(which_clock);
-
- cpu_base = raw_cpu_ptr(&hrtimer_bases);
- *tp = ktime_to_timespec(cpu_base->clock_base[base].resolution);
-
+ tp->tv_sec = 0;
+ tp->tv_nsec = hrtimer_resolution;
return 0;
}
EXPORT_SYMBOL_GPL(hrtimer_get_res);
Index: tip/kernel/time/timer_list.c
===================================================================
--- tip.orig/kernel/time/timer_list.c
+++ tip/kernel/time/timer_list.c
@@ -120,10 +120,10 @@ static void
print_base(struct seq_file *m, struct hrtimer_clock_base *base, u64 now)
{
SEQ_printf(m, " .base: %pK\n", base);
- SEQ_printf(m, " .index: %d\n",
- base->index);
- SEQ_printf(m, " .resolution: %Lu nsecs\n",
- (unsigned long long)ktime_to_ns(base->resolution));
+ SEQ_printf(m, " .index: %d\n", base->index);
+
+ SEQ_printf(m, " .resolution: %u nsecs\n", (unsigned) hrtimer_resolution);
+
SEQ_printf(m, " .get_time: ");
print_name_offset(m, base->get_time);
SEQ_printf(m, "\n");
On Tue, Apr 14, 2015 at 11:08 PM, Thomas Gleixner <[email protected]> wrote:
> The field has no value because all clock bases have the same
> resolution. The resolution only changes when we switch to high
> resolution timer mode. We can evaluate that from a single static
> variable as well. In the !HIGHRES case its simply a constant.
>
> Export the variable, so we can simplify the usage sites.
There seems to be only one usage site outside hrtimer.c itself. That
one only reads the value. Wouldn't it make sense to keep the variable
from the interface and use a read function instead?
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> include/linux/hrtimer.h | 6 ++++--
> kernel/time/hrtimer.c | 26 +++++++++-----------------
> kernel/time/timer_list.c | 8 ++++----
> 3 files changed, 17 insertions(+), 23 deletions(-)
>
> Index: tip/include/linux/hrtimer.h
> ===================================================================
> --- tip.orig/include/linux/hrtimer.h
> +++ tip/include/linux/hrtimer.h
> @@ -137,7 +137,6 @@ struct hrtimer_sleeper {
> * timer to a base on another cpu.
> * @clockid: clock id for per_cpu support
> * @active: red black tree root node for the active timers
> - * @resolution: the resolution of the clock, in nanoseconds
> * @get_time: function to retrieve the current time of the clock
> * @softirq_time: the time when running the hrtimer queue in the softirq
> * @offset: offset of this clock to the monotonic base
> @@ -147,7 +146,6 @@ struct hrtimer_clock_base {
> int index;
> clockid_t clockid;
> struct timerqueue_head active;
> - ktime_t resolution;
> ktime_t (*get_time)(void);
> ktime_t softirq_time;
> ktime_t offset;
> @@ -295,11 +293,15 @@ extern void hrtimer_peek_ahead_timers(vo
>
> extern void clock_was_set_delayed(void);
>
> +extern unsigned int hrtimer_resolution;
> +
> #else
>
> # define MONOTONIC_RES_NSEC LOW_RES_NSEC
> # define KTIME_MONOTONIC_RES KTIME_LOW_RES
>
> +#define hrtimer_resolution LOW_RES_NSEC
> +
> static inline void hrtimer_peek_ahead_timers(void) { }
>
> /*
> Index: tip/kernel/time/hrtimer.c
> ===================================================================
> --- tip.orig/kernel/time/hrtimer.c
> +++ tip/kernel/time/hrtimer.c
> @@ -66,7 +66,6 @@
> */
> DEFINE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases) =
> {
> -
> .lock = __RAW_SPIN_LOCK_UNLOCKED(hrtimer_bases.lock),
> .clock_base =
> {
> @@ -74,25 +73,21 @@ DEFINE_PER_CPU(struct hrtimer_cpu_base,
> .index = HRTIMER_BASE_MONOTONIC,
> .clockid = CLOCK_MONOTONIC,
> .get_time = &ktime_get,
> - .resolution = KTIME_LOW_RES,
> },
> {
> .index = HRTIMER_BASE_REALTIME,
> .clockid = CLOCK_REALTIME,
> .get_time = &ktime_get_real,
> - .resolution = KTIME_LOW_RES,
> },
> {
> .index = HRTIMER_BASE_BOOTTIME,
> .clockid = CLOCK_BOOTTIME,
> .get_time = &ktime_get_boottime,
> - .resolution = KTIME_LOW_RES,
> },
> {
> .index = HRTIMER_BASE_TAI,
> .clockid = CLOCK_TAI,
> .get_time = &ktime_get_clocktai,
> - .resolution = KTIME_LOW_RES,
> },
> }
> };
> @@ -478,6 +473,8 @@ static ktime_t __hrtimer_get_next_event(
> * High resolution timer enabled ?
> */
> static int hrtimer_hres_enabled __read_mostly = 1;
> +unsigned int hrtimer_resolution __read_mostly = LOW_RES_NSEC;
> +EXPORT_SYMBOL_GPL(hrtimer_resolution);
>
> /*
> * Enable / Disable high resolution mode
> @@ -660,7 +657,7 @@ static void retrigger_next_event(void *a
> */
> static int hrtimer_switch_to_hres(void)
> {
> - int i, cpu = smp_processor_id();
> + int cpu = smp_processor_id();
> struct hrtimer_cpu_base *base = &per_cpu(hrtimer_bases, cpu);
> unsigned long flags;
>
> @@ -676,8 +673,7 @@ static int hrtimer_switch_to_hres(void)
> return 0;
> }
> base->hres_active = 1;
> - for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++)
> - base->clock_base[i].resolution = KTIME_HIGH_RES;
> + hrtimer_resolution = HIGH_RES_NSEC;
>
> tick_setup_sched_timer();
> /* "Retrigger" the interrupt to get things going */
> @@ -812,8 +808,8 @@ u64 hrtimer_forward(struct hrtimer *time
> if (delta.tv64 < 0)
> return 0;
>
> - if (interval.tv64 < timer->base->resolution.tv64)
> - interval.tv64 = timer->base->resolution.tv64;
> + if (interval.tv64 < hrtimer_resolution)
> + interval.tv64 = hrtimer_resolution;
>
> if (unlikely(delta.tv64 >= interval.tv64)) {
> s64 incr = ktime_to_ns(interval);
> @@ -955,7 +951,7 @@ int __hrtimer_start_range_ns(struct hrti
> * timeouts. This will go away with the GTOD framework.
> */
> #ifdef CONFIG_TIME_LOW_RES
> - tim = ktime_add_safe(tim, base->resolution);
> + tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
> #endif
> }
>
> @@ -1185,12 +1181,8 @@ EXPORT_SYMBOL_GPL(hrtimer_init);
> */
> int hrtimer_get_res(const clockid_t which_clock, struct timespec *tp)
> {
> - struct hrtimer_cpu_base *cpu_base;
> - int base = hrtimer_clockid_to_base(which_clock);
> -
> - cpu_base = raw_cpu_ptr(&hrtimer_bases);
> - *tp = ktime_to_timespec(cpu_base->clock_base[base].resolution);
> -
> + tp->tv_sec = 0;
> + tp->tv_nsec = hrtimer_resolution;
> return 0;
> }
> EXPORT_SYMBOL_GPL(hrtimer_get_res);
> Index: tip/kernel/time/timer_list.c
> ===================================================================
> --- tip.orig/kernel/time/timer_list.c
> +++ tip/kernel/time/timer_list.c
> @@ -120,10 +120,10 @@ static void
> print_base(struct seq_file *m, struct hrtimer_clock_base *base, u64 now)
> {
> SEQ_printf(m, " .base: %pK\n", base);
> - SEQ_printf(m, " .index: %d\n",
> - base->index);
> - SEQ_printf(m, " .resolution: %Lu nsecs\n",
> - (unsigned long long)ktime_to_ns(base->resolution));
> + SEQ_printf(m, " .index: %d\n", base->index);
> +
> + SEQ_printf(m, " .resolution: %u nsecs\n", (unsigned) hrtimer_resolution);
hrtimer_resolution already is an unsigned int. Doesn't that make this
cast pointless?
Thanks,
Frans
On Wed, Apr 15, 2015 at 8:29 AM, Frans Klaver <[email protected]> wrote:
> On Tue, Apr 14, 2015 at 11:08 PM, Thomas Gleixner <[email protected]> wrote:
>> The field has no value because all clock bases have the same
>> resolution. The resolution only changes when we switch to high
>> resolution timer mode. We can evaluate that from a single static
>> variable as well. In the !HIGHRES case its simply a constant.
>>
>> Export the variable, so we can simplify the usage sites.
>
> There seems to be only one usage site outside hrtimer.c itself. That
> one only reads the value.
OK, this is incorrect. Still, if it's only meant to be read from
outside hrtimer.c:
> Wouldn't it make sense to keep the variable
> from the interface and use a read function instead?
Thanks,
Frans
On 04/15/2015 02:38 AM, Thomas Gleixner wrote:
>>The field has no value because all clock bases have the same
>>resolution. The resolution only changes when we switch to high
>>resolution timer mode. We can evaluate that from a single static
>>variable as well. In the !HIGHRES case its simply a constant.
>>
>>Export the variable, so we can simplify the usage sites.
>>
>>Signed-off-by: Thomas Gleixner <[email protected]>
>>---
Reviewed-by: Preeti U Murthy <[email protected]>
Commit-ID: 398ca17fb54b212cdc9da7ff4a17a35c48dd2103
Gitweb: http://git.kernel.org/tip/398ca17fb54b212cdc9da7ff4a17a35c48dd2103
Author: Thomas Gleixner <[email protected]>
AuthorDate: Tue, 14 Apr 2015 21:08:27 +0000
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 22 Apr 2015 17:06:48 +0200
hrtimer: Get rid of the resolution field in hrtimer_clock_base
The field has no value because all clock bases have the same
resolution. The resolution only changes when we switch to high
resolution timer mode. We can evaluate that from a single static
variable as well. In the !HIGHRES case its simply a constant.
Export the variable, so we can simplify the usage sites.
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Preeti U Murthy <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: Marcelo Tosatti <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
include/linux/hrtimer.h | 6 ++++--
kernel/time/hrtimer.c | 26 +++++++++-----------------
kernel/time/timer_list.c | 8 ++++----
3 files changed, 17 insertions(+), 23 deletions(-)
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 7770676..bc6f91b 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -137,7 +137,6 @@ struct hrtimer_sleeper {
* timer to a base on another cpu.
* @clockid: clock id for per_cpu support
* @active: red black tree root node for the active timers
- * @resolution: the resolution of the clock, in nanoseconds
* @get_time: function to retrieve the current time of the clock
* @softirq_time: the time when running the hrtimer queue in the softirq
* @offset: offset of this clock to the monotonic base
@@ -147,7 +146,6 @@ struct hrtimer_clock_base {
int index;
clockid_t clockid;
struct timerqueue_head active;
- ktime_t resolution;
ktime_t (*get_time)(void);
ktime_t softirq_time;
ktime_t offset;
@@ -295,11 +293,15 @@ extern void hrtimer_peek_ahead_timers(void);
extern void clock_was_set_delayed(void);
+extern unsigned int hrtimer_resolution;
+
#else
# define MONOTONIC_RES_NSEC LOW_RES_NSEC
# define KTIME_MONOTONIC_RES KTIME_LOW_RES
+#define hrtimer_resolution LOW_RES_NSEC
+
static inline void hrtimer_peek_ahead_timers(void) { }
/*
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 9abd50b..965687a 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -66,7 +66,6 @@
*/
DEFINE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases) =
{
-
.lock = __RAW_SPIN_LOCK_UNLOCKED(hrtimer_bases.lock),
.clock_base =
{
@@ -74,25 +73,21 @@ DEFINE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases) =
.index = HRTIMER_BASE_MONOTONIC,
.clockid = CLOCK_MONOTONIC,
.get_time = &ktime_get,
- .resolution = KTIME_LOW_RES,
},
{
.index = HRTIMER_BASE_REALTIME,
.clockid = CLOCK_REALTIME,
.get_time = &ktime_get_real,
- .resolution = KTIME_LOW_RES,
},
{
.index = HRTIMER_BASE_BOOTTIME,
.clockid = CLOCK_BOOTTIME,
.get_time = &ktime_get_boottime,
- .resolution = KTIME_LOW_RES,
},
{
.index = HRTIMER_BASE_TAI,
.clockid = CLOCK_TAI,
.get_time = &ktime_get_clocktai,
- .resolution = KTIME_LOW_RES,
},
}
};
@@ -478,6 +473,8 @@ static ktime_t __hrtimer_get_next_event(struct hrtimer_cpu_base *cpu_base)
* High resolution timer enabled ?
*/
static int hrtimer_hres_enabled __read_mostly = 1;
+unsigned int hrtimer_resolution __read_mostly = LOW_RES_NSEC;
+EXPORT_SYMBOL_GPL(hrtimer_resolution);
/*
* Enable / Disable high resolution mode
@@ -660,7 +657,7 @@ static void retrigger_next_event(void *arg)
*/
static int hrtimer_switch_to_hres(void)
{
- int i, cpu = smp_processor_id();
+ int cpu = smp_processor_id();
struct hrtimer_cpu_base *base = &per_cpu(hrtimer_bases, cpu);
unsigned long flags;
@@ -676,8 +673,7 @@ static int hrtimer_switch_to_hres(void)
return 0;
}
base->hres_active = 1;
- for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++)
- base->clock_base[i].resolution = KTIME_HIGH_RES;
+ hrtimer_resolution = HIGH_RES_NSEC;
tick_setup_sched_timer();
/* "Retrigger" the interrupt to get things going */
@@ -820,8 +816,8 @@ u64 hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval)
if (delta.tv64 < 0)
return 0;
- if (interval.tv64 < timer->base->resolution.tv64)
- interval.tv64 = timer->base->resolution.tv64;
+ if (interval.tv64 < hrtimer_resolution)
+ interval.tv64 = hrtimer_resolution;
if (unlikely(delta.tv64 >= interval.tv64)) {
s64 incr = ktime_to_ns(interval);
@@ -963,7 +959,7 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
* timeouts. This will go away with the GTOD framework.
*/
#ifdef CONFIG_TIME_LOW_RES
- tim = ktime_add_safe(tim, base->resolution);
+ tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
#endif
}
@@ -1193,12 +1189,8 @@ EXPORT_SYMBOL_GPL(hrtimer_init);
*/
int hrtimer_get_res(const clockid_t which_clock, struct timespec *tp)
{
- struct hrtimer_cpu_base *cpu_base;
- int base = hrtimer_clockid_to_base(which_clock);
-
- cpu_base = raw_cpu_ptr(&hrtimer_bases);
- *tp = ktime_to_timespec(cpu_base->clock_base[base].resolution);
-
+ tp->tv_sec = 0;
+ tp->tv_nsec = hrtimer_resolution;
return 0;
}
EXPORT_SYMBOL_GPL(hrtimer_get_res);
diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c
index 5960af21..bdd5e98 100644
--- a/kernel/time/timer_list.c
+++ b/kernel/time/timer_list.c
@@ -127,10 +127,10 @@ static void
print_base(struct seq_file *m, struct hrtimer_clock_base *base, u64 now)
{
SEQ_printf(m, " .base: %pK\n", base);
- SEQ_printf(m, " .index: %d\n",
- base->index);
- SEQ_printf(m, " .resolution: %Lu nsecs\n",
- (unsigned long long)ktime_to_ns(base->resolution));
+ SEQ_printf(m, " .index: %d\n", base->index);
+
+ SEQ_printf(m, " .resolution: %u nsecs\n", (unsigned) hrtimer_resolution);
+
SEQ_printf(m, " .get_time: ");
print_name_offset(m, base->get_time);
SEQ_printf(m, "\n");