linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sched/clock: Make local_clock/cpu_clock inline
@ 2016-04-01 20:04 Daniel Lezcano
  2016-04-13  9:41 ` Ingo Molnar
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Lezcano @ 2016-04-01 20:04 UTC (permalink / raw)
  To: peterz; +Cc: Ingo Molnar, open list:SCHEDULER

The local_clock/cpu_clock functions were changed to prevent a double
identical test with sched_clock_cpu() when HAVE_UNSTABLE_SCHED_CLOCK
is set. That resulted in one line functions.

As these functions are in all the cases one line functions and in the
hot path, it is useful to specify them as static inline in order to
give a strong hint to the compiler.

After verification, it appears the compiler does not inline them
without this hint. Change those functions to static inline.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 include/linux/sched.h | 32 ++++++++++++++++++++++++++++++--
 kernel/sched/clock.c  | 41 -----------------------------------------
 2 files changed, 30 insertions(+), 43 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 52c4847..13c1c1d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2303,8 +2303,6 @@ extern unsigned long long notrace sched_clock(void);
 /*
  * See the comment in kernel/sched/clock.c
  */
-extern u64 cpu_clock(int cpu);
-extern u64 local_clock(void);
 extern u64 running_clock(void);
 extern u64 sched_clock_cpu(int cpu);
 
@@ -2323,6 +2321,16 @@ static inline void sched_clock_idle_sleep_event(void)
 static inline void sched_clock_idle_wakeup_event(u64 delta_ns)
 {
 }
+
+static inline u64 cpu_clock(int cpu)
+{
+	return sched_clock();
+}
+
+static inline u64 local_clock(void)
+{
+	return sched_clock();
+}
 #else
 /*
  * Architectures can set this to 1 if they have specified
@@ -2337,6 +2345,26 @@ extern void clear_sched_clock_stable(void);
 extern void sched_clock_tick(void);
 extern void sched_clock_idle_sleep_event(void);
 extern void sched_clock_idle_wakeup_event(u64 delta_ns);
+
+/*
+ * As outlined in clock.c, provides a fast, high resolution, nanosecond
+ * time source that is monotonic per cpu argument and has bounded drift
+ * between cpus.
+ *
+ * ######################### BIG FAT WARNING ##########################
+ * # when comparing cpu_clock(i) to cpu_clock(j) for i != j, time can #
+ * # go backwards !!                                                  #
+ * ####################################################################
+ */
+static inline u64 cpu_clock(int cpu)
+{
+	return sched_clock_cpu(cpu);
+}
+
+static inline u64 local_clock(void)
+{
+	return sched_clock_cpu(raw_smp_processor_id());
+}
 #endif
 
 #ifdef CONFIG_IRQ_TIME_ACCOUNTING
diff --git a/kernel/sched/clock.c b/kernel/sched/clock.c
index 30c4b20..e9d6d6a 100644
--- a/kernel/sched/clock.c
+++ b/kernel/sched/clock.c
@@ -363,33 +363,6 @@ void sched_clock_idle_wakeup_event(u64 delta_ns)
 }
 EXPORT_SYMBOL_GPL(sched_clock_idle_wakeup_event);
 
-/*
- * As outlined at the top, provides a fast, high resolution, nanosecond
- * time source that is monotonic per cpu argument and has bounded drift
- * between cpus.
- *
- * ######################### BIG FAT WARNING ##########################
- * # when comparing cpu_clock(i) to cpu_clock(j) for i != j, time can #
- * # go backwards !!                                                  #
- * ####################################################################
- */
-u64 cpu_clock(int cpu)
-{
-	return sched_clock_cpu(cpu);
-}
-
-/*
- * Similar to cpu_clock() for the current cpu. Time will only be observed
- * to be monotonic if care is taken to only compare timestampt taken on the
- * same CPU.
- *
- * See cpu_clock().
- */
-u64 local_clock(void)
-{
-	return sched_clock_cpu(raw_smp_processor_id());
-}
-
 #else /* CONFIG_HAVE_UNSTABLE_SCHED_CLOCK */
 
 void sched_clock_init(void)
@@ -404,22 +377,8 @@ u64 sched_clock_cpu(int cpu)
 
 	return sched_clock();
 }
-
-u64 cpu_clock(int cpu)
-{
-	return sched_clock();
-}
-
-u64 local_clock(void)
-{
-	return sched_clock();
-}
-
 #endif /* CONFIG_HAVE_UNSTABLE_SCHED_CLOCK */
 
-EXPORT_SYMBOL_GPL(cpu_clock);
-EXPORT_SYMBOL_GPL(local_clock);
-
 /*
  * Running clock - returns the time that has elapsed while a guest has been
  * running.
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] sched/clock: Make local_clock/cpu_clock inline
  2016-04-01 20:04 [PATCH] sched/clock: Make local_clock/cpu_clock inline Daniel Lezcano
@ 2016-04-13  9:41 ` Ingo Molnar
  2016-04-13  9:46   ` Daniel Lezcano
  0 siblings, 1 reply; 5+ messages in thread
From: Ingo Molnar @ 2016-04-13  9:41 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: peterz, Ingo Molnar, open list:SCHEDULER


* Daniel Lezcano <daniel.lezcano@linaro.org> wrote:

> The local_clock/cpu_clock functions were changed to prevent a double
> identical test with sched_clock_cpu() when HAVE_UNSTABLE_SCHED_CLOCK
> is set. That resulted in one line functions.
> 
> As these functions are in all the cases one line functions and in the
> hot path, it is useful to specify them as static inline in order to
> give a strong hint to the compiler.
> 
> After verification, it appears the compiler does not inline them
> without this hint. Change those functions to static inline.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  include/linux/sched.h | 32 ++++++++++++++++++++++++++++++--
>  kernel/sched/clock.c  | 41 -----------------------------------------
>  2 files changed, 30 insertions(+), 43 deletions(-)

Hm, this does not seem to apply to v4.6-rc3 cleanly.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] sched/clock: Make local_clock/cpu_clock inline
  2016-04-13  9:41 ` Ingo Molnar
