[PATCH] clocksource: Fix arm_arch_timer clockmode when vDSO disabled

[PATCH] clocksource: Fix arm_arch_timer clockmode when vDSO disabled

[Vincenzo Frascino]

The arm_arch_timer requires that VDSO_CLOCKMODE_ARCHTIMER to be
defined to compile correctly. On arm the vDSO can be disabled and when
this is the case the compilation ends prematurely with an error:

 $ make ARCH=arm multi_v7_defconfig
 $ ./scripts/config -d VDSO
 $ make

drivers/clocksource/arm_arch_timer.c:73:44: error:
‘VDSO_CLOCKMODE_ARCHTIMER’ undeclared here (not in a function)
  static enum vdso_clock_mode vdso_default = VDSO_CLOCKMODE_ARCHTIMER;
                                             ^
scripts/Makefile.build:267: recipe for target
'drivers/clocksource/arm_arch_timer.o' failed
make[2]: *** [drivers/clocksource/arm_arch_timer.o] Error 1
make[2]: *** Waiting for unfinished jobs....
scripts/Makefile.build:505: recipe for target 'drivers/clocksource' failed
make[1]: *** [drivers/clocksource] Error 2
make[1]: *** Waiting for unfinished jobs....
Makefile:1683: recipe for target 'drivers' failed
make: *** [drivers] Error 2

Define VDSO_CLOCKMODE_ARCHTIMER as VDSO_CLOCKMODE_NONE when the vDSOs are
not enabled to address the issue.

Fixes: 5e3c6a312a09 ("ARM/arm64: vdso: Use common vdso clock mode storage")
Cc: Russell King <linux@armlinux.org.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 drivers/clocksource/arm_arch_timer.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index ee2420d56f67..619839221f94 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -49,6 +49,11 @@
 #define CNTV_TVAL	0x38
 #define CNTV_CTL	0x3c
 
+#ifndef CONFIG_GENERIC_GETTIMEOFDAY
+/* The define below is required because on arm the VDSOs can be disabled */
+#define VDSO_CLOCKMODE_ARCHTIMER	VDSO_CLOCKMODE_NONE
+#endif /* CONFIG_GENERIC_GETTIMEOFDAY */
+
 static unsigned arch_timers_present __initdata;
 
 static void __iomem *arch_counter_base;
-- 
2.25.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource: Fix arm_arch_timer clockmode when vDSO disabled

[Marek Szyprowski]

Hi,

