Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH v1 0/2] PCI: Include human-readable BAR and window sizes
@ 2026-08-08 17:23 Bjorn Helgaas
  2026-08-08 17:23 ` [PATCH v1 1/2] vsprintf: Add %pR human-readable size Bjorn Helgaas
  2026-08-08 17:23 ` [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages Bjorn Helgaas
  0 siblings, 2 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2026-08-08 17:23 UTC (permalink / raw)
  To: Ilpo Järvinen, Andrew Morton, Petr Mladek, Steven Rostedt
  Cc: Andy Shevchenko, Rasmus Villemoes, Sergey Senozhatsky, linux-pci,
	linux-kernel, Bjorn Helgaas

PCI and other subsystems often use %pR to decode and print memory ranges.
The size of the range is useful to know but hard to read off at a glance,
e.g., 

  BAR 0 [mem 0xfea80000-0xfea9ffff]

Add more user-friendly size information to the %pR format, e.g.,

  BAR 0 [mem 0xfea80000-0xfea9ffff (128 KiB)]

%pR is also used to print I/O port, bus number, and DMA resources, where
the size is not as relevant, so this only applies to IORESOURCE_MEM
resources.

Bjorn Helgaas (2):
  vsprintf: Add %pR human-readable size
  PCI: Include human-readable sizes in resource assignment messages


vfio_pci_info_atomic_cap
 drivers/pci/setup-bus.c | 46 +++++++++++++++++++++++++++++------------
 drivers/pci/setup-res.c | 11 ++++++----
 lib/vsprintf.c          | 18 ++++++++++++----
 3 files changed, 54 insertions(+), 21 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v1 1/2] vsprintf: Add %pR human-readable size
  2026-08-08 17:23 [PATCH v1 0/2] PCI: Include human-readable BAR and window sizes Bjorn Helgaas
@ 2026-08-08 17:23 ` Bjorn Helgaas
  2026-08-08 17:38   ` sashiko-bot
  2026-08-08 17:43   ` Andy Shevchenko
  2026-08-08 17:23 ` [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages Bjorn Helgaas
  1 sibling, 2 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2026-08-08 17:23 UTC (permalink / raw)
  To: Ilpo Järvinen, Andrew Morton, Petr Mladek, Steven Rostedt
  Cc: Andy Shevchenko, Rasmus Villemoes, Sergey Senozhatsky, linux-pci,
	linux-kernel, Bjorn Helgaas

Include human-readable size when printing struct resource memory ranges
(not including DMA, bus number, or I/O port ranges) with the %pR format to
make it easier to read, e.g.,

  - pci 0000:00:02.0: BAR 0 [mem 0xfea80000-0xfea9ffff]
  + pci 0000:00:02.0: BAR 0 [mem 0xfea80000-0xfea9ffff (128 KiB)]

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 lib/vsprintf.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 2bc6ef483576..c49044b6dbee 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1094,15 +1094,17 @@ char *resource_string(char *buf, char *end, struct resource *res,
 	/* 32-bit res (sizeof==4): 10 chars in dec, 10 in hex ("0x" + 8)
 	 * 64-bit res (sizeof==8): 20 chars in dec, 18 in hex ("0x" + 16) */
 #define RSRC_BUF_SIZE		((2 * sizeof(resource_size_t)) + 4)
+#define RSRC_STR_SIZE		sizeof(" (xxxx.xxx MiB)")
 #define FLAG_BUF_SIZE		(2 * sizeof(res->flags))
-#define DECODED_BUF_SIZE	sizeof("[mem - 64bit pref window disabled]")
-#define RAW_BUF_SIZE		sizeof("[mem - flags 0x]")
-	char sym[MAX(2*RSRC_BUF_SIZE + DECODED_BUF_SIZE,
-		     2*RSRC_BUF_SIZE + FLAG_BUF_SIZE + RAW_BUF_SIZE)];
+#define DECODED_BUF_SIZE	sizeof("[mem - () 64bit pref window disabled]")
+#define RAW_BUF_SIZE		sizeof("[mem - () flags 0x]")
+	char sym[MAX(2*RSRC_BUF_SIZE + RSRC_STR_SIZE + DECODED_BUF_SIZE,
+		     2*RSRC_BUF_SIZE + RSRC_STR_SIZE + FLAG_BUF_SIZE + RAW_BUF_SIZE)];
 
 	char *p = sym, *pend = sym + sizeof(sym);
 	bool decode = fmt[0] == 'R';
 	const struct printf_spec *specp;
+	char size_buf[32];
 
 	if (check_pointer(&buf, end, res, spec))
 		return buf;
@@ -1134,6 +1136,14 @@ char *resource_string(char *buf, char *end, struct resource *res,
 	} else {
 		p = hex_range(p, pend, res->start, res->end, *specp);
 	}
+	if (res->flags & IORESOURCE_MEM) {
+		*p++ = ' ';
+		*p++ = '(';
+		string_get_size(resource_size(res), 1, STRING_UNITS_2,
+				size_buf, sizeof(size_buf));
+		p = string_nocheck(p, pend, size_buf, str_spec);
+		*p++ = ')';
+	}
 	if (decode) {
 		if (res->flags & IORESOURCE_MEM_64)
 			p = string_nocheck(p, pend, " 64bit", str_spec);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages
  2026-08-08 17:23 [PATCH v1 0/2] PCI: Include human-readable BAR and window sizes Bjorn Helgaas
  2026-08-08 17:23 ` [PATCH v1 1/2] vsprintf: Add %pR human-readable size Bjorn Helgaas
@ 2026-08-08 17:23 ` Bjorn Helgaas
  2026-08-08 17:27   ` sashiko-bot
  2026-08-08 17:48   ` Andy Shevchenko
  1 sibling, 2 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2026-08-08 17:23 UTC (permalink / raw)
  To: Ilpo Järvinen, Andrew Morton, Petr Mladek, Steven Rostedt
  Cc: Andy Shevchenko, Rasmus Villemoes, Sergey Senozhatsky, linux-pci,
	linux-kernel, Bjorn Helgaas

Include human-readable sizes, e.g., "16.0 MiB", in addition to the hex
"0x1000000" size, in resource-related messages.  Also consistently include
the "0x" prefix.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/setup-bus.c | 46 +++++++++++++++++++++++++++++------------
 drivers/pci/setup-res.c | 11 ++++++----
 2 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index c0a949f2c995..4f62ba6f4d7f 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -28,6 +28,7 @@
 #include <linux/limits.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
+#include <linux/string_helpers.h>
 #include <linux/acpi.h>
 #include "pci.h"
 
@@ -427,6 +428,7 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
 	struct resource *res;
 	const char *res_name;
 	resource_size_t add_size, align;
+	char size_buf[32];
 	int idx;
 
 	list_for_each_entry_safe(add_res, tmp, realloc_head, list) {
@@ -460,10 +462,14 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
 		} else if (add_size > 0 || !IS_ALIGNED(res->start, align)) {
 			res->flags |= add_res->flags &
 				 (IORESOURCE_STARTALIGN|IORESOURCE_SIZEALIGN);
-			if (pci_reassign_resource(dev, idx, add_size, align))
-				pci_info(dev, "%s %pR: failed to add optional %llx\n",
+			if (pci_reassign_resource(dev, idx, add_size, align)) {
+				string_get_size(add_size, 1, STRING_UNITS_2,
+                                        size_buf, sizeof(size_buf));
+				pci_info(dev, "%s %pR: failed to add optional %#llx (%s)\n",
 					 res_name, res,
-					 (unsigned long long) add_size);
+					 (unsigned long long) add_size,
+					 size_buf);
+			}
 		}
 out:
 		list_del(&add_res->list);
@@ -1076,6 +1082,7 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t add_size,
 	resource_size_t size = 0, size0 = 0, size1 = 0;
 	resource_size_t children_add_size = 0;
 	resource_size_t min_align, align;
+	char size_buf[32];
 
 	if (!b_res)
 		return;
@@ -1138,11 +1145,14 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t add_size,
 	b_res->flags |= IORESOURCE_STARTALIGN;
 	if (bus->self && size1 > size0 && realloc_head) {
 		b_res->flags &= ~IORESOURCE_DISABLED;
+		add_size = size1 - size0;
 		pci_dev_res_add_to_list(realloc_head, bus->self, b_res,
-					size1 - size0, min_align);
-		pci_info(bus->self, "bridge window %pR to %pR add_size %llx\n",
+					add_size, min_align);
+		string_get_size(add_size, 1, STRING_UNITS_2,
+                                size_buf, sizeof(size_buf));
+		pci_info(bus->self, "bridge window %pR to %pR add_size %#llx (%s)\n",
 			 b_res, &bus->busn_res,
-			 (unsigned long long) size1 - size0);
+			 (unsigned long long) add_size, size_buf);
 	}
 }
 
@@ -1284,6 +1294,7 @@ static void pbus_size_mem(struct pci_bus *bus, struct resource *b_res,
 	resource_size_t aligns[28] = {}; /* Alignments from 1MB to 128TB */
 	int order, max_order;
 	resource_size_t children_add_size = 0;
+	char size_buf[32], align_buf[32];
 	resource_size_t add_align = 0;
 
 	if (!b_res)
@@ -1378,10 +1389,14 @@ static void pbus_size_mem(struct pci_bus *bus, struct resource *b_res,
 		add_size = size1 > size0 ? size1 - size0 : 0;
 		pci_dev_res_add_to_list(realloc_head, bus->self, b_res,
 					add_size, add_align);
-		pci_info(bus->self, "bridge window %pR to %pR add_size %llx add_align %llx\n",
+		string_get_size(add_size, 1, STRING_UNITS_2, size_buf,
+				sizeof(size_buf));
+		string_get_size(add_align, 1, STRING_UNITS_2, align_buf,
+				sizeof(align_buf));
+		pci_info(bus->self, "bridge window %pR to %pR add_size %#llx (%s) add_align %#llx (%s)\n",
 			   b_res, &bus->busn_res,
-			   (unsigned long long) add_size,
-			   (unsigned long long) add_align);
+			   (unsigned long long) add_size, size_buf,
+			   (unsigned long long) add_align, align_buf);
 	}
 }
 
@@ -1857,6 +1872,7 @@ static void adjust_bridge_window(struct pci_dev *bridge, struct resource *res,
 {
 	resource_size_t add_size, size = resource_size(res);
 	struct pci_dev_resource *dev_res;
+	char size_buf[32];
 
 	if (resource_assigned(res))
 		return;
@@ -1866,8 +1882,10 @@ static void adjust_bridge_window(struct pci_dev *bridge, struct resource *res,
 
 	if (new_size > size) {
 		add_size = new_size - size;
-		pci_dbg(bridge, "bridge window %pR extended by %pa\n", res,
-			&add_size);
+		string_get_size(add_size, 1, STRING_UNITS_2,
+				size_buf, sizeof(size_buf));
+		pci_dbg(bridge, "bridge window %pR extended by %pa (%s)\n", res,
+			&add_size, size_buf);
 	} else if (new_size < size) {
 		int idx = pci_resource_num(bridge, res);
 
@@ -1900,8 +1918,10 @@ static void adjust_bridge_window(struct pci_dev *bridge, struct resource *res,
 		add_size = size - new_size;
 		if (add_size < dev_res->add_size) {
 			dev_res->add_size -= add_size;
-			pci_dbg(bridge, "bridge window %pR optional size shrunken by %pa\n",
-				res, &add_size);
+			string_get_size(add_size, 1, STRING_UNITS_2,
+					size_buf, sizeof(size_buf));
+			pci_dbg(bridge, "bridge window %pR optional size shrunken by %pa (%s)\n",
+				res, &add_size, size_buf);
 		} else {
 			pci_dbg(bridge, "bridge window %pR optional size removed\n",
 				res);
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 376f09630a4a..707c405000b8 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -20,6 +20,7 @@
 #include <linux/ioport.h>
 #include <linux/cache.h>
 #include <linux/slab.h>
+#include <linux/string_helpers.h>
 #include "pci.h"
 
 static void pci_std_update_resource(struct pci_dev *dev, int resno)
@@ -431,6 +432,7 @@ int pci_reassign_resource(struct pci_dev *dev, int resno,
 	const char *res_name = pci_resource_name(dev, resno);
 	unsigned long flags;
 	resource_size_t new_size;
+	char size_buf[32];
 	int ret;
 
 	if (res->flags & IORESOURCE_PCI_FIXED)
@@ -444,19 +446,20 @@ int pci_reassign_resource(struct pci_dev *dev, int resno,
 		return -EINVAL;
 	}
 
+	string_get_size(addsize, 1, STRING_UNITS_2, size_buf, sizeof(size_buf));
 	new_size = resource_size(res) + addsize;
 	ret = _pci_assign_resource(dev, resno, new_size, min_align);
 	if (ret) {
 		res->flags = flags;
-		pci_info(dev, "%s %pR: failed to expand by %#llx\n",
-			 res_name, res, (unsigned long long) addsize);
+		pci_info(dev, "%s %pR: failed to expand by %#llx (%s)\n",
+			 res_name, res, (unsigned long long) addsize, size_buf);
 		return ret;
 	}
 
 	res->flags &= ~IORESOURCE_UNSET;
 	res->flags &= ~IORESOURCE_STARTALIGN;
-	pci_info(dev, "%s %pR: reassigned; expanded by %#llx\n",
-		 res_name, res, (unsigned long long) addsize);
+	pci_info(dev, "%s %pR: reassigned; expanded by %#llx (%s)\n",
+		 res_name, res, (unsigned long long) addsize, size_buf);
 	if (resno < PCI_BRIDGE_RESOURCES)
 		pci_update_resource(dev, resno);
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages
  2026-08-08 17:23 ` [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages Bjorn Helgaas
@ 2026-08-08 17:27   ` sashiko-bot
  2026-08-08 17:48   ` Andy Shevchenko
  1 sibling, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-08-08 17:27 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci

> Include human-readable sizes, e.g., "16.0 MiB", in addition to the hex
> "0x1000000" size, in resource-related messages.  Also consistently include
> the "0x" prefix.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260808172308.282591-1-bhelgaas@google.com?part=2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] vsprintf: Add %pR human-readable size
  2026-08-08 17:23 ` [PATCH v1 1/2] vsprintf: Add %pR human-readable size Bjorn Helgaas
@ 2026-08-08 17:38   ` sashiko-bot
  2026-08-08 17:43   ` Andy Shevchenko
  1 sibling, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-08-08 17:38 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a KUnit test regression by unconditionally appending a human-readable resource size for memory ranges, affecting the raw '%pr' format as well as the decoded '%pR' format. The commit message states it adds the size to '%pR', but the code incorrectly modifies both formats, breaking the printf test suite.
--

--- Patch [1]: [PATCH v1 1/2] vsprintf: Add %pR human-readable size ---
commit d77ec66543a5589a88168c27ffcdb3693182c105
Author: Bjorn Helgaas <helgaas@kernel.org>

vsprintf: Add %pR human-readable size

Include human-readable size when printing struct resource memory ranges
with the %pR format to make it easier to read.

> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 2bc6ef483576c..c49044b6dbeeb 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
[ ... ]
> @@ -1134,6 +1136,14 @@ char *resource_string(char *buf, char *end, struct resource *res,
>  	} else {
>  		p = hex_range(p, pend, res->start, res->end, *specp);
>  	}
> +	if (res->flags & IORESOURCE_MEM) {
> +		*p++ = ' ';
> +		*p++ = '(';
> +		string_get_size(resource_size(res), 1, STRING_UNITS_2,
> +				size_buf, sizeof(size_buf));
> +		p = string_nocheck(p, pend, size_buf, str_spec);
> +		*p++ = ')';
> +	}

[Severity: Medium]
Does this code inadvertently modify the raw '%pr' format as well?

The commit message states it adds the size to '%pR', but this check doesn't
verify the 'decode' boolean before appending the human-readable size.

This unconditionally appends the human-readable resource size for memory ranges,
which affects the raw '%pr' format and breaks KUnit tests expecting the
unmodified output:

lib/tests/printf_kunit.c:struct_resource() {
    ...
    test("[mem 0xc0ffee00 flags 0x200]",
         "%pr", &test_resource);
    ...
}

>  	if (decode) {
>  		if (res->flags & IORESOURCE_MEM_64)
>  			p = string_nocheck(p, pend, " 64bit", str_spec);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260808172308.282591-1-bhelgaas@google.com?part=1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] vsprintf: Add %pR human-readable size
  2026-08-08 17:23 ` [PATCH v1 1/2] vsprintf: Add %pR human-readable size Bjorn Helgaas
  2026-08-08 17:38   ` sashiko-bot
@ 2026-08-08 17:43   ` Andy Shevchenko
  2026-08-08 23:45     ` Bjorn Helgaas
  1 sibling, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2026-08-08 17:43 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Ilpo Järvinen, Andrew Morton, Petr Mladek, Steven Rostedt,
	Rasmus Villemoes, Sergey Senozhatsky, linux-pci, linux-kernel,
	Bjorn Helgaas

On Sat, Aug 08, 2026 at 12:23:07PM -0500, Bjorn Helgaas wrote:
> Include human-readable size when printing struct resource memory ranges
> (not including DMA, bus number, or I/O port ranges) with the %pR format to
> make it easier to read, e.g.,
> 
>   - pci 0000:00:02.0: BAR 0 [mem 0xfea80000-0xfea9ffff]
>   + pci 0000:00:02.0: BAR 0 [mem 0xfea80000-0xfea9ffff (128 KiB)]

...

>  lib/vsprintf.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)

Sorry, we do not accept this without added test cases.

...

> +	if (res->flags & IORESOURCE_MEM) {

Besides we have resource_type() macro, why only this? I would expect IO as well
as 64-bit MEM version.

> +		*p++ = ' ';
> +		*p++ = '(';
> +		string_get_size(resource_size(res), 1, STRING_UNITS_2,
> +				size_buf, sizeof(size_buf));

> +		p = string_nocheck(p, pend, size_buf, str_spec);

Why do you need this? Use returned value from string_get_size() directly.

> +		*p++ = ')';
> +	}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages
  2026-08-08 17:23 ` [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages Bjorn Helgaas
  2026-08-08 17:27   ` sashiko-bot
@ 2026-08-08 17:48   ` Andy Shevchenko
  2026-08-21 13:46     ` Petr Mladek
  1 sibling, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2026-08-08 17:48 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Ilpo Järvinen, Andrew Morton, Petr Mladek, Steven Rostedt,
	Rasmus Villemoes, Sergey Senozhatsky, linux-pci, linux-kernel,
	Bjorn Helgaas

On Sat, Aug 08, 2026 at 12:23:08PM -0500, Bjorn Helgaas wrote:
> Include human-readable sizes, e.g., "16.0 MiB", in addition to the hex
> "0x1000000" size, in resource-related messages.  Also consistently include
> the "0x" prefix.

Instead of repeating many times the %#llx (%s) and accompanying
string_get_size() calls can we rather introduce a (sub-)extension
to %p[R] (perhaps against 'R' to print only size) and use it?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] vsprintf: Add %pR human-readable size
  2026-08-08 17:43   ` Andy Shevchenko
@ 2026-08-08 23:45     ` Bjorn Helgaas
  2026-08-09  8:25       ` Andy Shevchenko
  0 siblings, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2026-08-08 23:45 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Ilpo Järvinen, Andrew Morton, Petr Mladek, Steven Rostedt,
	Rasmus Villemoes, Sergey Senozhatsky, linux-pci, linux-kernel,
	Bjorn Helgaas

On Sat, Aug 08, 2026 at 08:43:30PM +0300, Andy Shevchenko wrote:
> On Sat, Aug 08, 2026 at 12:23:07PM -0500, Bjorn Helgaas wrote:
> > Include human-readable size when printing struct resource memory ranges
> > (not including DMA, bus number, or I/O port ranges) with the %pR format to
> > make it easier to read, e.g.,
> > 
> >   - pci 0000:00:02.0: BAR 0 [mem 0xfea80000-0xfea9ffff]
> >   + pci 0000:00:02.0: BAR 0 [mem 0xfea80000-0xfea9ffff (128 KiB)]
> 
> ...
> 
> >  lib/vsprintf.c | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> Sorry, we do not accept this without added test cases.

TIL about these tests, they look great and I'll fix this up and update
printk-formats.rst as well, thank you!

> ...
> 
> > +	if (res->flags & IORESOURCE_MEM) {
> 
> Besides we have resource_type() macro, why only this? I would expect
> IO as well as 64-bit MEM version.

We could use resource_type() here, but would have to check other bits
directly (IORESOURCE_PREFETCH, IORESOURCE_MEM_64, etc) since they're
not included.

I considered including IO and certainly could do that.  IO resources
are smaller and it's not as interesting, but maybe we should.

64-bit MEM resources are already included because they have both
IORESOURCE_MEM and IORESOURCE_MEM_64 set.

> > +		*p++ = ' ';
> > +		*p++ = '(';
> > +		string_get_size(resource_size(res), 1, STRING_UNITS_2,
> > +				size_buf, sizeof(size_buf));
> 
> > +		p = string_nocheck(p, pend, size_buf, str_spec);
> 
> Why do you need this? Use returned value from string_get_size() directly.

string_get_size() doesn't return a char * like string_nocheck() et al
do.  We could do something like "p += string_get_size(..., p, pend - p)",
but it's a little messy because the return value is the length that
*would* have been written if it was truncated, so I think we'd have to
check for "p > pend".  I'll ponder this one.

> > +		*p++ = ')';
> > +	}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] vsprintf: Add %pR human-readable size
  2026-08-08 23:45     ` Bjorn Helgaas
@ 2026-08-09  8:25       ` Andy Shevchenko
  2026-08-24  8:31         ` Ilpo Järvinen
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2026-08-09  8:25 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Ilpo Järvinen, Andrew Morton, Petr Mladek, Steven Rostedt,
	Rasmus Villemoes, Sergey Senozhatsky, linux-pci, linux-kernel,
	Bjorn Helgaas

On Sat, Aug 08, 2026 at 06:45:30PM -0500, Bjorn Helgaas wrote:
> On Sat, Aug 08, 2026 at 08:43:30PM +0300, Andy Shevchenko wrote:
> > On Sat, Aug 08, 2026 at 12:23:07PM -0500, Bjorn Helgaas wrote:

...

> > > +	if (res->flags & IORESOURCE_MEM) {
> > 
> > Besides we have resource_type() macro, why only this? I would expect
> > IO as well as 64-bit MEM version.
> 
> We could use resource_type() here, but would have to check other bits
> directly (IORESOURCE_PREFETCH, IORESOURCE_MEM_64, etc) since they're
> not included.

Sure.

> I considered including IO and certainly could do that.  IO resources
> are smaller and it's not as interesting, but maybe we should.
> 
> 64-bit MEM resources are already included because they have both
> IORESOURCE_MEM and IORESOURCE_MEM_64 set.

Is it guaranteed to have MEM when MEM_64 is set?

...

> > > +		*p++ = ' ';
> > > +		*p++ = '(';
> > > +		string_get_size(resource_size(res), 1, STRING_UNITS_2,
> > > +				size_buf, sizeof(size_buf));
> > 
> > > +		p = string_nocheck(p, pend, size_buf, str_spec);
> > 
> > Why do you need this? Use returned value from string_get_size() directly.
> 
> string_get_size() doesn't return a char * like string_nocheck() et al
> do.  We could do something like "p += string_get_size(..., p, pend - p)",
> but it's a little messy

See the use of string_escape_mem() in the same file.

> because the return value is the length that
> *would* have been written if it was truncated, so I think we'd have to
> check for "p > pend".  I'll ponder this one.

Yes, and that's exactly what we want from the vsnprintf() as a returned value.
What I mean is to use the given buffer directly without any intermediate one.
The *p++ = 'xxx' will become an idiomatic

	if (end > ...)
		*buf = 'xxx';
	++buf;

which vsprintf.c is full of.

> > > +		*p++ = ')';
> > > +	}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages
  2026-08-08 17:48   ` Andy Shevchenko
@ 2026-08-21 13:46     ` Petr Mladek
  2026-08-24  8:18       ` Andy Shevchenko
  0 siblings, 1 reply; 12+ messages in thread
From: Petr Mladek @ 2026-08-21 13:46 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Bjorn Helgaas, Ilpo Järvinen, Andrew Morton, Steven Rostedt,
	Rasmus Villemoes, Sergey Senozhatsky, linux-pci, linux-kernel,
	Bjorn Helgaas

On Sat 2026-08-08 20:48:00, Andy Shevchenko wrote:
> On Sat, Aug 08, 2026 at 12:23:08PM -0500, Bjorn Helgaas wrote:
> > Include human-readable sizes, e.g., "16.0 MiB", in addition to the hex
> > "0x1000000" size, in resource-related messages.  Also consistently include
> > the "0x" prefix.
> 
> Instead of repeating many times the %#llx (%s) and accompanying
> string_get_size() calls can we rather introduce a (sub-)extension
> to %p[R] (perhaps against 'R' to print only size) and use it?

I am not sure if I understand it correctly. It looks to me that
this patch uses string_get_size() for printing some "arbitrary" size
values. Some are not part of struct resources, so using %pRR
might be confusing.

Unfortunately, implementing a generic printf modifier for printing
human readable size is complicated. It should keep the type-size
checks. Also it should allow to distinguish binary vs decimal
size calculation, for example 1kB vs 1kHz for 1024B vs 1000Hz.
See https://lore.kernel.org/all/ZbFd5TZ_pi7q3hso@casper.infradead.org/

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages
  2026-08-21 13:46     ` Petr Mladek
@ 2026-08-24  8:18       ` Andy Shevchenko
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2026-08-24  8:18 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Bjorn Helgaas, Ilpo Järvinen, Andrew Morton, Steven Rostedt,
	Rasmus Villemoes, Sergey Senozhatsky, linux-pci, linux-kernel,
	Bjorn Helgaas

On Fri, Aug 21, 2026 at 03:46:38PM +0200, Petr Mladek wrote:
> On Sat 2026-08-08 20:48:00, Andy Shevchenko wrote:
> > On Sat, Aug 08, 2026 at 12:23:08PM -0500, Bjorn Helgaas wrote:
> > > Include human-readable sizes, e.g., "16.0 MiB", in addition to the hex
> > > "0x1000000" size, in resource-related messages.  Also consistently include
> > > the "0x" prefix.
> > 
> > Instead of repeating many times the %#llx (%s) and accompanying
> > string_get_size() calls can we rather introduce a (sub-)extension
> > to %p[R] (perhaps against 'R' to print only size) and use it?
> 
> I am not sure if I understand it correctly. It looks to me that
> this patch uses string_get_size() for printing some "arbitrary" size
> values. Some are not part of struct resources, so using %pRR
> might be confusing.

AFAICS (but I might have missed something) they all can be containered into
the local variables of type 'struct resource' and then be used with that
extension directly. So, I don't see that it will be confusing.

> Unfortunately, implementing a generic printf modifier for printing
> human readable size is complicated. It should keep the type-size
> checks. Also it should allow to distinguish binary vs decimal
> size calculation, for example 1kB vs 1kHz for 1024B vs 1000Hz.
> See https://lore.kernel.org/all/ZbFd5TZ_pi7q3hso@casper.infradead.org/

I have an idea about this, but I think it's too premature for that type
of extension. So far, this series (AFAIU) is only about known type and
hence known units to print with the format also kinda fixed.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] vsprintf: Add %pR human-readable size
  2026-08-09  8:25       ` Andy Shevchenko
@ 2026-08-24  8:31         ` Ilpo Järvinen
  0 siblings, 0 replies; 12+ messages in thread
From: Ilpo Järvinen @ 2026-08-24  8:31 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Bjorn Helgaas, Andrew Morton, Petr Mladek, Steven Rostedt,
	Rasmus Villemoes, Sergey Senozhatsky, linux-pci, LKML,
	Bjorn Helgaas

On Sun, 9 Aug 2026, Andy Shevchenko wrote:

> On Sat, Aug 08, 2026 at 06:45:30PM -0500, Bjorn Helgaas wrote:
> > On Sat, Aug 08, 2026 at 08:43:30PM +0300, Andy Shevchenko wrote:
> > > On Sat, Aug 08, 2026 at 12:23:07PM -0500, Bjorn Helgaas wrote:
> 
> ...
> 
> > > > +	if (res->flags & IORESOURCE_MEM) {
> > > 
> > > Besides we have resource_type() macro, why only this? I would expect
> > > IO as well as 64-bit MEM version.
> > 
> > We could use resource_type() here, but would have to check other bits
> > directly (IORESOURCE_PREFETCH, IORESOURCE_MEM_64, etc) since they're
> > not included.
> 
> Sure.
> 
> > I considered including IO and certainly could do that.  IO resources
> > are smaller and it's not as interesting, but maybe we should.
> > 
> > 64-bit MEM resources are already included because they have both
> > IORESOURCE_MEM and IORESOURCE_MEM_64 set.
> 
> Is it guaranteed to have MEM when MEM_64 is set?

While I suspect nothing really guarantees that (by checking if invariant 
holds), I'd expect many things to do weird things if a resouce only has 
MEM_64 but not MEM.

-- 
 i.


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-08-24  8:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-08 17:23 [PATCH v1 0/2] PCI: Include human-readable BAR and window sizes Bjorn Helgaas
2026-08-08 17:23 ` [PATCH v1 1/2] vsprintf: Add %pR human-readable size Bjorn Helgaas
2026-08-08 17:38   ` sashiko-bot
2026-08-08 17:43   ` Andy Shevchenko
2026-08-08 23:45     ` Bjorn Helgaas
2026-08-09  8:25       ` Andy Shevchenko
2026-08-24  8:31         ` Ilpo Järvinen
2026-08-08 17:23 ` [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages Bjorn Helgaas
2026-08-08 17:27   ` sashiko-bot
2026-08-08 17:48   ` Andy Shevchenko
2026-08-21 13:46     ` Petr Mladek
2026-08-24  8:18       ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox