Re: [PATCH v4 02/28] printk: Add print format (%pra) for struct range

Fan Ni
2024-10-09T17:33:47Z

On Mon, Oct 07, 2024 at 06:16:08PM -0500, Ira Weiny wrote:
> The use of struct range in the CXL subsystem is growing. In particular,
> the addition of Dynamic Capacity devices uses struct range in a number
> of places which are reported in debug and error messages.
>
> To wit requiring the printing of the start/end fields in each print
> became cumbersome. Dan Williams mentions in [1] that it might be time
> to have a print specifier for struct range similar to struct resource
>
> A few alternatives were considered including '%par', '%r', and '%pn'.
> %pra follows that struct range is similar to struct resource (%p[rR])
> but need to be different. Based on discussions with Petr and Andy
> '%pra' was chosen.[2]
>
> Andy also suggested to keep the range prints similar to struct resource
> though combined code. Add hex_range() to handle printing for both
> pointer types.
>
> To: Petr Mladek <pmladek@suse.com>
> To: Steven Rostedt <rostedt@goodmis.org>
> To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> To: Sergey Senozhatsky <senozhatsky@chromium.org>
> To: Jonathan Corbet <corbet@lwn.net> (maintainer:DOCUMENTATION)
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org (open list)
> Link: https://lore.kernel.org/all/663922b475e50_d54d72945b@dwillia2-xfh.jf.intel.com.notmuch/ [1]
> Link: https://lore.kernel.org/all/66cea3bf3332f_f937b29424@iweiny-mobl.notmuch/ [2]
> Suggested-by: "Dan Williams" <dan.j.williams@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>
> ---
> Changes:
> [Andy: create new hex_range() and use it in both range/resource]
> [Petr/Andy: Use %pra]
> [Andy: Add test case start > end]
> [Petr: Update documentation]
> [Petr: use 'range -']
> [Petr: fixup printf_spec specifiers]
> [Petr: add lib/test_printf test]
> ---
> Documentation/core-api/printk-formats.rst | 13 ++++++++
> lib/test_printf.c | 26 +++++++++++++++
> lib/vsprintf.c | 55 +++++++++++++++++++++++++++----
> 3 files changed, 88 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index 14e093da3ccd..03b102fc60bb 100644
> --- a/Documentation/core-api/printk-formats.rst
> +++ b/Documentation/core-api/printk-formats.rst
> @@ -231,6 +231,19 @@ width of the CPU data path.
>
> Passed by reference.
>
> +Struct Range
> +------------
> +
> +::
> +
> + %pra [range 0x0000000060000000-0x000000006fffffff]
> + %pra [range 0x0000000060000000]
> +
> +For printing struct range. struct range holds an arbitrary range of u64
> +values. If start is equal to end only 1 value is printed.
> +
> +Passed by reference.
> +
> DMA address types dma_addr_t
> ----------------------------
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 5afdf5efc627..e3e75b6d10a0 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -432,6 +432,31 @@ struct_resource(void)
> "%pR", &test_resource);
> }
>
> +static void __init
> +struct_range(void)
> +{
> + struct range test_range = {
> + .start = 0xc0ffee00ba5eba11,
> + .end = 0xc0ffee00ba5eba11,
> + };
> +
> + test("[range 0xc0ffee00ba5eba11]", "%pra", &test_range);
> +
> + test_range = (struct range) {
> + .start = 0xc0ffee,
> + .end = 0xba5eba11,
> + };
> + test("[range 0x0000000000c0ffee-0x00000000ba5eba11]",
> + "%pra", &test_range);
> +
> + test_range = (struct range) {
> + .start = 0xba5eba11,
> + .end = 0xc0ffee,
> + };
> + test("[range 0x00000000ba5eba11-0x0000000000c0ffee]",
> + "%pra", &test_range);
> +}
> +
...
> static noinline_for_stack
> char *hex_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
> const char *fmt)
> @@ -2277,6 +2314,7 @@ char *rust_fmt_argument(char *buf, char *end, void *ptr);
> * - 'Bb' as above with module build ID (for use in backtraces)
> * - 'R' For decoded struct resource, e.g., [mem 0x0-0x1f 64bit pref]
> * - 'r' For raw struct resource, e.g., [mem 0x0-0x1f flags 0x201]
> + * - 'ra' struct ranges [range 0x00 - 0xff]

Maybe follow the existing examples here, like
'ra" For struct ranges, e.g., ...

fan

> * - 'b[l]' For a bitmap, the number of bits is determined by the field
> * width which must be explicitly specified either as part of the
> * format string '%32b[l]' or through '%*b[l]', [l] selects
> @@ -2399,8 +2437,13 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
> fallthrough;
> case 'B':
> return symbol_string(buf, end, ptr, spec, fmt);
> - case 'R':
> case 'r':
> + switch (fmt[1]) {
> + case 'a':
> + return range_string(buf, end, ptr, spec, fmt);
> + }
> + fallthrough;
> + case 'R':
> return resource_string(buf, end, ptr, spec, fmt);
> case 'h':
> return hex_string(buf, end, ptr, spec, fmt);
>
> --
> 2.46.0
>

--
Fan Ni