Re: [PATCH v2 1/3] powerpc/time: Export boot_tb and log initial timebase at boot

[Christophe Leroy]

Le 04/02/2025 à 07:39, Kajol Jain a écrit :
> From: Aboorva Devarajan <aboorvad@xxxxxxxxxxxxx>
>
> - Export `boot_tb` for external use, this is useful in perf vpa-dtl
>    interface, where `boot_tb` can be used to convert raw timebase
>    values to it's relative boot timestamp.

A modification of boot_tb can be catastrophic. I would not export it as 
is as it doesn't need to be modified, would be better to provide it 
through an accessor, for instance a function called get_boot_tb()

By the way, it is only modified by time_init() which is an __init 
function, so boot_tb should be marked __ro_after_init instead of 
__read_mostly, then in that case it would be more acceptable to export it.

> - Log the initial timebase at `time_init` as it is a useful
>    information which can be referred to as needed.
>
> Signed-off-by: Aboorva Devarajan <aboorvad@xxxxxxxxxxxxx>
> Signed-off-by: Kajol Jain <kjain@xxxxxxxxxxxxx>
> ---
>   arch/powerpc/include/asm/time.h | 1 +
>   arch/powerpc/kernel/time.c      | 4 +++-
>   2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index 9bdd8080299b..b6fc5df01d53 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -23,6 +23,7 @@ extern u64 decrementer_max;
>   extern unsigned long tb_ticks_per_jiffy;
>   extern unsigned long tb_ticks_per_usec;
>   extern unsigned long tb_ticks_per_sec;
> +extern u64 boot_tb;
>   extern struct clock_event_device decrementer_clockevent;
>   extern u64 decrementer_max;
>   
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index 0727332ad86f..6e8548f0e48f 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -137,7 +137,8 @@ EXPORT_SYMBOL_GPL(rtc_lock);
>   
>   static u64 tb_to_ns_scale __read_mostly;
>   static unsigned tb_to_ns_shift __read_mostly;
> -static u64 boot_tb __read_mostly;
> +u64 boot_tb __read_mostly;
> +EXPORT_SYMBOL_GPL(boot_tb);
>   
>   extern struct timezone sys_tz;
>   static long timezone_offset;
> @@ -943,6 +944,7 @@ void __init time_init(void)
>   	tb_to_ns_shift = shift;
>   	/* Save the current timebase to pretty up CONFIG_PRINTK_TIME */
>   	boot_tb = get_tb();
> +	pr_debug("%s: timebase at boot: %llu\n", __func__, (unsigned long long)boot_tb);
>   
>   	/* If platform provided a timezone (pmac), we correct the time */
>   	if (timezone_offset) {