@ 2016-04-13  9:46   ` Daniel Lezcano
  2016-04-13  9:51     ` Ingo Molnar
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Lezcano @ 2016-04-13  9:46 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: peterz, Ingo Molnar, open list:SCHEDULER

On 04/13/2016 11:41 AM, Ingo Molnar wrote:
>
> * Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
>> The local_clock/cpu_clock functions were changed to prevent a double
>> identical test with sched_clock_cpu() when HAVE_UNSTABLE_SCHED_CLOCK
>> is set. That resulted in one line functions.
>>
>> As these functions are in all the cases one line functions and in the
>> hot path, it is useful to specify them as static inline in order to
>> give a strong hint to the compiler.
>>
>> After verification, it appears the compiler does not inline them
>> without this hint. Change those functions to static inline.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>   include/linux/sched.h | 32 ++++++++++++++++++++++++++++++--
>>   kernel/sched/clock.c  | 41 -----------------------------------------
>>   2 files changed, 30 insertions(+), 43 deletions(-)
>
> Hm, this does not seem to apply to v4.6-rc3 cleanly.

Actually I based the patches on top of tip/sched/core.

Shall I rebase them against v4.6-rc3 ?


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] sched/clock: Make local_clock/cpu_clock inline
  2016-04-13  9:46   ` Daniel Lezcano
@ 2016-04-13  9:51     ` Ingo Molnar
  2016-04-13 10:05       ` Daniel Lezcano
  0 siblings, 1 reply; 5+ messages in thread
From: Ingo Molnar @ 2016-04-13  9:51 UTC (permalink / raw)
  To: Daniel Lezcano; +Cc: peterz, Ingo Molnar, open list:SCHEDULER


* Daniel Lezcano <daniel.lezcano@linaro.org> wrote:

> On 04/13/2016 11:41 AM, Ingo Molnar wrote:
> >
> >* Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >
> >>The local_clock/cpu_clock functions were changed to prevent a double
> >>identical test with sched_clock_cpu() when HAVE_UNSTABLE_SCHED_CLOCK
> >>is set. That resulted in one line functions.
> >>
> >>As these functions are in all the cases one line functions and in the
> >>hot path, it is useful to specify them as static inline in order to
> >>give a strong hint to the compiler.
> >>
> >>After verification, it appears the compiler does not inline them
> >>without this hint. Change those functions to static inline.
> >>
> >>Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> >>---
> >>  include/linux/sched.h | 32 ++++++++++++++++++++++++++++++--
> >>  kernel/sched/clock.c  | 41 -----------------------------------------
> >>  2 files changed, 30 insertions(+), 43 deletions(-)
> >
> >Hm, this does not seem to apply to v4.6-rc3 cleanly.
> 
> Actually I based the patches on top of tip/sched/core.

This patch does not seem to apply to tip/sched/core either (2b8c41daba32).

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] sched/clock: Make local_clock/cpu_clock inline
  2016-04-13  9:51     ` Ingo Molnar
@ 2016-04-13 10:05       ` Daniel Lezcano
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Lezcano @ 2016-04-13 10:05 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: peterz, Ingo Molnar, open list:SCHEDULER

On 04/13/2016 11:51 AM, Ingo Molnar wrote:
>
> * Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
>> On 04/13/2016 11:41 AM, Ingo Molnar wrote:
>>>
>>> * Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>>
>>>> The local_clock/cpu_clock functions were changed to prevent a double
>>>> identical test with sched_clock_cpu() when HAVE_UNSTABLE_SCHED_CLOCK
>>>> is set. That resulted in one line functions.
>>>>
>>>> As these functions are in all the cases one line functions and in the
>>>> hot path, it is useful to specify them as static inline in order to
>>>> give a strong hint to the compiler.
>>>>
>>>> After verification, it appears the compiler does not inline them
>>>> without this hint. Change those functions to static inline.
>>>>
>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> ---
>>>>   include/linux/sched.h | 32 ++++++++++++++++++++++++++++++--
>>>>   kernel/sched/clock.c  | 41 -----------------------------------------
>>>>   2 files changed, 30 insertions(+), 43 deletions(-)
>>>
>>> Hm, this does not seem to apply to v4.6-rc3 cleanly.
>>
>> Actually I based the patches on top of tip/sched/core.
>
> This patch does not seem to apply to tip/sched/core either (2b8c41daba32).

Ah, yes. I received a compilation error because of a missing 
EXPORT_SYMBOL_GPL for an inlined function call from kbuild test:

ERROR: "sched_clock_cpu" [kernel/torture.ko] undefined!
ERROR: "sched_clock_cpu" [drivers/gpu/drm/i915/i915.ko] undefined!

So I resent a fixed V2.

https://lkml.org/lkml/2016/4/11/603
https://lkml.org/lkml/2016/4/11/602

Sorry, I did not notice you were mentioning the first version which is 
outdated. The V2 is based on top of 2b8c41daba32.

   -- Daniel

-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-04-13 10:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-01 20:04 [PATCH] sched/clock: Make local_clock/cpu_clock inline Daniel Lezcano
2016-04-13  9:41 ` Ingo Molnar
2016-04-13  9:46   ` Daniel Lezcano
2016-04-13  9:51     ` Ingo Molnar
2016-04-13 10:05       ` Daniel Lezcano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).