From: Dan Williams <dan.j.williams@intel.com>
To: Ira Weiny <ira.weiny@intel.com>,
Andrew Morton <akpm@linux-foundation.org>,
Petr Mladek <pmladek@suse.com>,
Steven Rostedt <rostedt@goodmis.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Sergey Senozhatsky <senozhatsky@chromium.org>,
Jonathan Corbet <corbet@lwn.net>,
Davidlohr Bueso <dave@stgolabs.net>,
Jonathan Cameron <jonathan.cameron@huawei.com>,
"Dave Jiang" <dave.jiang@intel.com>,
Alison Schofield <alison.schofield@intel.com>,
Vishal Verma <vishal.l.verma@intel.com>,
Dan Williams <dan.j.williams@intel.com>
Cc: Fan Ni <fan.ni@samsung.com>, Bagas Sanjaya <bagasdotme@gmail.com>,
<linux-kernel@vger.kernel.org>, <linux-doc@vger.kernel.org>,
<linux-cxl@vger.kernel.org>, Ira Weiny <ira.weiny@intel.com>
Subject: Re: [PATCH 2/3] printf: Add print format (%pra) for struct range
Date: Fri, 18 Oct 2024 13:05:22 -0700 [thread overview]
Message-ID: <6712bf8240b8d_10a03294a6@dwillia2-mobl3.amr.corp.intel.com.notmuch> (raw)
In-Reply-To: <20241018-cxl-pra-v1-2-7f49ba58208b@intel.com>
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 needs 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.
>
> Finally introduce DEFINE_RANGE() as a parallel to DEFINE_RES_*() and use
> it in the tests.
>
> Cc: 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:
> [iweiny: split from CXL patch set]
> [Andy: Define DEFINE_RANGE()]
> [Andy: rework hex_range() logic]
> [Andy: Don't use fallthrough]
> [Andy: clean up help comment]
> [Fan: clean up printk comment]
> [Rasmus: fix buf processing in hex_range()]
> [Rasmus: simplify buffer sizing]
> [Rasmus: add '[' to string]
> [Bagas: Clarify the start == end print behavior]
> ---
> Documentation/core-api/printk-formats.rst | 13 +++++++
> include/linux/range.h | 6 ++++
> lib/test_printf.c | 17 +++++++++
> lib/vsprintf.c | 57 +++++++++++++++++++++++++++----
> 4 files changed, 87 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
> index 14e093da3ccd..e1ebf0376154 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 print the start value.
I was going to say "why this special case that does not exist for the
%pr case?", but then checked the code and found it *does* do this for %pr.
So if you're going to document this special case for %pra might as well
update the documentation for %pr too.
Alternatively, drop the new %pra documentation for this corner case as
accommodating the U64_MAX size range case is arguably a mistake in the
caller.
Either way, just make it consistent.
next prev parent reply other threads:[~2024-10-18 20:05 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-18 19:46 [PATCH 0/3] printf: Add struct range print specifier Ira Weiny
2024-10-18 19:46 ` [PATCH 1/3] test printf: Add very basic struct resource tests Ira Weiny
2024-10-21 15:31 ` Petr Mladek
2024-10-18 19:46 ` [PATCH 2/3] printf: Add print format (%pra) for struct range Ira Weiny
2024-10-18 20:05 ` Dan Williams [this message]
2024-10-21 2:49 ` Ira Weiny
2024-10-21 14:30 ` Petr Mladek
2024-10-21 19:04 ` Ira Weiny
2024-10-21 6:57 ` Andy Shevchenko
2024-10-21 7:07 ` Andy Shevchenko
2024-10-18 19:46 ` [PATCH 3/3] cxl/cdat: Use %pra for dpa range outputs Ira Weiny
2024-10-22 1:40 ` Alison Schofield
2024-10-23 18:19 ` Ira Weiny
2024-10-24 2:14 ` Alison Schofield
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6712bf8240b8d_10a03294a6@dwillia2-mobl3.amr.corp.intel.com.notmuch \
--to=dan.j.williams@intel.com \
--cc=akpm@linux-foundation.org \
--cc=alison.schofield@intel.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=bagasdotme@gmail.com \
--cc=corbet@lwn.net \
--cc=dave.jiang@intel.com \
--cc=dave@stgolabs.net \
--cc=fan.ni@samsung.com \
--cc=ira.weiny@intel.com \
--cc=jonathan.cameron@huawei.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=senozhatsky@chromium.org \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®