| Message ID | 20210510041424.233565-1-kw@linux.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [01/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions | expand |
[+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
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
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 --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); /*
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(-)