On 21.02.2020 14:03, Vincenzo Frascino wrote:
> The arm_arch_timer requires that VDSO_CLOCKMODE_ARCHTIMER to be
> defined to compile correctly. On arm the vDSO can be disabled and when
> this is the case the compilation ends prematurely with an error:
>
>   $ make ARCH=arm multi_v7_defconfig
>   $ ./scripts/config -d VDSO
>   $ make
>
> drivers/clocksource/arm_arch_timer.c:73:44: error:
> ‘VDSO_CLOCKMODE_ARCHTIMER’ undeclared here (not in a function)
>    static enum vdso_clock_mode vdso_default = VDSO_CLOCKMODE_ARCHTIMER;
>                                               ^
> scripts/Makefile.build:267: recipe for target
> 'drivers/clocksource/arm_arch_timer.o' failed
> make[2]: *** [drivers/clocksource/arm_arch_timer.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
> scripts/Makefile.build:505: recipe for target 'drivers/clocksource' failed
> make[1]: *** [drivers/clocksource] Error 2
> make[1]: *** Waiting for unfinished jobs....
> Makefile:1683: recipe for target 'drivers' failed
> make: *** [drivers] Error 2
>
> Define VDSO_CLOCKMODE_ARCHTIMER as VDSO_CLOCKMODE_NONE when the vDSOs are
> not enabled to address the issue.
>
> Fixes: 5e3c6a312a09 ("ARM/arm64: vdso: Use common vdso clock mode storage")
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

Thanks, this fixes the issue.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>   drivers/clocksource/arm_arch_timer.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index ee2420d56f67..619839221f94 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -49,6 +49,11 @@
>   #define CNTV_TVAL	0x38
>   #define CNTV_CTL	0x3c
>   
> +#ifndef CONFIG_GENERIC_GETTIMEOFDAY
> +/* The define below is required because on arm the VDSOs can be disabled */
> +#define VDSO_CLOCKMODE_ARCHTIMER	VDSO_CLOCKMODE_NONE
> +#endif /* CONFIG_GENERIC_GETTIMEOFDAY */
> +
>   static unsigned arch_timers_present __initdata;
>   
>   static void __iomem *arch_counter_base;

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource: Fix arm_arch_timer clockmode when vDSO disabled

[Marc Zyngier]

Vincenzo,

Please include Mark and myself for anything that touches the arch timers
(get_maintainer.pl will tell you who you need to cc).

On 2020-02-21 13:03, Vincenzo Frascino wrote:
> The arm_arch_timer requires that VDSO_CLOCKMODE_ARCHTIMER to be
> defined to compile correctly. On arm the vDSO can be disabled and when
> this is the case the compilation ends prematurely with an error:
> 
>  $ make ARCH=arm multi_v7_defconfig
>  $ ./scripts/config -d VDSO
>  $ make
> 
> drivers/clocksource/arm_arch_timer.c:73:44: error:
> ‘VDSO_CLOCKMODE_ARCHTIMER’ undeclared here (not in a function)
>   static enum vdso_clock_mode vdso_default = VDSO_CLOCKMODE_ARCHTIMER;
>                                              ^
> scripts/Makefile.build:267: recipe for target
> 'drivers/clocksource/arm_arch_timer.o' failed
> make[2]: *** [drivers/clocksource/arm_arch_timer.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
> scripts/Makefile.build:505: recipe for target 'drivers/clocksource' 
> failed
> make[1]: *** [drivers/clocksource] Error 2
> make[1]: *** Waiting for unfinished jobs....
> Makefile:1683: recipe for target 'drivers' failed
> make: *** [drivers] Error 2
> 
> Define VDSO_CLOCKMODE_ARCHTIMER as VDSO_CLOCKMODE_NONE when the vDSOs 
> are
> not enabled to address the issue.
> 
> Fixes: 5e3c6a312a09 ("ARM/arm64: vdso: Use common vdso clock mode 
> storage")
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
>  drivers/clocksource/arm_arch_timer.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c
> b/drivers/clocksource/arm_arch_timer.c
> index ee2420d56f67..619839221f94 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -49,6 +49,11 @@
>  #define CNTV_TVAL	0x38
>  #define CNTV_CTL	0x3c
> 
> +#ifndef CONFIG_GENERIC_GETTIMEOFDAY
> +/* The define below is required because on arm the VDSOs can be 
> disabled */
> +#define VDSO_CLOCKMODE_ARCHTIMER	VDSO_CLOCKMODE_NONE
> +#endif /* CONFIG_GENERIC_GETTIMEOFDAY */

This feels pretty clunky.

I'd extect VDSO_ARCH_CLOCKMODES (or some similar architecture-specific
symbol) to be used for vdso_default, and that symbol to be defined as
VDSO_CLOCKMODE_NONE when CONFIG_GENERIC_GETTIMEOFDAY isn't selected.

Otherwise, you'll end-up replicating the same pattern in every
clock-source that gets used by the VDSO.

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource: Fix arm_arch_timer clockmode when vDSO disabled

[Vincenzo Frascino]

[-- Attachment #1: Type: text/plain, Size: 1654 bytes --]

Hi Marc,

On 21/02/2020 13:34, Marc Zyngier wrote:
> Vincenzo,
> 
> Please include Mark and myself for anything that touches the arch timers
> (get_maintainer.pl will tell you who you need to cc).
> 

Sorry about that, I posted it too quickly without the proper Cc on the patch.

> On 2020-02-21 13:03, Vincenzo Frascino wrote:

[...]

> 
> This feels pretty clunky.
> 
> I'd extect VDSO_ARCH_CLOCKMODES (or some similar architecture-specific
> symbol) to be used for vdso_default, and that symbol to be defined as
> VDSO_CLOCKMODE_NONE when CONFIG_GENERIC_GETTIMEOFDAY isn't selected.
>

My understanding is that currently VDSO_ARCH_CLOCKMODES depending on the
architecture can identify one or more clocks. In the case of arm and the
arm_arch_timer the arch specific symbol is VDSO_CLOCKMODE_ARCHTIMER (used for
vdso_default), which as you are correctly stating has to be defined as
VDSO_CLOCKMODE_NONE when CONFIG_GENERIC_GETTIMEOFDAY isn't selected.

> Otherwise, you'll end-up replicating the same pattern in every
> clock-source that gets used by the VDSO.

Based on my investigation this fix should be replicated for all the clocksources
used by architectures supported by Unified VDSO and of which VDSOs can be
disabled (otherwise the current solution works). After a quick grep on the
kernel tree:

 $ grep -nr "config VDSO" *

arch/arm/mm/Kconfig:895:config VDSO

Since the only clocksource that falls into these conditions seems to be
arm_arch_timer I modified its driver.

Please feel free to correct me if I missed or misunderstood something.

> 
>         M.

-- 
Regards,
Vincenzo

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 14291 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource: Fix arm_arch_timer clockmode when vDSO disabled

[Marc Zyngier]

On 2020-02-21 14:48, Vincenzo Frascino wrote:
> Hi Marc,
> 
> On 21/02/2020 13:34, Marc Zyngier wrote:
>> Vincenzo,
>> 
>> Please include Mark and myself for anything that touches the arch 
>> timers
>> (get_maintainer.pl will tell you who you need to cc).
>> 
> 
> Sorry about that, I posted it too quickly without the proper Cc on the 
> patch.
> 
>> On 2020-02-21 13:03, Vincenzo Frascino wrote:
> 
> [...]
> 
>> 
>> This feels pretty clunky.
>> 
>> I'd extect VDSO_ARCH_CLOCKMODES (or some similar architecture-specific
>> symbol) to be used for vdso_default, and that symbol to be defined as
>> VDSO_CLOCKMODE_NONE when CONFIG_GENERIC_GETTIMEOFDAY isn't selected.
>> 
> 
> My understanding is that currently VDSO_ARCH_CLOCKMODES depending on 
> the
> architecture can identify one or more clocks. In the case of arm and 
> the
> arm_arch_timer the arch specific symbol is VDSO_CLOCKMODE_ARCHTIMER 
> (used for
> vdso_default), which as you are correctly stating has to be defined as
> VDSO_CLOCKMODE_NONE when CONFIG_GENERIC_GETTIMEOFDAY isn't selected.

This isn't what I'm saying. What I'm suggesting here is that there is
possibly a missing indirection, which defaults to ARCH_TIMER when the
VDSO is selected, and NONE when it isn't.

Overloading a known symbol feels like papering over the issue.

Ideally, this default symbol would be provided by asm/clocksource.h, but
that may not even be the right thing to do.

>> Otherwise, you'll end-up replicating the same pattern in every
>> clock-source that gets used by the VDSO.
> 
> Based on my investigation this fix should be replicated for all the 
> clocksources
> used by architectures supported by Unified VDSO and of which VDSOs can 
> be
> disabled (otherwise the current solution works). After a quick grep on 
> the
> kernel tree:
> 
>  $ grep -nr "config VDSO" *
> 
> arch/arm/mm/Kconfig:895:config VDSO
> 
> Since the only clocksource that falls into these conditions seems to be
> arm_arch_timer I modified its driver.

Fair enough. But don't override the symbol locally. Create a new one:

diff --git a/drivers/clocksource/arm_arch_timer.c 
b/drivers/clocksource/arm_arch_timer.c
index ee2420d56f67..7eb3db75211d 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -69,7 +69,12 @@ static enum arch_timer_ppi_nr arch_timer_uses_ppi = 
ARCH_TIMER_VIRT_PPI;
  static bool arch_timer_c3stop;
  static bool arch_timer_mem_use_virtual;
  static bool arch_counter_suspend_stop;
-static enum vdso_clock_mode vdso_default = VDSO_CLOCKMODE_ARCHTIMER;
+#ifdef CONFIG_GENERIC_GETTIMEOFDAY
+#define __VDSO_DEFAULT VDSO_CLOCKMODE_ARCHTIMER
+#else
+#define __VDSO_DEFAULT VDSO_CLOCKMODE_NONE
+#endif
+static enum vdso_clock_mode vdso_default = __VDSO_DEFAULT;

  static cpumask_t evtstrm_available = CPU_MASK_NONE;
  static bool evtstrm_enable = 
IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM);

