diff mbox series

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

Message ID 20210510041424.233565-1-kw@linux.com
State Not Applicable
Headers show
Series [01/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (fcc98c6d0289241dded10b74f8198fc4ecb22bd1)
snowpatch_ozlabs/checkpatch warning total: 1 errors, 1 warnings, 0 checks, 8 lines checked
snowpatch_ozlabs/needsstable success Patch has no Fixes tags

Commit Message

Krzysztof Wilczyński May 10, 2021, 4:14 a.m. UTC
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 <kw@linux.com>
---
 drivers/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Krzysztof Wilczyński May 10, 2021, 10:30 a.m. UTC | #1
[+cc Joe for visibility]

[...]
>  	spin_lock(&resource_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(&resource_alignment_lock);

Following the work that Joe did recently, see:

  https://lore.kernel.org/lkml/aa1819fa5faf786573df298e5e2e7d357ba7d4ad.camel@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
Logan Gunthorpe May 10, 2021, 3:57 p.m. UTC | #2
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 <kw@linux.com>

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

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

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

Logan
Krzysztof Wilczyński May 13, 2021, 12:49 p.m. UTC | #3
Hi Logan,

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

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
diff mbox series

Patch

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(&resource_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(&resource_alignment_lock);
 
 	/*