Re: [PATCH v1 00/25] lib, rtc: Print rtc_time via %pt[dt][rv]
On Thu, 2017-07-20 at 10:57 -0700, Mark Salyzyn wrote: > It would probably need to take struct timespec64 as an argument. Pass by > structure might be difficult to swallow, so pass by pointer? Every %p extension is passed via a pointer.
Re: [PATCH v1 00/25] lib, rtc: Print rtc_time via %pt[dt][rv]
On 07/20/2017 03:33 AM, Andy Shevchenko wrote: On Tue, 2017-07-18 at 12:57 -0700, Mark Salyzyn wrote: On 07/18/2017 10:50 AM, Joe Perches wrote: On Thu, 2017-06-08 at 16:47 +0300, Andy Shevchenko wrote: Recently I have noticed too many users of struct rtc_time that printing its content field by field. In this series I introduce %pt[dt][rv] specifier to make life a bit easier. Hey Andy. I just saw a patch with a printk for rtc time from Mark Salyzyn. https://lkml.org/lkml/2017/7/18/885 Any idea if you want to push this extension? I like the concept and still think it could be extended a bit more. from: https://lkml.org/lkml/2017/6/8/1134 My preference would be for %pt[type] where is mandatory and could be: r for struct rtc_time 6 for time64_t k for ktime_t T for struct timespec64 etc and has an unspecified default of -MM-DD:hh:mm:ss Perhaps use the "date" formats without the leading % uses for for additional styles. -MM-DD hh:mm:ss.n ? As a separate modifier, yes. See my answer to subthread in patch 4. It would probably need to take struct timespec64 as an argument. Pass by structure might be difficult to swallow, so pass by pointer? As for my need for this in my suspend/resume/hibernate/restore patch set, we have already been told three times to _not_ report wall clock time. I could imagine being a consumer of it in the future if we have difficulty migrating the analysis tools ... so tepid support from me. -- Mark
Re: [PATCH v1 00/25] lib, rtc: Print rtc_time via %pt[dt][rv]
On Tue, 2017-07-18 at 12:57 -0700, Mark Salyzyn wrote: > On 07/18/2017 10:50 AM, Joe Perches wrote: > > On Thu, 2017-06-08 at 16:47 +0300, Andy Shevchenko wrote: > > > Recently I have noticed too many users of struct rtc_time that > > > printing > > > its content field by field. > > > > > > In this series I introduce %pt[dt][rv] specifier to make life a > > > bit > > > easier. > > > > Hey Andy. > > > > I just saw a patch with a printk for rtc time from Mark Salyzyn. > > https://lkml.org/lkml/2017/7/18/885 > > > > Any idea if you want to push this extension? > > > > I like the concept and still think it could be extended a bit more. > > > > from: https://lkml.org/lkml/2017/6/8/1134 > > > > My preference would be for %pt[type] > > where is mandatory and could be: > > > > r for struct rtc_time > > 6 for time64_t > > k for ktime_t > > T for struct timespec64 > > etc > > > > and has an unspecified default of > > -MM-DD:hh:mm:ss > > > > Perhaps use the "date" formats without the leading > > % uses for for additional styles. > > > > -MM-DD hh:mm:ss.n ? As a separate modifier, yes. See my answer to subthread in patch 4. -- Andy Shevchenko Intel Finland Oy
Re: [PATCH v1 00/25] lib, rtc: Print rtc_time via %pt[dt][rv]
On 07/18/2017 10:50 AM, Joe Perches wrote: On Thu, 2017-06-08 at 16:47 +0300, Andy Shevchenko wrote: Recently I have noticed too many users of struct rtc_time that printing its content field by field. In this series I introduce %pt[dt][rv] specifier to make life a bit easier. Hey Andy. I just saw a patch with a printk for rtc time from Mark Salyzyn. https://lkml.org/lkml/2017/7/18/885 Any idea if you want to push this extension? I like the concept and still think it could be extended a bit more. from: https://lkml.org/lkml/2017/6/8/1134 My preference would be for %pt[type] where is mandatory and could be: r for struct rtc_time 6 for time64_t k for ktime_t T for struct timespec64 etc and has an unspecified default of -MM-DD:hh:mm:ss Perhaps use the "date" formats without the leading % uses for for additional styles. -MM-DD hh:mm:ss.n ?
Re: [PATCH v1 00/25] lib, rtc: Print rtc_time via %pt[dt][rv]
On Tue, 2017-07-18 at 20:55 +0300, Andy Shevchenko wrote: > On Tue, 2017-07-18 at 10:50 -0700, Joe Perches wrote: > > On Thu, 2017-06-08 at 16:47 +0300, Andy Shevchenko wrote: > > > Recently I have noticed too many users of struct rtc_time that > > > printing > > > its content field by field. > > > > > > In this series I introduce %pt[dt][rv] specifier to make life a bit > > > easier. > > > > Hey Andy. > > > > I just saw a patch with a printk for rtc time from Mark Salyzyn. > > https://lkml.org/lkml/2017/7/18/885 > > Same! > > > Any idea if you want to push this extension? > > Yes, just really lack of time for everything. > > I like the idea to make it conditional (config BLABLABLA). It will > address some comments about footprint for no users. Only one of the other %p extensions is conditional and that conditional is probably not too useful. I think the code size is relatively small and not particularly valuable for the additional complexity. For instance, all of the code that emits MAC and IP[46] addresses %pM and %pI variants is 2.5K. (x86 allnoconfig) There are lots more code size savings than that lying about. And auditing all the code that might emit a MAC address when CONFIG_NET is not set is probably not worth the effort for the size reduction. $ size lib/vsprintf.o* text data bss dec hex filename 12140 4 0 12144 2f70 lib/vsprintf.o.allnoconfig.new 14785 4 0 14789 39c5 lib/vsprintf.o.allnoconfig.old --- lib/vsprintf.c | 4 1 file changed, 4 insertions(+) diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 86c3385b9eb3..de95e78ca5f0 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -933,6 +933,7 @@ char *bitmap_list_string(char *buf, char *end, unsigned long *bitmap, return buf; } +#ifdef CONFIG_NET static noinline_for_stack char *mac_address_string(char *buf, char *end, u8 *addr, struct printf_spec spec, const char *fmt) @@ -1241,6 +1242,7 @@ char *ip4_addr_string_sa(char *buf, char *end, const struct sockaddr_in *sa, return string(buf, end, ip4_addr, spec); } +#endif static noinline_for_stack char *escaped_string(char *buf, char *end, u8 *addr, struct printf_spec spec, @@ -1741,6 +1743,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, default: return bitmap_string(buf, end, ptr, spec, fmt); } +#ifdef CONFIG_NET case 'M': /* Colon separated: 00:01:02:03:04:05 */ case 'm': /* Contiguous: 000102030405 */ /* [mM]F (FDDI) */ @@ -1777,6 +1780,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, }} } break; +#endif case 'E': return escaped_string(buf, end, ptr, spec, fmt); case 'U':
Re: [PATCH v1 00/25] lib, rtc: Print rtc_time via %pt[dt][rv]
On Tue, 2017-07-18 at 10:50 -0700, Joe Perches wrote: > On Thu, 2017-06-08 at 16:47 +0300, Andy Shevchenko wrote: > > Recently I have noticed too many users of struct rtc_time that > > printing > > its content field by field. > > > > In this series I introduce %pt[dt][rv] specifier to make life a bit > > easier. > > Hey Andy. > > I just saw a patch with a printk for rtc time from Mark Salyzyn. > https://lkml.org/lkml/2017/7/18/885 Same! > Any idea if you want to push this extension? Yes, just really lack of time for everything. I like the idea to make it conditional (config BLABLABLA). It will address some comments about footprint for no users. > I like the concept and still think it could be extended a bit more. > > from: https://lkml.org/lkml/2017/6/8/1134 > > My preference would be for %pt[type] > where is mandatory and could be: > > r for struct rtc_time > 6 for time64_t > k for ktime_t > T for struct timespec64 > etc I dunno about this. However, I like this more than do conversion in each case where input reference has different type. > and has an unspecified default of > -MM-DD:hh:mm:ss I'm against this, sorry. Too many variations for almost no use (users). > Perhaps use the "date" formats without the leading > % uses for for additional styles. -- Andy Shevchenko Intel Finland Oy
Re: [PATCH v1 00/25] lib, rtc: Print rtc_time via %pt[dt][rv]
On Thu, 2017-06-08 at 16:47 +0300, Andy Shevchenko wrote: > Recently I have noticed too many users of struct rtc_time that printing > its content field by field. > > In this series I introduce %pt[dt][rv] specifier to make life a bit > easier. Hey Andy. I just saw a patch with a printk for rtc time from Mark Salyzyn. https://lkml.org/lkml/2017/7/18/885 Any idea if you want to push this extension? I like the concept and still think it could be extended a bit more. from: https://lkml.org/lkml/2017/6/8/1134 My preference would be for %pt[type] where is mandatory and could be: r for struct rtc_time 6 for time64_t k for ktime_t T for struct timespec64 etc and has an unspecified default of -MM-DD:hh:mm:ss Perhaps use the "date" formats without the leading % uses for for additional styles.
Re: [PATCH v1 00/25] lib, rtc: Print rtc_time via %pt[dt][rv]
On Thu, 2017-06-08 at 18:02 +0300, Andy Shevchenko wrote: > On Thu, Jun 8, 2017 at 5:52 PM, Joe Perches wrote: > > On Thu, 2017-06-08 at 16:47 +0300, Andy Shevchenko wrote: > > > Recently I have noticed too many users of struct rtc_time that printing > > > its content field by field. > > > > > > In this series I introduce %pt[dt][rv] specifier to make life a bit > > > easier. [] > > > Most of the users currently are RTC drivers, thus the patch series is > > > assumed to go via RTC tree. > > > > What I wonder about this series is how much > > larger it makes a typical kernel and how > > often multiple rtc clocks are built for a > > single kernel? > > We may hide it under CONFIG_RTC_??? if we want to reduce kernel for > non RTC cases. Depends whether it is for rtc_time only > > What is the size impact on an embedded kernel > > that uses a single rtc driver? > > I would You would what?
Re: [PATCH v1 00/25] lib, rtc: Print rtc_time via %pt[dt][rv]
On Thu, Jun 8, 2017 at 5:52 PM, Joe Perches wrote: > On Thu, 2017-06-08 at 16:47 +0300, Andy Shevchenko wrote: >> Recently I have noticed too many users of struct rtc_time that printing >> its content field by field. >> >> In this series I introduce %pt[dt][rv] specifier to make life a bit >> easier. >> >> There are still users of detailed output of the struct rtc_time, but we >> can introduce an additional extension for them in the future if needed, >> otherwise they might be converted to the proposed output format. >> >> Some of the changes slightly modify the output. In those cases we are on >> the safe side since they are pure debug. Nevertheless I tried to leave >> numbers to be the same or quite close: in some cases year is printed + >> 1900, though month is left in the range [0,11] instead of [1,12]. >> >> I didn't compile everything there, though I did a basic smoke test on >> some x86 hardware. So, I rely on kbuild test robot as well :-) >> >> Most of the users currently are RTC drivers, thus the patch series is >> assumed to go via RTC tree. > > What I wonder about this series is how much > larger it makes a typical kernel and how > often multiple rtc clocks are built for a > single kernel? We may hide it under CONFIG_RTC_??? if we want to reduce kernel for non RTC cases. > What is the size impact on an embedded kernel > that uses a single rtc driver? I would > trivia: Actually not. See my answer to Arnd. I have patches for 4 users of struct tm, but it should be converted first to struct rtc_time first (otherwise it might uglify the code due to endianess of tm_year memeber) > > Aren't there also uses of struct tm that are > nearly identical? > > e.g.: drivers/usb/host/xhci-tegra.c -- With Best Regards, Andy Shevchenko
Re: [PATCH v1 00/25] lib, rtc: Print rtc_time via %pt[dt][rv]
On Thu, 2017-06-08 at 16:47 +0300, Andy Shevchenko wrote: > Recently I have noticed too many users of struct rtc_time that printing > its content field by field. > > In this series I introduce %pt[dt][rv] specifier to make life a bit > easier. > > There are still users of detailed output of the struct rtc_time, but we > can introduce an additional extension for them in the future if needed, > otherwise they might be converted to the proposed output format. > > Some of the changes slightly modify the output. In those cases we are on > the safe side since they are pure debug. Nevertheless I tried to leave > numbers to be the same or quite close: in some cases year is printed + > 1900, though month is left in the range [0,11] instead of [1,12]. > > I didn't compile everything there, though I did a basic smoke test on > some x86 hardware. So, I rely on kbuild test robot as well :-) > > Most of the users currently are RTC drivers, thus the patch series is > assumed to go via RTC tree. What I wonder about this series is how much larger it makes a typical kernel and how often multiple rtc clocks are built for a single kernel? What is the size impact on an embedded kernel that uses a single rtc driver? trivia: Aren't there also uses of struct tm that are nearly identical? e.g.: drivers/usb/host/xhci-tegra.c
[PATCH v1 00/25] lib, rtc: Print rtc_time via %pt[dt][rv]
Recently I have noticed too many users of struct rtc_time that printing its content field by field. In this series I introduce %pt[dt][rv] specifier to make life a bit easier. There are still users of detailed output of the struct rtc_time, but we can introduce an additional extension for them in the future if needed, otherwise they might be converted to the proposed output format. Some of the changes slightly modify the output. In those cases we are on the safe side since they are pure debug. Nevertheless I tried to leave numbers to be the same or quite close: in some cases year is printed + 1900, though month is left in the range [0,11] instead of [1,12]. I didn't compile everything there, though I did a basic smoke test on some x86 hardware. So, I rely on kbuild test robot as well :-) Most of the users currently are RTC drivers, thus the patch series is assumed to go via RTC tree. Andy Shevchenko (25): lib/vsprintf: Remove useless NULL checks lib/vsprintf: Make decspec global lib/vsprintf: Make strspec global lib/vsprintf: Print time and date in human readable format via %pt ds1302: Switch to use %pt rtc: Switch to use %pt rtc: at91rm9200: Switch to use %pt rtc: at91sam9: Switch to use %pt rtc: m41t80: Switch to use %pt rtc: m48t59: Switch to use %pt rtc: mcp795: Switch to use %pt rtc: pcf50633: Switch to use %pt rtc: pic32: Switch to use %pt rtc: pm8xxx: Switch to use %pt rtc: puv3: Switch to use %pt rtc: rk808: Switch to use %pt rtc: rx6110: Switch to use %pt rtc: rx8025: Switch to use %pt rtc: s3c: Switch to use %pt rtc: s5m: Switch to use %pt rtc: tegra: Switch to use %pt mk68/mac: Switch to use %pt Input: hp_sdc_rtc - Switch to use %pt kdb: Switch to use %pt PM: Switch to use %pt Documentation/printk-formats.txt | 17 arch/m68k/mac/misc.c | 8 +- drivers/base/power/trace.c | 4 +- drivers/char/ds1302.c| 38 +++-- drivers/char/rtc.c | 7 +- drivers/input/misc/hp_sdc_rtc.c | 8 +- drivers/rtc/hctosys.c| 8 +- drivers/rtc/interface.c | 8 +- drivers/rtc/rtc-at91rm9200.c | 16 +--- drivers/rtc/rtc-at91sam9.c | 16 +--- drivers/rtc/rtc-m41t80.c | 6 +- drivers/rtc/rtc-m48t59.c | 8 +- drivers/rtc/rtc-mcp795.c | 18 ++--- drivers/rtc/rtc-pcf50633.c | 8 +- drivers/rtc/rtc-pic32.c | 18 + drivers/rtc/rtc-pm8xxx.c | 16 ++-- drivers/rtc/rtc-proc.c | 36 ++--- drivers/rtc/rtc-puv3.c | 18 + drivers/rtc/rtc-rk808.c | 20 ++--- drivers/rtc/rtc-rx6110.c | 12 +-- drivers/rtc/rtc-rx8025.c | 19 + drivers/rtc/rtc-s3c.c| 21 ++--- drivers/rtc/rtc-s5m.c| 27 ++- drivers/rtc/rtc-sysfs.c | 12 +-- drivers/rtc/rtc-tegra.c | 30 +-- kernel/debug/kdb/kdb_main.c | 7 +- lib/vsprintf.c | 167 --- 27 files changed, 248 insertions(+), 325 deletions(-) -- 2.11.0