Or even this (no, I'm not suggesting this seriously):

diff --git a/drivers/clocksource/arm_arch_timer.c 
b/drivers/clocksource/arm_arch_timer.c
index ee2420d56f67..836b500d1bf1 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -69,7 +69,7 @@ static enum arch_timer_ppi_nr arch_timer_uses_ppi = 
ARCH_TIMER_VIRT_PPI;
  static bool arch_timer_c3stop;
  static bool arch_timer_mem_use_virtual;
  static bool arch_counter_suspend_stop;
-static enum vdso_clock_mode vdso_default = VDSO_CLOCKMODE_ARCHTIMER;
+static enum vdso_clock_mode vdso_default = VDSO_CLOCKMODE_MAX - 1;

  static cpumask_t evtstrm_available = CPU_MASK_NONE;
  static bool evtstrm_enable = 
IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM);

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource: Fix arm_arch_timer clockmode when vDSO disabled

[Vincenzo Frascino]

[-- Attachment #1: Type: text/plain, Size: 1860 bytes --]

Hi Marc,

On 21/02/2020 15:28, Marc Zyngier wrote:

[...]

> 
> This isn't what I'm saying. What I'm suggesting here is that there is
> possibly a missing indirection, which defaults to ARCH_TIMER when the
> VDSO is selected, and NONE when it isn't.
> 
> Overloading a known symbol feels like papering over the issue.
> 
> Ideally, this default symbol would be provided by asm/clocksource.h, but
> that may not even be the right thing to do.
> 

I must admit I really like this idea :), how about:

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 03bbfc312fe7..97864aabc2a6 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -3,6 +3,7 @@ config ARM
        bool
        default y
        select ARCH_32BIT_OFF_T
