| Message ID | 20210518034109.158450-12-kw@linux.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [v3,01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions | expand |
[+cc Greg and Sasha for visibility]
Hi Bjorn and Logan,
[...]
> Fixes: e499081da1a2 ("PCI: Force trailing new line to resource_alignment_param in sysfs")
[...]
This probably would be a candidate for a back-port to stable and
long-term releases. But, since the move to sysfs_emit()/sysfs_emit_at()
would be then irrelevant, I can split this patch so that it fixes the
issue first, and then other patch will move it to sysfs_emit(), so that
it would be easier to apply when back-porting.
Would this be fine with you Logan? Especially since you already offered
your review. I think Bjorn wouldn't mind the split either.
Do let me know folks, and I will change things accordingly.
Krzysztof
On 2021-05-21 9:14 a.m., Krzysztof Wilczyński wrote: > [+cc Greg and Sasha for visibility] > > Hi Bjorn and Logan, > > [...] >> Fixes: e499081da1a2 ("PCI: Force trailing new line to resource_alignment_param in sysfs") > [...] > > This probably would be a candidate for a back-port to stable and > long-term releases. But, since the move to sysfs_emit()/sysfs_emit_at() > would be then irrelevant, I can split this patch so that it fixes the > issue first, and then other patch will move it to sysfs_emit(), so that > it would be easier to apply when back-porting. > > Would this be fine with you Logan? Especially since you already offered > your review. I think Bjorn wouldn't mind the split either. Totally fine by me. I can re-review the two patches if you like. Thanks, Logan
On Fri, May 21, 2021 at 05:14:43PM +0200, Krzysztof Wilczyński wrote: > [+cc Greg and Sasha for visibility] > > Hi Bjorn and Logan, > > [...] > > Fixes: e499081da1a2 ("PCI: Force trailing new line to resource_alignment_param in sysfs") > [...] > > This probably would be a candidate for a back-port to stable and > long-term releases. But, since the move to sysfs_emit()/sysfs_emit_at() > would be then irrelevant, I can split this patch so that it fixes the > issue first, and then other patch will move it to sysfs_emit(), so that > it would be easier to apply when back-porting. sysfs_emit() and sysfs_emit_at() are in all supported stable and long-term kernel trees at this time, so there's no need to shy away from using them. thanks, greg k-h
Hi Greg, [...] > > This probably would be a candidate for a back-port to stable and > > long-term releases. But, since the move to sysfs_emit()/sysfs_emit_at() > > would be then irrelevant, I can split this patch so that it fixes the > > issue first, and then other patch will move it to sysfs_emit(), so that > > it would be easier to apply when back-porting. > > sysfs_emit() and sysfs_emit_at() are in all supported stable and > long-term kernel trees at this time, so there's no need to shy away from > using them. Excellent! With some changes requested by Bjorn this will then be easier to apply to current stable and long-term kernels. Krzysztof
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 5ed316ea5831..7cde86bdcc8e 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -6439,34 +6439,37 @@ static ssize_t resource_alignment_show(struct bus_type *bus, char *buf) spin_lock(&resource_alignment_lock); if (resource_alignment_param) - count = sysfs_emit(buf, "%s", resource_alignment_param); + count = sysfs_emit(buf, "%s\n", resource_alignment_param); spin_unlock(&resource_alignment_lock); - /* - * When set by the command line, resource_alignment_param will not - * have a trailing line feed, which is ugly. So conditionally add - * it here. - */ - if (count >= 2 && buf[count - 2] != '\n' && count < PAGE_SIZE - 1) { - buf[count - 1] = '\n'; - buf[count++] = 0; - } - return count; } static ssize_t resource_alignment_store(struct bus_type *bus, const char *buf, size_t count) { - char *param = kstrndup(buf, count, GFP_KERNEL); + char *param, *old, *end; + param = kstrndup(buf, count, GFP_KERNEL); if (!param) return -ENOMEM; + end = strchr(param, '\n'); + if (end) + *end = '\0'; + spin_lock(&resource_alignment_lock); - kfree(resource_alignment_param); - resource_alignment_param = param; + old = resource_alignment_param; + if (strlen(param)) { + resource_alignment_param = param; + } else { + kfree(resource_alignment_param); + resource_alignment_param = NULL; + } spin_unlock(&resource_alignment_lock); + + kfree(old); + return count; }
The value of the "resource_alignment" can be specified using a kernel command-line argument (using the "pci=resource_alignment=") or through the corresponding sysfs object under the /sys/bus/pci path. Currently, when the value is set via the kernel command-line argument, and then subsequently accessed through sysfs object, the value read back will not be correct, as per: # grep -oE 'pci=resource_alignment.+' /proc/cmdline pci=resource_alignment=20@00:1f.2 # cat /sys/bus/pci/resource_alignment 20@00:1f. This is also true when the value is set through the sysfs object, but the trailing newline has not been included, as per: # echo -n 20@00:1f.2 > /sys/bus/pci/resource_alignment # cat /sys/bus/pci/resource_alignment 20@00:1f. When the value set through the sysfs object includes the trailing newline, then reading it back will work as intended, as per: # echo 20@00:1f.2 > /sys/bus/pci/resource_alignment # cat /sys/bus/pci/resource_alignment 20@00:1f.2 To fix this inconsistency, append a trailing newline in the show() function and strip the trailing line in the store() function if one is present. Also, allow for the value previously set using either a command-line argument or through the sysfs object to be cleared at run-time. Fixes: e499081da1a2 ("PCI: Force trailing new line to resource_alignment_param in sysfs") Signed-off-by: Krzysztof Wilczyński <kw@linux.com> Reviewed-by: Logan Gunthorpe <logang@deltatee.com> --- Changes in v2: None. Changes in v3: Added Logan Gunthorpe's "Reviewed-by". drivers/pci/pci.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-)