From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id DDDF8C433EF for ; Mon, 20 Jun 2022 04:19:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 518A88D0001; Mon, 20 Jun 2022 00:19:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4C6ED6B0074; Mon, 20 Jun 2022 00:19:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 38F888D0001; Mon, 20 Jun 2022 00:19:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 26A4A6B0073 for ; Mon, 20 Jun 2022 00:19:38 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id EC80460718 for ; Mon, 20 Jun 2022 04:19:37 +0000 (UTC) X-FDA: 79597310394.08.1327B61 Received: from eu-smtp-delivery-151.mimecast.com (eu-smtp-delivery-151.mimecast.com [185.58.86.151]) by imf24.hostedemail.com (Postfix) with ESMTP id 4F308180018 for ; Mon, 20 Jun 2022 04:19:36 +0000 (UTC) Received: from AcuMS.aculab.com (156.67.243.121 [156.67.243.121]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id uk-mta-143-xc5O7tanPo2ikZ1Vp4r_Kw-1; Mon, 20 Jun 2022 05:19:34 +0100 X-MC-Unique: xc5O7tanPo2ikZ1Vp4r_Kw-1 Received: from AcuMS.Aculab.com (fd9f:af1c:a25b:0:994c:f5c2:35d6:9b65) by AcuMS.aculab.com (fd9f:af1c:a25b:0:994c:f5c2:35d6:9b65) with Microsoft SMTP Server (TLS) id 15.0.1497.36; Mon, 20 Jun 2022 05:19:31 +0100 Received: from AcuMS.Aculab.com ([fe80::994c:f5c2:35d6:9b65]) by AcuMS.aculab.com ([fe80::994c:f5c2:35d6:9b65%12]) with mapi id 15.00.1497.036; Mon, 20 Jun 2022 05:19:31 +0100 From: David Laight To: 'Kent Overstreet' , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "pmladek@suse.com" CC: "rostedt@goodmis.org" , "enozhatsky@chromium.org" , "linux@rasmusvillemoes.dk" , "willy@infradead.org" Subject: RE: [PATCH v4 00/34] Printbufs - new data structure for building strings Thread-Topic: [PATCH v4 00/34] Printbufs - new data structure for building strings Thread-Index: AQHYhD6oUJ9HZdIC7US1XHhH4yp3Qa1XsEIg Date: Mon, 20 Jun 2022 04:19:31 +0000 Message-ID: <0a5901f8460f452a89c9b0cda32fb833@AcuMS.aculab.com> References: <20220620004233.3805-1-kent.overstreet@gmail.com> In-Reply-To: <20220620004233.3805-1-kent.overstreet@gmail.com> Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.202.205.107] MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: aculab.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1655698777; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=cUK704oUmo6r2XW9bhzyw1gg8ktfGkHuWkV8V/49BhE=; b=4oO4zew84suUb7jj/9Ik0LxKLGd/M3cI+VYB6p8zKlfo2j1Zv1o0CWvmGYG7R0s/8QOAYT S2aCB4ixcYMu4DGoBCsFrZGt0L4wA/ScTJUe+Gcjl/Evxlqgp9+h1CN01KEFrWFU+6sUMB rvA1T1oH2x4sduR3HD3dOK1AB79dTEA= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=none; spf=pass (imf24.hostedemail.com: domain of david.laight@aculab.com designates 185.58.86.151 as permitted sender) smtp.mailfrom=david.laight@aculab.com; dmarc=pass (policy=none) header.from=aculab.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1655698777; a=rsa-sha256; cv=none; b=PJ4Dwl9QGvpIHCiIjtQcrzBQJSqevC3gDhUFcvbmRmUOXPMntlomwHK4RHjO/PyqrEM03X oHzL1XrmnnymE9Fg2Eso0Ru+qW7LeYc68KUuti8QPmUOhFQFmUykKWlHST1ONLva8E53Sv mORa2kLcB9rT5EGufNzygsP1ml8DEiI= X-Stat-Signature: wie1k5rt8jwkthq1ty5fd9hweimiethg X-Rspamd-Queue-Id: 4F308180018 X-Rspamd-Server: rspam11 X-Rspam-User: Authentication-Results: imf24.hostedemail.com; dkim=none; spf=pass (imf24.hostedemail.com: domain of david.laight@aculab.com designates 185.58.86.151 as permitted sender) smtp.mailfrom=david.laight@aculab.com; dmarc=pass (policy=none) header.from=aculab.com X-HE-Tag: 1655698776-669038 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: From: Kent Overstreet > Sent: 20 June 2022 01:42 >=20 > Previous discussions: > https://lore.kernel.org/all/20220419203202.2670193-1-kent.overstreet@gmai= l.com/ > https://lore.kernel.org/all/20220519172421.162394-1-kent.overstreet@gmail= .com/ > https://lore.kernel.org/all/20220604193042.1674951-1-kent.overstreet@gmai= l.com/ >=20 > Git repo: > https://evilpiepirate.org/git/bcachefs.git/log/?h=3Dprintbuf_v4 >=20 > Changes since v3: > Bugfixes and performance improvements, the latest iteration of this pat= ch > series has been baking in the bcachefs tree and that shook out some bug= s. >=20 > Rasmus pointed out that -fno-strict-aliasing is going to cause gcc to g= enerate > nasty code, and indeed it unfortunately does but according to worst cas= e > scenario microbenchmarks it's not a problem for actual performance. Just copy some of the structure members to local variables and, if necessary, write them back at the end. > Using > memcpy() and memset() in the printbuf helpers _was_ a problem for perfo= rmance, > so that's been fixed. >=20 > ----------- >=20 > Core idea: Wouldn't it be nice if we had a common data structure and call= ing > convention for outputting strings? >=20 > The core concept this patch series is aimed at cleaning up and standardiz= ing is > that of a "pretty-printer", which is now a function like prt_foo() or > foo_to_text(): >=20 > void foo_to_text(struct printbuf *out, struct foo) >=20 > What this patch series does or enables: >=20 > - It becomes quite a bit easier to write composable pretty printers! Thi= s is > huge. >=20 > - A ton of code that works in terms of raw char * pointers and lengths > (snprintf style, and many weird variations) gets cleaned up, with erro= r prone > raw pointers arithmetic replaced by proper helpers >=20 > - A ton of code that emits either directly via printk() or to other plac= es > (sysfs, debugfs) can now output to printbufs, and becomes more reusabl= e and > composable >=20 > - Countesy of Matthew Wilcox, the new and very cool %pf() format string,= which > allows passing a pretty printer function and its arguments to sprintf(= ) and > family. This means we can now call type specific pretty-printers witho= ut > adding them to lib/vsprintf.c and writing a bunch of crazy > parsing-and-dispatch code. For example, >=20 > printk("%pd", dentry); >=20 > becomes >=20 > printk("%pf(%p)", prt_dentry, dentry); >=20 > My OOM debugging & reporting patch series that builds off of this uses= this > to solve a very real problem that Michal Hocko brought up at LSF - wit= h this > we write shrinkers_to_text(), slab_to_text() which can _also_ now be u= sed for > reporting in debugfs (which Roman has been working on), as well as in = the > show_mem() report - the "%pf()" syntax lets us print the output of tho= se > functions without allocating (and having to preallocate) a separate bu= ffer. I really think that is a bad idea. printk() already uses a lot of stack, anything doing a recursive call is just making that worse. Especially since these calls can often be in error paths which are not often tested and can already be on deep stacks. =09David >=20 > - Some new formatting helpers: >=20 > Nicely aligned text is much easier to read, and something that we want= a > _lot, but outputting nicely aligned text with printf() is a pain in th= e ass. > Printbufs add tabstops, which can be used for right or left justificat= ion - > simple, easy. prt_tab() emits spaces up to the next tabstop, prt_tab_r= just() > advances to the next tabstop right justifying text since the previous > tabstop. >=20 > Printbufs also add an indent level, obeyed by prt_newline() which can = be very > useful for multi line output. >=20 > In the future, \n and \t in format strings may learn to obey these as = well. >=20 > - Optional heap allocation - no need to statically allocate buffers on t= he > stack and guess at the output size. >=20 > - Lots of consolidating and refactoring >=20 > This series replaces seq_buf, which does basically what an earlier ver= sion of > printbufs did. >=20 > A good chunk of lib/string_helpers.c, as well as lib/hexdump.c are con= verted > (and simplified!). >=20 > Pretty printers in lib/vsprintf.c previously outputted to buffers on t= he > stack and then copied _that_ to the actual output buffer, that's all g= one > (replaced by proper helpers for outputting chars and strings), and the= y also > used printf_spec for argument passing in ad-hoc ways. This patch serie= s does > a lot towards converting them to more standard pretty printers that ca= n be > called via %pf() instead of having to live in lib/vsprintf.c. Still to= do: > format string decoding for argument passing is a mess that's scattered= all > over the place. >=20 > In the course of working on this patch series, I've spotted a _lot_ more > consolidation and refactoring that needs to be done - we've got a ton of = API > fragmentation leading to lots of code duplication. >=20 > But I'm already really excited about what this patch series enables. >=20 > Cheers! >=20 > Kent Overstreet (34): > 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 > vsprintf: %pf(%p) > lib/string_helpers: string_get_size() now returns characters wrote > lib/printbuf: Heap allocation > lib/printbuf: Tabstops, indenting > lib/printbuf: Unit specifiers > lib/pretty-printers: prt_string_option(), prt_bitflags() > 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 >=20 > Documentation/core-api/printk-formats.rst | 22 + > 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/pretty-printers.h | 10 + > include/linux/printbuf.h | 253 +++ > 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 | 32 +- > 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/pretty-printers.c | 60 + > lib/printbuf.c | 253 +++ > lib/seq_buf.c | 397 ----- > lib/string_helpers.c | 224 +-- > lib/test_hexdump.c | 30 +- > lib/test_printf.c | 33 +- > lib/vsprintf.c | 1723 ++++++++++----------- > mm/memcontrol.c | 68 +- > tools/testing/nvdimm/test/ndtest.c | 22 +- > 37 files changed, 2050 insertions(+), 1976 deletions(-) > create mode 100644 include/linux/pretty-printers.h > create mode 100644 include/linux/printbuf.h > delete mode 100644 include/linux/seq_buf.h > create mode 100644 lib/pretty-printers.c > create mode 100644 lib/printbuf.c > delete mode 100644 lib/seq_buf.c >=20 > -- > 2.36.1 - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1= PT, UK Registration No: 1397386 (Wales)