+       select ARCH_CLOCKSOURCE_DATA
        select ARCH_HAS_BINFMT_FLAT
        select ARCH_HAS_DEBUG_VIRTUAL if MMU
        select ARCH_HAS_DEVMEM_IS_ALLOWED
diff --git a/arch/arm/include/asm/clocksource.h b/arch/arm/include/asm/clocksource.h
index 73beb7f131de..e37f6d74ba49 100644
--- a/arch/arm/include/asm/clocksource.h
+++ b/arch/arm/include/asm/clocksource.h
@@ -1,7 +1,18 @@
 #ifndef _ASM_CLOCKSOURCE_H
 #define _ASM_CLOCKSOURCE_H

+/*
+ * Unused required for compilation only
+ */
+struct arch_clocksource_data {
+       bool __reserved;
+};
+
+#ifdef CONFIG_GENERIC_GETTIMEOFDAY
 #define VDSO_ARCH_CLOCKMODES   \
        VDSO_CLOCKMODE_ARCHTIMER
+#else
+#define VDSO_CLOCKMODE_ARCHTIMER       VDSO_CLOCKMODE_NONE
+#endif

 #endif


[...]

> 
> Fair enough. But don't override the symbol locally. Create a new one:
> 

I see what you mean now, you mean to not overload the semantical meaning of the
symbol. The symbol (VDSO_CLOCKMODE_ARCHTIMER) at this point is never defined
when VDSO=n, but I agree with you it can cause confusion.

-- 
Regards,
Vincenzo

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 14291 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource: Fix arm_arch_timer clockmode when vDSO disabled

[Marc Zyngier]

On 2020-02-21 15:56, Vincenzo Frascino wrote:
> Hi Marc,
> 
> On 21/02/2020 15:28, Marc Zyngier wrote:
> 
> [...]
> 
>> 
>> This isn't what I'm saying. What I'm suggesting here is that there is
>> possibly a missing indirection, which defaults to ARCH_TIMER when the
>> VDSO is selected, and NONE when it isn't.
>> 
>> Overloading a known symbol feels like papering over the issue.
>> 
>> Ideally, this default symbol would be provided by asm/clocksource.h, 
>> but
>> that may not even be the right thing to do.
>> 
> 
> I must admit I really like this idea :), how about:
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 03bbfc312fe7..97864aabc2a6 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -3,6 +3,7 @@ config ARM
>         bool
>         default y
>         select ARCH_32BIT_OFF_T
> +       select ARCH_CLOCKSOURCE_DATA
>         select ARCH_HAS_BINFMT_FLAT
>         select ARCH_HAS_DEBUG_VIRTUAL if MMU
>         select ARCH_HAS_DEVMEM_IS_ALLOWED
> diff --git a/arch/arm/include/asm/clocksource.h
> b/arch/arm/include/asm/clocksource.h
> index 73beb7f131de..e37f6d74ba49 100644
> --- a/arch/arm/include/asm/clocksource.h
> +++ b/arch/arm/include/asm/clocksource.h
> @@ -1,7 +1,18 @@
>  #ifndef _ASM_CLOCKSOURCE_H
>  #define _ASM_CLOCKSOURCE_H
> 
> +/*
> + * Unused required for compilation only
> + */
> +struct arch_clocksource_data {
> +       bool __reserved;
> +};
> +
> +#ifdef CONFIG_GENERIC_GETTIMEOFDAY
>  #define VDSO_ARCH_CLOCKMODES   \
>         VDSO_CLOCKMODE_ARCHTIMER
> +#else
> +#define VDSO_CLOCKMODE_ARCHTIMER       VDSO_CLOCKMODE_NONE
> +#endif

Which is exactly the same thing as before. It's not an indirection,
it is just another overloading of an existing symbol.

