lkml.org 
[lkml]   [2017]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 1/4] gpio: sysfs: change 'value' attribute to prealloc
2017-12-18 11:08 GMT+01:00 Christophe Leroy <christophe.leroy@c-s.fr>:
> The GPIO 'value' attribute is time critical. A small bench with
> 'perf record' on the app below shows that 80% of the time spent in
> sysfs_kf_seq_show() is spent in memset() for zeroising the buffer.
>
> |--67.48%--sysfs_kf_seq_show
> | |
> | |--54.40%--memset
> | |
> | |--11.49%--dev_attr_show
> | | |
> | | |--10.06%--value_show
> | | | |
> | | | |--4.75%--sprintf
> | | | | |
>
> This patch changes the attribute type to prealloc, eliminating the
> need to zeroise the buffer at each read. 'perf record' gives the
> following result.
>
> |--42.41%--sysfs_kf_read
> | |
> | |--39.73%--dev_attr_show
> | | |
> | | |--38.23%--value_show
> | | | |
> | | | |--29.22%--sprintf
> | | | | |
>
> Test done with the following small app:
>
> int main(int argc, char **argv)
> {
> int fd = open(argv[1], O_RDONLY);
>
> for (;;) {
> int buf[512];
>
> read(fd, buf, 512);
> lseek(fd, 0, SEEK_SET);
> }
> exit(0);
> }
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> drivers/gpio/gpiolib-sysfs.c | 2 +-
> include/linux/device.h | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
> index 3f454eaf2101..7a3f4271393b 100644
> --- a/drivers/gpio/gpiolib-sysfs.c
> +++ b/drivers/gpio/gpiolib-sysfs.c
> @@ -138,7 +138,7 @@ static ssize_t value_store(struct device *dev,
>
> return status;
> }
> -static DEVICE_ATTR_RW(value);
> +static DEVICE_ATTR_PREALLOC(value, S_IWUSR | S_IRUGO, value_show, value_store);
>
> static irqreturn_t gpio_sysfs_irq(int irq, void *priv)
> {
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 9d32000725da..46ac622e5c6f 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -575,6 +575,9 @@ ssize_t device_store_bool(struct device *dev, struct device_attribute *attr,
>
> #define DEVICE_ATTR(_name, _mode, _show, _store) \
> struct device_attribute dev_attr_##_name = __ATTR(_name, _mode, _show, _store)
> +#define DEVICE_ATTR_PREALLOC(_name, _mode, _show, _store) \
> + struct device_attribute dev_attr_##_name = \
> + __ATTR_PREALLOC(_name, _mode, _show, _store)
> #define DEVICE_ATTR_RW(_name) \
> struct device_attribute dev_attr_##_name = __ATTR_RW(_name)
> #define DEVICE_ATTR_RO(_name) \
> --
> 2.13.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html

I'm not sure we should be fixing performance issues in an interface
that's deprecated anyway...

The character device doesn't deal with strings so it will be much faster anyway.

Best regards,
Bartosz Golaszewski

\
 
 \ /
  Last update: 2017-12-18 11:19    [W:0.082 / U:0.912 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site