This adds options to printbuf for specifying whether units should be
printed raw (default) or with human readable units, and for controlling
whether human-readable units should be base 2 (default), or base 10.
This also adds new helpers that obey these options:
- pr_human_readable_u64
- pr_human_readable_s64
These obey printbuf->si_units
- pr_units_u64
- pr_units_s64
These obey both printbuf-human_readable_units and printbuf->si_units
Signed-off-by: Kent Overstreet <[email protected]>
---
include/linux/printbuf.h | 15 +++++++++++
lib/printbuf.c | 57 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 72 insertions(+)
diff --git a/include/linux/printbuf.h b/include/linux/printbuf.h
index 798e067457..c7919ae721 100644
--- a/include/linux/printbuf.h
+++ b/include/linux/printbuf.h
@@ -53,10 +53,19 @@
*
* Make sure you use pr_newline() instead of \n in the format string for indent
* level and tabstops to work corretly.
+ *
+ * Output units: printbuf->units exists to tell pretty-printers how to output
+ * numbers: a raw value (e.g. directly from a superblock field), as bytes, or as
+ * human readable bytes. pr_units() and pr_sectors() obey it.
*/
#include <linux/string.h>
+enum printbuf_si {
+ PRINTBUF_UNITS_2, /* use binary powers of 2^10 */
+ PRINTBUF_UNITS_10, /* use powers of 10^3 (standard SI) */
+};
+
struct printbuf {
char *buf;
unsigned size;
@@ -70,6 +79,8 @@ struct printbuf {
u8 atomic;
bool allocation_failure:1;
bool heap_allocated:1;
+ enum printbuf_si si_units:1;
+ bool human_readable_units:1;
u8 tabstop;
u8 tabstops[4];
};
@@ -83,6 +94,10 @@ void pr_indent_add(struct printbuf *, unsigned);
void pr_indent_sub(struct printbuf *, unsigned);
void pr_tab(struct printbuf *);
void pr_tab_rjust(struct printbuf *);
+void pr_human_readable_u64(struct printbuf *, u64);
+void pr_human_readable_s64(struct printbuf *, s64);
+void pr_units_u64(struct printbuf *, u64);
+void pr_units_s64(struct printbuf *, s64);
/* Initializer for a heap allocated printbuf: */
#define PRINTBUF ((struct printbuf) { .heap_allocated = true })
diff --git a/lib/printbuf.c b/lib/printbuf.c
index c9f730a215..b99d4b0dff 100644
--- a/lib/printbuf.c
+++ b/lib/printbuf.c
@@ -9,6 +9,7 @@
#endif
#include <linux/slab.h>
+#include <linux/string_helpers.h>
#include <linux/printbuf.h>
static inline size_t printbuf_linelen(struct printbuf *buf)
@@ -193,3 +194,59 @@ void pr_tab_rjust(struct printbuf *buf)
buf->tabstop++;
}
EXPORT_SYMBOL(pr_tab_rjust);
+
+/**
+ * pr_human_readable_u64 - Print out a u64 in human readable units
+ *
+ * Units of 2^10 (default) or 10^3 are controlled via @buf->si_units
+ */
+void pr_human_readable_u64(struct printbuf *buf, u64 v)
+{
+ printbuf_make_room(buf, 10);
+ buf->pos += string_get_size(v, 1, !buf->si_units,
+ buf->buf + buf->pos,
+ printbuf_remaining(buf));
+}
+EXPORT_SYMBOL(pr_human_readable_u64);
+
+/**
+ * pr_human_readable_s64 - Print out a s64 in human readable units
+ *
+ * Units of 2^10 (default) or 10^3 are controlled via @buf->si_units
+ */
+void pr_human_readable_s64(struct printbuf *buf, s64 v)
+{
+ if (v < 0)
+ pr_char(buf, '-');
+ pr_human_readable_u64(buf, abs(v));
+}
+EXPORT_SYMBOL(pr_human_readable_s64);
+
+/**
+ * pr_human_readable_u64 - Print out a u64 according to printbuf unit options
+ *
+ * Units are either raw (default), or human reabable units (controlled via
+ * @buf->human_readable_units)
+ */
+void pr_units_u64(struct printbuf *out, u64 v)
+{
+ if (out->human_readable_units)
+ pr_human_readable_u64(out, v);
+ else
+ pr_buf(out, "%llu", v);
+}
+EXPORT_SYMBOL(pr_units_u64);
+
+/**
+ * pr_human_readable_s64 - Print out a s64 according to printbuf unit options
+ *
+ * Units are either raw (default), or human reabable units (controlled via
+ * @buf->human_readable_units)
+ */
+void pr_units_s64(struct printbuf *out, s64 v)
+{
+ if (v < 0)
+ pr_char(out, '-');
+ pr_units_u64(out, v);
+}
+EXPORT_SYMBOL(pr_units_s64);
--
2.36.0
On Thu, May 19, 2022 at 11:21:41PM +0300, Andy Shevchenko wrote:
> On Thu, May 19, 2022 at 01:24:00PM -0400, Kent Overstreet wrote:
> > +void pr_human_readable_s64(struct printbuf *buf, s64 v)
> > +{
> > + if (v < 0)
> > + pr_char(buf, '-');
> > + pr_human_readable_u64(buf, abs(v));
>
> Wouldn't -v work?
This is a bit terser
> > + * pr_human_readable_u64 - Print out a u64 according to printbuf unit options
>
> Have you ever compile this? We have kernel doc validator running when compiling
> the code...
Yes I have, but I've never seen the kernel doc validator - can you point me to
something so I can get that working?
> > +void pr_units_s64(struct printbuf *out, s64 v)
> > +{
> > + if (v < 0)
> > + pr_char(out, '-');
> > + pr_units_u64(out, v);
>
> Please, start with test cases.
I suppose now that we've got kunit I should learn how to use it :) And thanks,
good catch
On Fri, May 20, 2022 at 12:16:41AM +0300, Andy Shevchenko wrote:
> I run `make W=1`, but I think the kernel doc validator is run anyway.
No, it's too slow and used to emit too many warnings to run it at any
time. That's why I made it opt-in with W=1 (and even that caused
screeches from people who were compiling with W=1 in their CI frameworks
;-)
On Fri, May 20, 2022 at 12:16:41AM +0300, Andy Shevchenko wrote:
> On Thu, May 19, 2022 at 04:26:26PM -0400, Kent Overstreet wrote:
> > On Thu, May 19, 2022 at 11:21:41PM +0300, Andy Shevchenko wrote:
> > > On Thu, May 19, 2022 at 01:24:00PM -0400, Kent Overstreet wrote:
>
> ...
>
> > > > + if (v < 0)
> > > > + pr_char(buf, '-');
> > > > + pr_human_readable_u64(buf, abs(v));
> > >
> > > Wouldn't -v work?
> >
> > This is a bit terser
>
> Have you checked assembly? Basically the question here is does the compiler see
> the same conditional and avoid duplicating it?
No. I check the assembly for code where it matters - for this, I'm going to
write it the way I find it most readable.
> I run `make W=1`, but I think the kernel doc validator is run anyway.
Got it, and fixed the doc warnings.
On Thu, May 19, 2022 at 04:26:26PM -0400, Kent Overstreet wrote:
> On Thu, May 19, 2022 at 11:21:41PM +0300, Andy Shevchenko wrote:
> > On Thu, May 19, 2022 at 01:24:00PM -0400, Kent Overstreet wrote:
> > > +void pr_human_readable_s64(struct printbuf *buf, s64 v)
> > > +{
> > > + if (v < 0)
> > > + pr_char(buf, '-');
> > > + pr_human_readable_u64(buf, abs(v));
> >
> > Wouldn't -v work?
>
> This is a bit terser
How about:
if (v < 0) {
pr_char(buf, '-');
v = -v;
}
pr_human_readable_u64(buf, v);
(some pedantic compilers might warn about the behaviour of S64_MIN, but
I think we're OK)
> > > + * pr_human_readable_u64 - Print out a u64 according to printbuf unit options
> >
> > Have you ever compile this? We have kernel doc validator running when compiling
> > the code...
>
> Yes I have, but I've never seen the kernel doc validator - can you point me to
> something so I can get that working?
You have to run 'make W=1' to get it, so Andy is being a bit overly
harsh here. Most people use neither W=1, nor C=1.
On Thu, May 19, 2022 at 10:11:40PM +0100, Matthew Wilcox wrote:
> How about:
>
> if (v < 0) {
> pr_char(buf, '-');
> v = -v;
> }
> pr_human_readable_u64(buf, v);
>
> (some pedantic compilers might warn about the behaviour of S64_MIN, but
> I think we're OK)
Yes, since S64_MAX == -S64_MIN and S64_MIN == S64_MAX + 1, the code is correct
for v == S64_MIN.
But I still prefer my way :)
On Thu, May 19, 2022 at 04:26:26PM -0400, Kent Overstreet wrote:
> On Thu, May 19, 2022 at 11:21:41PM +0300, Andy Shevchenko wrote:
> > On Thu, May 19, 2022 at 01:24:00PM -0400, Kent Overstreet wrote:
...
> > > + if (v < 0)
> > > + pr_char(buf, '-');
> > > + pr_human_readable_u64(buf, abs(v));
> >
> > Wouldn't -v work?
>
> This is a bit terser
Have you checked assembly? Basically the question here is does the compiler see
the same conditional and avoid duplicating it?
...
> > > + * pr_human_readable_u64 - Print out a u64 according to printbuf unit options
> >
> > Have you ever compile this? We have kernel doc validator running when compiling
> > the code...
>
> Yes I have, but I've never seen the kernel doc validator - can you point me to
> something so I can get that working?
I run `make W=1`, but I think the kernel doc validator is run anyway.
You always can run it manually
scripts/kernel-doc -v -none lib/printbuf.c
--
With Best Regards,
Andy Shevchenko
On Thu, May 19, 2022 at 01:24:00PM -0400, Kent Overstreet wrote:
> This adds options to printbuf for specifying whether units should be
> printed raw (default) or with human readable units, and for controlling
> whether human-readable units should be base 2 (default), or base 10.
>
> This also adds new helpers that obey these options:
>
> - pr_human_readable_u64
> - pr_human_readable_s64
> These obey printbuf->si_units
>
> - pr_units_u64
> - pr_units_s64
> These obey both printbuf-human_readable_units and printbuf->si_units
...
> +void pr_human_readable_s64(struct printbuf *buf, s64 v)
> +{
> + if (v < 0)
> + pr_char(buf, '-');
> + pr_human_readable_u64(buf, abs(v));
Wouldn't -v work?
> +}
...
> + * pr_human_readable_u64 - Print out a u64 according to printbuf unit options
Have you ever compile this? We have kernel doc validator running when compiling
the code...
> + * Units are either raw (default), or human reabable units (controlled via
> + * @buf->human_readable_units)
> + */
> +void pr_units_u64(struct printbuf *out, u64 v)
> +{
> + if (out->human_readable_units)
> + pr_human_readable_u64(out, v);
> + else
> + pr_buf(out, "%llu", v);
> +}
> +EXPORT_SYMBOL(pr_units_u64);
> +
> +/**
> + * pr_human_readable_s64 - Print out a s64 according to printbuf unit options
Ditto.
> + * Units are either raw (default), or human reabable units (controlled via
> + * @buf->human_readable_units)
> + */
> +void pr_units_s64(struct printbuf *out, s64 v)
> +{
> + if (v < 0)
> + pr_char(out, '-');
> + pr_units_u64(out, v);
Please, start with test cases.
> +}
--
With Best Regards,
Andy Shevchenko