| Message ID | 20170718221607.132539-1-salyzyn@android.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, 18 Jul 2017, Mark Salyzyn wrote: > Go directly to the rtc for persistent wall clock time and print. > Useful if REALTIME is required to be logged in a low level power > management function or when clock activities are suspended. An > aid to permit user space alignment of kernel activities. Can you please spare the churn and wait until the discussion about the general issues is resolved before sending out yet another set of patches, which do not address any of the real important questions? Thanks, tglx
On Tue, Jul 18, 2017 at 3:15 PM, Mark Salyzyn <salyzyn@android.com> wrote: > Go directly to the rtc for persistent wall clock time and print. So, first, the above doesn't seem accurate to me. You're using getnstimeofday64() which doesn't touch the RTC. > Useful if REALTIME is required to be logged in a low level power > management function or when clock activities are suspended. An > aid to permit user space alignment of kernel activities. This explanation doesn't make much sense either. I've tried looking over the patch series to better understand but its not super clear right off. A good practice is to start by explaining what you want to do ("allow for battery and suspend time analysis"), and why the current upstream kernel doesn't provide what you need ("timekeeping can be suspended, so we want to reference the persistent clock that is always running and provide a way to align those timestamps with CLOCK_MONOTONIC/CLOCK_REALTIME timestamps that userspace uses"), and only then explain how your solution addresses this issue. > Feature activated by CONFIG_RTC_SHOW_TIME. > > Signed-off-by: Mark Salyzyn <salyzyn@android.com> > > v2 > - move implementation to kernel timekeeping from rtc_lib files > - use rtc_time64_to_tm() instead of rtc_time_to_tm() > - use inline in include/linux/rtc.h for !CONFIG_RTC_SHOW_TIME > v3 > - _correctly_ use rtc_time64_to_tm > --- > include/linux/rtc.h | 5 +++++ > kernel/time/Kconfig | 11 +++++++++++ > kernel/time/Makefile | 1 + > kernel/time/rtc_show_time.c | 23 +++++++++++++++++++++++ > 4 files changed, 40 insertions(+) > create mode 100644 kernel/time/rtc_show_time.c > > diff --git a/include/linux/rtc.h b/include/linux/rtc.h > index 0a0f0d14a5fb..779401c937d5 100644 > --- a/include/linux/rtc.h > +++ b/include/linux/rtc.h > @@ -22,6 +22,11 @@ extern int rtc_year_days(unsigned int day, unsigned int month, unsigned int year > extern int rtc_valid_tm(struct rtc_time *tm); > extern time64_t rtc_tm_to_time64(struct rtc_time *tm); > extern void rtc_time64_to_tm(time64_t time, struct rtc_time *tm); > +#ifdef CONFIG_RTC_SHOW_TIME > +extern void rtc_show_time(const char *prefix_msg); > +#else > +static inline void rtc_show_time(const char *prefix_msg) { } > +#endif > ktime_t rtc_tm_to_ktime(struct rtc_time tm); > struct rtc_time rtc_ktime_to_tm(ktime_t kt); > > diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig > index ac09bc29eb08..4da093ae3e37 100644 > --- a/kernel/time/Kconfig > +++ b/kernel/time/Kconfig > @@ -145,3 +145,14 @@ config HIGH_RES_TIMERS > > endmenu > endif > + > +config RTC_SHOW_TIME > + bool "rtc_show_time instrumentation" > + select RTC_LIB > + help > + Activate rtc_show_time(const char *msg) wall clock time > + instrumentation. > + > + The instrumentation is used to help triage and synchronize > + kernel logs using CLOCK_MONOTONIC and user space activity > + logs utilizing CLOCK_REALTIME references. RTC_SHOW_TIME seems like a poor name for this, as you're not really doing much with the RTC here (other then printing the current CLOCK_REALTIME value to be printed out in in a format that's commonly used w/ RTC hardware). As for the reference to CLOCK_MONOTONIC, I assume you are correlating that via printk timestamp (which may not always be enabled)? > diff --git a/kernel/time/Makefile b/kernel/time/Makefile > index 938dbf33ef49..66f17e641052 100644 > --- a/kernel/time/Makefile > +++ b/kernel/time/Makefile > @@ -17,3 +17,4 @@ obj-$(CONFIG_GENERIC_SCHED_CLOCK) += sched_clock.o > obj-$(CONFIG_TICK_ONESHOT) += tick-oneshot.o tick-sched.o > obj-$(CONFIG_DEBUG_FS) += timekeeping_debug.o > obj-$(CONFIG_TEST_UDELAY) += test_udelay.o > +obj-$(CONFIG_RTC_SHOW_TIME) += rtc_show_time.o > diff --git a/kernel/time/rtc_show_time.c b/kernel/time/rtc_show_time.c > new file mode 100644 > index 000000000000..bbf4f92abf4f > --- /dev/null > +++ b/kernel/time/rtc_show_time.c > @@ -0,0 +1,23 @@ > +/* > + * rtc time printing utility functions > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/rtc.h> > + > +void rtc_show_time(const char *prefix_msg) > +{ > + struct timespec64 ts; > + struct rtc_time tm; > + > + getnstimeofday64(&ts); > + rtc_time64_to_tm(ts.tv_sec, &tm); > + pr_info("%s %d-%02d-%02d %02d:%02d:%02d.%09lu UTC\n", > + prefix_msg ? prefix_msg : "Time:", > + tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, > + tm.tm_hour, tm.tm_min, tm.tm_sec, ts.tv_nsec); > +} > +EXPORT_SYMBOL(rtc_show_time); Overall, this does feel very single-use-case specific. thanks -john
On 07/18/2017 05:00 PM, John Stultz wrote: > On Tue, Jul 18, 2017 at 3:15 PM, Mark Salyzyn <salyzyn@android.com> wrote: >> Go directly to the rtc for persistent wall clock time and print. > So, first, the above doesn't seem accurate to me. You're using > getnstimeofday64() which doesn't touch the RTC. I admit I got that backwards. I copied and pasted a line, that was supposed to read "We can not go" ... The point was that persistent or RTC clock is _not_ always available, so we have to print suspend times when the timeofday clock is available at the points before suspend, and after suspend is complete. The "Suspended for" message (in ms, we want us at least) albeit helpful, is performed at the bottom, and can only be reported if there is a persistent clock available. On most Android devices, it is not available. I will post update, a better description in the primary patch, dropping the intro, printing in <epoch>.<ns> format. I asked in another thread if it would be OK to preserve the Legacy of RTC time format printing with another CONFIG parameter since we have 5 years of tooling that depends on that format. -- Mark
diff --git a/include/linux/rtc.h b/include/linux/rtc.h index 0a0f0d14a5fb..779401c937d5 100644 --- a/include/linux/rtc.h +++ b/include/linux/rtc.h @@ -22,6 +22,11 @@ extern int rtc_year_days(unsigned int day, unsigned int month, unsigned int year extern int rtc_valid_tm(struct rtc_time *tm); extern time64_t rtc_tm_to_time64(struct rtc_time *tm); extern void rtc_time64_to_tm(time64_t time, struct rtc_time *tm); +#ifdef CONFIG_RTC_SHOW_TIME +extern void rtc_show_time(const char *prefix_msg); +#else +static inline void rtc_show_time(const char *prefix_msg) { } +#endif ktime_t rtc_tm_to_ktime(struct rtc_time tm); struct rtc_time rtc_ktime_to_tm(ktime_t kt); diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig index ac09bc29eb08..4da093ae3e37 100644 --- a/kernel/time/Kconfig +++ b/kernel/time/Kconfig @@ -145,3 +145,14 @@ config HIGH_RES_TIMERS endmenu endif + +config RTC_SHOW_TIME + bool "rtc_show_time instrumentation" + select RTC_LIB + help + Activate rtc_show_time(const char *msg) wall clock time + instrumentation. + + The instrumentation is used to help triage and synchronize + kernel logs using CLOCK_MONOTONIC and user space activity + logs utilizing CLOCK_REALTIME references. diff --git a/kernel/time/Makefile b/kernel/time/Makefile index 938dbf33ef49..66f17e641052 100644 --- a/kernel/time/Makefile +++ b/kernel/time/Makefile @@ -17,3 +17,4 @@ obj-$(CONFIG_GENERIC_SCHED_CLOCK) += sched_clock.o obj-$(CONFIG_TICK_ONESHOT) += tick-oneshot.o tick-sched.o obj-$(CONFIG_DEBUG_FS) += timekeeping_debug.o obj-$(CONFIG_TEST_UDELAY) += test_udelay.o +obj-$(CONFIG_RTC_SHOW_TIME) += rtc_show_time.o diff --git a/kernel/time/rtc_show_time.c b/kernel/time/rtc_show_time.c new file mode 100644 index 000000000000..bbf4f92abf4f --- /dev/null +++ b/kernel/time/rtc_show_time.c @@ -0,0 +1,23 @@ +/* + * rtc time printing utility functions + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/rtc.h> + +void rtc_show_time(const char *prefix_msg) +{ + struct timespec64 ts; + struct rtc_time tm; + + getnstimeofday64(&ts); + rtc_time64_to_tm(ts.tv_sec, &tm); + pr_info("%s %d-%02d-%02d %02d:%02d:%02d.%09lu UTC\n", + prefix_msg ? prefix_msg : "Time:", + tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, + tm.tm_hour, tm.tm_min, tm.tm_sec, ts.tv_nsec); +} +EXPORT_SYMBOL(rtc_show_time);
Go directly to the rtc for persistent wall clock time and print. Useful if REALTIME is required to be logged in a low level power management function or when clock activities are suspended. An aid to permit user space alignment of kernel activities. Feature activated by CONFIG_RTC_SHOW_TIME. Signed-off-by: Mark Salyzyn <salyzyn@android.com> v2 - move implementation to kernel timekeeping from rtc_lib files - use rtc_time64_to_tm() instead of rtc_time_to_tm() - use inline in include/linux/rtc.h for !CONFIG_RTC_SHOW_TIME v3 - _correctly_ use rtc_time64_to_tm --- include/linux/rtc.h | 5 +++++ kernel/time/Kconfig | 11 +++++++++++ kernel/time/Makefile | 1 + kernel/time/rtc_show_time.c | 23 +++++++++++++++++++++++ 4 files changed, 40 insertions(+) create mode 100644 kernel/time/rtc_show_time.c