| Message ID | 20250204063948.643852-1-kjain@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/3] powerpc/time: Export boot_tb and log initial timebase at boot | expand |
Le 04/02/2025 à 07:39, Kajol Jain a écrit : > From: Aboorva Devarajan <aboorvad@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. > > - 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@linux.ibm.com> > Signed-off-by: Kajol Jain <kjain@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) {
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 <aboorvad@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 <aboorvad@linux.ibm.com> > > Signed-off-by: Kajol Jain <kjain@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) {
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) {