On Wed, May 3, 2023 at 12:28 PM Vlastimil Babka <vbabka@xxxxxxx> wrote: > > On 5/3/23 00:50, Dave Chinner wrote: > > On Tue, May 02, 2023 at 07:42:59AM -0400, James Bottomley wrote: > >> On Mon, 2023-05-01 at 23:17 -0400, Kent Overstreet wrote: > >> > On Mon, May 01, 2023 at 10:22:18PM -0400, James Bottomley wrote: > >> > > It is not used just for debug. It's used all over the kernel for > >> > > printing out device sizes. The output mostly goes to the kernel > >> > > print buffer, so it's anyone's guess as to what, if any, tools are > >> > > parsing it, but the concern about breaking log parsers seems to be > >> > > a valid one. > >> > > >> > Ok, there is sd_print_capacity() - but who in their right mind would > >> > be trying to scrape device sizes, in human readable units, > >> > >> If you bother to google "kernel log parser", you'll discover it's quite > >> an active area which supports a load of company business models. > > > > That doesn't mean log messages are unchangable ABI. Indeed, we had > > the whole "printk_index_emit()" addition recently to create > > an external index of printk message formats for such applications to > > use. [*] > > > >> > from log messages when it's available in sysfs/procfs (actually, is > >> > it in sysfs? if not, that's an oversight) in more reasonable units? > >> > >> It's not in sysfs, no. As aren't a lot of things, which is why log > >> parsing for system monitoring is big business. > > > > And that big business is why printk_index_emit() exists to allow > > them to easily determine how log messages change format and come and > > go across different kernel versions. > > > >> > Correct me if I'm wrong, but I've yet to hear about kernel log > >> > messages being consider a stable interface, and this seems a bit out > >> > there. > >> > >> It might not be listed as stable, but when it's known there's a large > >> ecosystem out there consuming it we shouldn't break it just because you > >> feel like it. > > > > But we've solved this problem already, yes? > > > > If the userspace applications are not using the kernel printk format > > index to detect such changes between kernel version, then they > > should be. This makes trivial issues like whether we have a space or > > not between units is completely irrelevant because the entry in the > > printk format index for the log output we emit will match whatever > > is output by the kernel.... > > If I understand that correctly from the commit changelog, this would have > indeed helped, but if the change was reflected in format string. But with > string_get_size() it's always an %s and the change of the helper's or a > switch to another variant of the helper that would omit the space, wouldn't > be reflected in the format string at all? I guess that would be an argument > for Andy's suggestion for adding a new %pt / %pT which would then be (Note, there is no respective %p extension for string_get_size() yet. %pt is for time and was used as an example when its evolution included a change like this) > reflected in the format string. And also more concise to use than using the > helper, fwiw. -- With Best Regards, Andy Shevchenko
