On Tue, 2025-02-04 at 11:28 +0100, Christophe Leroy wrote:
Hi Christophe,
>
> Le 04/02/2025 à 07:39, Kajol Jain a écrit :
> > From: Aboorva Devarajan <aboor...@linux.ibm.com>
> >
> > - 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.
Thanks for the review and suggestions. Yes `boot_tb` is just initialized once
and is never updated again, I’ll update the patch to mark it __ro_after_init.
I’ll also use an accessor function (get_boot_tb()) instead of exporting it,
as it will be clean and maintainable.
Will send a v3 with the updates.
Regards,
Aboorva
>
> > - 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 <aboor...@linux.ibm.com>
> > Signed-off-by: Kajol Jain <kj...@linux.ibm.com>
> > ---
> > 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) {