>> Fair enough. But don't override the symbol locally. Create a new one:
>> 
> 
> I see what you mean now, you mean to not overload the semantical 
> meaning of the
> symbol. The symbol (VDSO_CLOCKMODE_ARCHTIMER) at this point is never 
> defined
> when VDSO=n, but I agree with you it can cause confusion.

Exactly. It breaks the expectation that if VDSO_CLOCKMODE_ARCHTIMER 
exists,
it has a unique, known value. Yes, the outcome is the same. That doesn't
make it acceptable though.

So building on your above example, here's what I'd like to see:

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1dcc64bd3621..202b41dae05b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -3,6 +3,7 @@ config ARM
         bool
         default y
         select ARCH_32BIT_OFF_T
+       select ARCH_CLOCKSOURCE_DATA
         select ARCH_HAS_BINFMT_FLAT
         select ARCH_HAS_DEBUG_VIRTUAL if MMU
         select ARCH_HAS_DEVMEM_IS_ALLOWED
diff --git a/arch/arm/include/asm/clocksource.h 
b/arch/arm/include/asm/clocksource.h
index 73beb7f131de..bd4347865f6d 100644
--- a/arch/arm/include/asm/clocksource.h
+++ b/arch/arm/include/asm/clocksource.h
@@ -1,7 +1,17 @@
  #ifndef _ASM_CLOCKSOURCE_H
  #define _ASM_CLOCKSOURCE_H

+struct arch_clocksource_data {
+       /* Empty on purpose */
+};
+
+#ifdef CONFIG_GENERIC_GETTIMEOFDAY
  #define VDSO_ARCH_CLOCKMODES   \
         VDSO_CLOCKMODE_ARCHTIMER

+#define ARCH_VDSO_DEFAULT_CLOCKMODE    VDSO_CLOCKMODE_ARCHTIMER
+#else
+#define ARCH_VDSO_DEFAULT_CLOCKMODE    VDSO_CLOCKMODE_NONE
+#endif
+
  #endif
diff --git a/arch/arm64/include/asm/clocksource.h 
b/arch/arm64/include/asm/clocksource.h
index eb82e9d95c5d..de706362fa81 100644
--- a/arch/arm64/include/asm/clocksource.h
+++ b/arch/arm64/include/asm/clocksource.h
@@ -5,4 +5,6 @@
  #define VDSO_ARCH_CLOCKMODES   \
         VDSO_CLOCKMODE_ARCHTIMER

+#define ARCH_VDSO_DEFAULT_CLOCKMODE    VDSO_CLOCKMODE_ARCHTIMER
+
  #endif
diff --git a/drivers/clocksource/arm_arch_timer.c 
b/drivers/clocksource/arm_arch_timer.c
index ee2420d56f67..8b7081583eeb 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -69,7 +69,7 @@ static enum arch_timer_ppi_nr arch_timer_uses_ppi = 
ARCH_TIMER_VIRT_PPI;
  static bool arch_timer_c3stop;
  static bool arch_timer_mem_use_virtual;
  static bool arch_counter_suspend_stop;
-static enum vdso_clock_mode vdso_default = VDSO_CLOCKMODE_ARCHTIMER;
+static enum vdso_clock_mode vdso_default = ARCH_VDSO_DEFAULT_CLOCKMODE;

  static cpumask_t evtstrm_available = CPU_MASK_NONE;
  static bool evtstrm_enable = 
IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM);

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource: Fix arm_arch_timer clockmode when vDSO disabled

[Vincenzo Frascino]

[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]

Hi Marc,

On 21/02/2020 16:24, Marc Zyngier wrote:
> On 2020-02-21 15:56, Vincenzo Frascino wrote:

[...]

> 
> Which is exactly the same thing as before. It's not an indirection,
> it is just another overloading of an existing symbol.
>

Yes, sorry, I missed the indirection here. I wanted more to agree on the concept.

>>> Fair enough. But don't override the symbol locally. Create a new one:
>>>
>>
>> I see what you mean now, you mean to not overload the semantical meaning of the
>> symbol. The symbol (VDSO_CLOCKMODE_ARCHTIMER) at this point is never defined
>> when VDSO=n, but I agree with you it can cause confusion.
> 
> Exactly. It breaks the expectation that if VDSO_CLOCKMODE_ARCHTIMER exists,
> it has a unique, known value. Yes, the outcome is the same. That doesn't
> make it acceptable though.
> 
> So building on your above example, here's what I'd like to see:
> 

[...]
All right, I agree, I will post the patches later on today.

-- 
Regards,
Vincenzo

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 14291 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel