Re: [PATCH 01/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

Hi Logan,

[...]
> Thanks, this is a great cleanup. I've reviewed the entire series.
> 
> Reviewed-by: Logan Gunthorpe 

Thank you!  Appreciate it!

> I agree that the new lines that are missing should be added.

While working on the simple change to add the missing new lines, I've
found that we have slight inconsistency with handling these in one of
the attributes, as per:

  # uname -r
  5.13.0-rc1-00092-gc06a2ba62fc4
  
  # grep -oE 'pci=resource_alignment.+' /proc/cmdline 
  pci=resource_alignment=20@00:1f.2

  # cat /sys/bus/pci/resource_alignment
  20@00:1f.
  
  # echo 20@00:1f.2 > /sys/bus/pci/resource_alignment
  # cat /sys/bus/pci/resource_alignment
  20@00:1f.2

  # echo -n 20@00:1f.2 > /sys/bus/pci/resource_alignment
  # cat /sys/bus/pci/resource_alignment
  20@00:1f.

I adjusted the code so that we handle the input better in the upcoming
v2 - the change is simple, but since it adds more code, I'll drop your
"Reviewed-by", so that we can make sure that subsequent review will
cover this new change.  Sorry for troubles in advance!

Krzysztof

Re: [PATCH 01/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Logan Gunthorpe]

On 2021-05-09 10:14 p.m., Krzysztof Wilczyński wrote:
> The sysfs_emit() and sysfs_emit_at() functions were introduced to make
> it less ambiguous which function is preferred when writing to the output
> buffer in a device attribute's "show" callback [1].
> 
> Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
> and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
> latter is aware of the PAGE_SIZE buffer and correctly returns the number
> of bytes written into the buffer.
> 
> No functional change intended.
> 
> [1] Documentation/filesystems/sysfs.rst
> 
> Related to:
>   commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in 
> "show" functions")
> 
> Signed-off-by: Krzysztof Wilczyński 

Thanks, this is a great cleanup. I've reviewed the entire series.

Reviewed-by: Logan Gunthorpe 

I agree that the new lines that are missing should be added.

Logan

Re: [PATCH 01/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

[+cc Joe for visibility]

[...]
>   spin_lock(_alignment_lock);
>   if (resource_alignment_param)
> - count = scnprintf(buf, PAGE_SIZE, "%s", 
> resource_alignment_param);
> + count = sysfs_emit(buf, "%s", resource_alignment_param);
>   spin_unlock(_alignment_lock);

Following the work that Joe did recently, see:

  
https://lore.kernel.org/lkml/aa1819fa5faf786573df298e5e2e7d357ba7d4ad.ca...@perches.com/

I think we ought to also add the missing newline to our sysfs_emit() and
sysfs_emit_at() users, like the one above and the following:

  drivers/pci/pci-sysfs.c
  540:  return sysfs_emit(buf, "%pOF", np);

To keep things correct and consistent.

Bjorn, I can follow-up with a small patch after this one, or send a v2,
or, if that would be OK with you, then you could fix it during merging,
provided you decide to merge things as-is.

Krzysztof

[PATCH 01/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in 
"show" functions")

Signed-off-by: Krzysztof Wilczyński 
---
 drivers/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b717680377a9..5ed316ea5831 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6439,7 +6439,7 @@ static ssize_t resource_alignment_show(struct bus_type 
*bus, char *buf)
 
spin_lock(_alignment_lock);
if (resource_alignment_param)
-   count = scnprintf(buf, PAGE_SIZE, "%s", 
resource_alignment_param);
+   count = sysfs_emit(buf, "%s", resource_alignment_param);
spin_unlock(_alignment_lock);
 
/*
-- 
2.31.1