Messages in this thread | | | From | "Matthew Wilcox (Oracle)" <> | Subject | [PATCH v5 00/32] Printbufs | Date | Mon, 8 Aug 2022 03:40:56 +0100 |
| |
[all this code is from Kent, I'm helping him out because he's having trouble with git send-email and OAUTH]
This should (hopefully) be the final iteration of this patch series.
Previous discussion: https://lore.kernel.org/linux-mm/20220620004233.3805-1-kent.overstreet@gmail.com/
Changes since v4:
- %pf(%p) format extension has been dropped for now, since it turned out to be a bit controversial. I think we're going to want it, but I'm leaving it for a future patch series. - There was a small off by one error in the patch converting trace_events_synth. The original code using seq_buf had to calculate the size of the string to allocate; since printbufs have native support for heap allocating strings, the version of the patch in this series just uses that for a nice cleanup.
What are printbufs, and why? ============================
Printbufs are like the existing seq_buf, with some differences and new features:
- Heap allocation (optional) Currently, we have no standard mechanism for printing to a heap-allocated string: code that needs to do this must calculate the size of the buffer to allocate, which is tedious and error prone. IOW, this is a major ergonomic improvement.
- Indent level
Any time we're printing structured multi-line data, proper indenting makes the output considerably more readable. Printbufs have state for the current indent level, controlled by printbuf_indent_add() and printbuf_indent_sub() and obeyed by prt_newline().
In the future this may work with just \n, pending work to do this without performance regressions.
- Tab stops
Printing reports with nicely aligned columns is something we do a _lot_, and printbufs make this a lot easier. After setting tabstops (byte offsets from start of line), prt_tab() will output spaces up to the next tabstop, and pr_tab_rjust() will right justify output since the previous output against the next tabstap.
In the future this may work with just \t, pending work to do this without performance regressions.
Why a new data structure, instead of extending seq_buf? =======================================================
Some semantic changes were required. The main ones were:
- printbufs have different overflow semantics vs. seq_buf, to support snprintf() which is required to return the number of characters that were written even when an empty buffer is passed
- seq_buf has a readpos member which printbuf drops, since it's only for tracing.
Also, a number of the existing seq_buf users were converted to printbuf's heap allocation functionality, replacing open coded string size calculations and allocations.
Given all this, adding the new thing and converting existing users one by one felt like the more incremental/bisectable approach.
Goals of this patch series: ===========================
- A unified API for functions that output strings (i.e. pretty printers).
Currently, we've got a bit of a mess of different calling conventions for functions that output strings. There's sprintf/snprintf style, seq_buf, and various other variations of passing raw character pointers around.
- Low level helpers for building up strings (prt_char(), prt_str(), etc.).
This replaces a _lot_ of low level pointer arithmetic in vsprintf.c, and elsewhere, with proper helpers.
- Cleanup of pretty-printers in vsprintf.c
Currently, we have a lot of pretty-printers in vsprintf.c invoked by various %p extensions. However, arguments to those pretty printers are passed as additional format string characters, instead of normal function arguments, and we have the code that decodes format strings mixed with pretty-printer code, which is not great for readability and makes those pretty-printers unnecessarily tied to vsprintf.c internal details.
One of my goals is to give those pretty-printers normal function call interfaces, and separate out the format-string parsing: this patch series does some of the prep work. I've got more patches in progress in that vein, but this patch series is big enough as is so I'm leaving them for later.
Kent Overstreet (32): lib/printbuf: New data structure for printing strings lib/string_helpers: Convert string_escape_mem() to printbuf vsprintf: Convert to printbuf lib/hexdump: Convert to printbuf lib/string_helpers: string_get_size() now returns characters wrote lib/printbuf: Heap allocation lib/printbuf: Tabstops, indenting lib/printbuf: Unit specifiers vsprintf: Improve number() vsprintf: prt_u64_minwidth(), prt_u64() test_printf: Drop requirement that sprintf not write past nul vsprintf: Start consolidating printf_spec handling vsprintf: Refactor resource_string() vsprintf: Refactor fourcc_string() vsprintf: Refactor ip_addr_string() vsprintf: Refactor mac_address_string() vsprintf: time_and_date() no longer takes printf_spec vsprintf: flags_string() no longer takes printf_spec vsprintf: Refactor device_node_string, fwnode_string vsprintf: Refactor hex_string, bitmap_string_list, bitmap_string Input/joystick/analog: Convert from seq_buf -> printbuf mm/memcontrol.c: Convert to printbuf clk: tegra: bpmp: Convert to printbuf tools/testing/nvdimm: Convert to printbuf powerpc: Convert to printbuf x86/resctrl: Convert to printbuf PCI/P2PDMA: Convert to printbuf tracing: trace_events_synth: Convert to printbuf d_path: prt_path() ACPI/APEI: Add missing include tracing: Convert to printbuf Delete seq_buf
arch/powerpc/kernel/process.c | 16 +- arch/powerpc/kernel/security.c | 75 +- arch/powerpc/platforms/pseries/papr_scm.c | 34 +- arch/x86/kernel/cpu/resctrl/rdtgroup.c | 16 +- drivers/acpi/apei/erst-dbg.c | 1 + drivers/clk/tegra/clk-bpmp.c | 21 +- drivers/input/joystick/analog.c | 23 +- drivers/pci/p2pdma.c | 21 +- fs/d_path.c | 35 + include/linux/dcache.h | 1 + include/linux/kernel.h | 12 + include/linux/printbuf.h | 257 ++++ include/linux/seq_buf.h | 162 -- include/linux/string.h | 5 + include/linux/string_helpers.h | 8 +- include/linux/trace_events.h | 2 +- include/linux/trace_seq.h | 17 +- kernel/trace/trace.c | 45 +- kernel/trace/trace_dynevent.c | 34 +- kernel/trace/trace_events_filter.c | 2 +- kernel/trace/trace_events_synth.c | 53 +- kernel/trace/trace_functions_graph.c | 6 +- kernel/trace/trace_kprobe.c | 2 +- kernel/trace/trace_seq.c | 111 +- lib/Makefile | 4 +- lib/hexdump.c | 246 ++-- lib/printbuf.c | 258 ++++ lib/seq_buf.c | 397 ----- lib/string_helpers.c | 224 +-- lib/test_hexdump.c | 30 +- lib/test_printf.c | 6 - lib/vsprintf.c | 1640 ++++++++++----------- mm/memcontrol.c | 54 +- tools/testing/nvdimm/test/ndtest.c | 22 +- 34 files changed, 1855 insertions(+), 1985 deletions(-) create mode 100644 include/linux/printbuf.h delete mode 100644 include/linux/seq_buf.h create mode 100644 lib/printbuf.c delete mode 100644 lib/seq_buf.c
-- 2.35.1
| |