| Message ID | 20210604133230.983956-4-kw@linux.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions | expand |
On Fri, Jun 04, 2021 at 01:32:27PM +0000, Krzysztof Wilczyński wrote: > 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> > --- > drivers/pci/pci.c | 34 ++++++++++++++++++++-------------- > 1 file changed, 20 insertions(+), 14 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 5ed316ea5831..68f57d86b243 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -6439,34 +6439,40 @@ 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; > + > + if (count >= (PAGE_SIZE - 1)) > + return -EINVAL; > > + 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(param); I folded this kfree change (free "param" instead of "resource_alignment_param" as in v6) into my pci/sysfs branch. > + resource_alignment_param = NULL; > + } > spin_unlock(&resource_alignment_lock); > + > + kfree(old); > + > return count; > } > > -- > 2.31.1 >
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 5ed316ea5831..68f57d86b243 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -6439,34 +6439,40 @@ 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; + + if (count >= (PAGE_SIZE - 1)) + return -EINVAL; + 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(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> --- drivers/pci/pci.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-)