Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
Hi Logan, > > 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") > > I re-reviewed the whole series. It still looks good to me. > > Very nice solution in patch 12 to the new line issue. > > Reviewed-by: Logan Gunthorpe > > Thanks, Thank you! I will send v3 incorporating the style change as per Joe's suggestion and carry-over your "Reviewed-by", if you don't mind, as it will be a trivial change. Krzysztof
Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
On 2021-05-14 11:24 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 > Reviewed-by: Logan Gunthorpe I re-reviewed the whole series. It still looks good to me. Very nice solution in patch 12 to the new line issue. Reviewed-by: Logan Gunthorpe Thanks, Logan
Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
Hi Joe, [...] > Ideally, the additional newline check below this would use sysfs_emit_at > > drivers/pci/pci.c- /* > drivers/pci/pci.c: * When set by the command line, > resource_alignment_param will not > drivers/pci/pci.c- * have a trailing line feed, which is ugly. So > conditionally add > drivers/pci/pci.c- * it here. > drivers/pci/pci.c- */ > drivers/pci/pci.c- if (count >= 2 && buf[count - 2] != '\n' && count < > PAGE_SIZE - 1) { > drivers/pci/pci.c- buf[count - 1] = '\n'; > drivers/pci/pci.c- buf[count++] = 0; > drivers/pci/pci.c- } > drivers/pci/pci.c- > drivers/pci/pci.c- return count; I found some inconsistencies with adding newline this way, and decided to change the code slightly, see: https://lore.kernel.org/linux-pci/20210515052434.1413236-12...@linux.com/ Krzysztof
Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
On Sat, 2021-05-15 at 05:24 +, 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. [] > diff --git 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); Ideally, the additional newline check below this would use sysfs_emit_at drivers/pci/pci.c- /* drivers/pci/pci.c: * When set by the command line, resource_alignment_param will not drivers/pci/pci.c- * have a trailing line feed, which is ugly. So conditionally add drivers/pci/pci.c- * it here. drivers/pci/pci.c- */ drivers/pci/pci.c- if (count >= 2 && buf[count - 2] != '\n' && count < PAGE_SIZE - 1) { drivers/pci/pci.c- buf[count - 1] = '\n'; drivers/pci/pci.c- buf[count++] = 0; drivers/pci/pci.c- } drivers/pci/pci.c- drivers/pci/pci.c- return count;
Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
Hello, [...] > Reviewed-by: Logan Gunthorpe Please disregard this "Reviewed-by" from Logan for this version, as I've forgotten to remove it before sending v2 after pulling patches using b4. Apologies. Krzysztof
[PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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 Reviewed-by: Logan Gunthorpe --- 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