* [PATCH] leds: fix /sys/class/leds/<led>/trigger
@ 2019-09-12 14:39 Akinobu Mita
2019-09-12 14:39 ` [PATCH] leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger Akinobu Mita
0 siblings, 1 reply; 5+ messages in thread
From: Akinobu Mita @ 2019-09-12 14:39 UTC (permalink / raw
To: linux-leds, linux-kernel
Cc: Akinobu Mita, Greg Kroah-Hartman, Rafael J. Wysocki,
Jacek Anaszewski, Pavel Machek, Dan Murphy
Reading /sys/class/leds/<led>/trigger returns all available LED triggers.
However, the size of this file is limited to PAGE_SIZE because of the
limitation for sysfs attribute.
Enabling LED CPU trigger on systems with thousands of CPUs easily hits
PAGE_SIZE limit, and makes it impossible to see all available LED triggers
and which trigger is currently activated.
This patch converts /sys/class/leds/<led>/trigger to bin attribute and
removes the PAGE_SIZE limitation.
The first version of this seris provided the new api that follows the
"one value per file" rule of sysfs. This second version dropped it because
there have been a number of problems and it turns out that the new api
should be submitted separately.
* v2
- Update commit message
- Drop patches for new api
Akinobu Mita (1):
leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger
drivers/leds/led-class.c | 8 ++--
drivers/leds/led-triggers.c | 90 ++++++++++++++++++++++++++++++++++-----------
drivers/leds/leds.h | 6 +++
include/linux/leds.h | 5 ---
4 files changed, 79 insertions(+), 30 deletions(-)
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Dan Murphy <dmurphy@ti.com>
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger
2019-09-12 14:39 [PATCH] leds: fix /sys/class/leds/<led>/trigger Akinobu Mita
@ 2019-09-12 14:39 ` Akinobu Mita
2019-09-12 17:15 ` Jacek Anaszewski
0 siblings, 1 reply; 5+ messages in thread
From: Akinobu Mita @ 2019-09-12 14:39 UTC (permalink / raw
To: linux-leds, linux-kernel
Cc: Akinobu Mita, Greg Kroah-Hartman, Rafael J. Wysocki,
Jacek Anaszewski, Pavel Machek, Dan Murphy
Reading /sys/class/leds/<led>/trigger returns all available LED triggers.
However, the size of this file is limited to PAGE_SIZE because of the
limitation for sysfs attribute.
Enabling LED CPU trigger on systems with thousands of CPUs easily hits
PAGE_SIZE limit, and makes it impossible to see all available LED triggers
and which trigger is currently activated.
We work around it here by converting /sys/class/leds/<led>/trigger to
binary attribute, which is not limited by length. This is _not_ good
design, do not copy it.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Dan Murphy <dmurphy@ti.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
drivers/leds/led-class.c | 8 ++--
drivers/leds/led-triggers.c | 90 ++++++++++++++++++++++++++++++++++-----------
drivers/leds/leds.h | 6 +++
include/linux/leds.h | 5 ---
4 files changed, 79 insertions(+), 30 deletions(-)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 4793e77..8b5a1d1 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -73,13 +73,13 @@ static ssize_t max_brightness_show(struct device *dev,
static DEVICE_ATTR_RO(max_brightness);
#ifdef CONFIG_LEDS_TRIGGERS
-static DEVICE_ATTR(trigger, 0644, led_trigger_show, led_trigger_store);
-static struct attribute *led_trigger_attrs[] = {
- &dev_attr_trigger.attr,
+static BIN_ATTR(trigger, 0644, led_trigger_read, led_trigger_write, 0);
+static struct bin_attribute *led_trigger_bin_attrs[] = {
+ &bin_attr_trigger,
NULL,
};
static const struct attribute_group led_trigger_group = {
- .attrs = led_trigger_attrs,
+ .bin_attrs = led_trigger_bin_attrs,
};
#endif
diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index 8d11a5e..ed5a311 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -16,6 +16,7 @@
#include <linux/rwsem.h>
#include <linux/leds.h>
#include <linux/slab.h>
+#include <linux/mm.h>
#include "leds.h"
/*
@@ -26,9 +27,11 @@ LIST_HEAD(trigger_list);
/* Used by LED Class */
-ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count)
+ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t pos, size_t count)
{
+ struct device *dev = kobj_to_dev(kobj);
struct led_classdev *led_cdev = dev_get_drvdata(dev);
struct led_trigger *trig;
int ret = count;
@@ -64,39 +67,84 @@ ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
mutex_unlock(&led_cdev->led_access);
return ret;
}
-EXPORT_SYMBOL_GPL(led_trigger_store);
+EXPORT_SYMBOL_GPL(led_trigger_write);
-ssize_t led_trigger_show(struct device *dev, struct device_attribute *attr,
- char *buf)
+__printf(4, 5)
+static int led_trigger_snprintf(char *buf, size_t size, bool query,
+ const char *fmt, ...)
+{
+ va_list args;
+ int i;
+
+ va_start(args, fmt);
+ if (query)
+ i = vsnprintf(NULL, 0, fmt, args);
+ else
+ i = vscnprintf(buf, size, fmt, args);
+ va_end(args);
+
+ return i;
+}
+
+static int led_trigger_format(char *buf, size_t size, bool query,
+ struct led_classdev *led_cdev)
{
- struct led_classdev *led_cdev = dev_get_drvdata(dev);
struct led_trigger *trig;
- int len = 0;
+ int len = led_trigger_snprintf(buf, size, query, "%s",
+ led_cdev->trigger ? "none" : "[none]");
+
+ list_for_each_entry(trig, &trigger_list, next_trig) {
+ bool hit = led_cdev->trigger &&
+ !strcmp(led_cdev->trigger->name, trig->name);
+
+ len += led_trigger_snprintf(buf + len, size - len, query,
+ " %s%s%s", hit ? "[" : "",
+ trig->name, hit ? "]" : "");
+ }
+
+ len += led_trigger_snprintf(buf + len, size - len, query, "\n");
+
+ return len;
+}
+
+/*
+ * It was stupid to create 10000 cpu triggers, but we are stuck with it now.
+ * Don't make that mistake again. We work around it here by creating binary
+ * attribute, which is not limited by length. This is _not_ good design, do not
+ * copy it.
+ */
+ssize_t led_trigger_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf,
+ loff_t pos, size_t count)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ void *data;
+ int len;
down_read(&triggers_list_lock);
down_read(&led_cdev->trigger_lock);
- if (!led_cdev->trigger)
- len += scnprintf(buf+len, PAGE_SIZE - len, "[none] ");
+ len = led_trigger_format(NULL, 0, true, led_cdev);
+ data = kvmalloc(len + 1, GFP_KERNEL);
+ if (data)
+ len = led_trigger_format(data, len + 1, false, led_cdev);
else
- len += scnprintf(buf+len, PAGE_SIZE - len, "none ");
+ len = -ENOMEM;
- list_for_each_entry(trig, &trigger_list, next_trig) {
- if (led_cdev->trigger && !strcmp(led_cdev->trigger->name,
- trig->name))
- len += scnprintf(buf+len, PAGE_SIZE - len, "[%s] ",
- trig->name);
- else
- len += scnprintf(buf+len, PAGE_SIZE - len, "%s ",
- trig->name);
- }
up_read(&led_cdev->trigger_lock);
up_read(&triggers_list_lock);
- len += scnprintf(len+buf, PAGE_SIZE - len, "\n");
+ if (len < 0)
+ return len;
+
+ len = memory_read_from_buffer(buf, count, &pos, data, len);
+
+ kvfree(data);
+
return len;
}
-EXPORT_SYMBOL_GPL(led_trigger_show);
+EXPORT_SYMBOL_GPL(led_trigger_read);
/* Caller must ensure led_cdev->trigger_lock held */
int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 47b2294..a0ee33c 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -23,6 +23,12 @@ void led_set_brightness_nopm(struct led_classdev *led_cdev,
enum led_brightness value);
void led_set_brightness_nosleep(struct led_classdev *led_cdev,
enum led_brightness value);
+ssize_t led_trigger_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf,
+ loff_t pos, size_t count);
+ssize_t led_trigger_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t pos, size_t count);
extern struct rw_semaphore leds_list_lock;
extern struct list_head leds_list;
diff --git a/include/linux/leds.h b/include/linux/leds.h
index fd2eb7c..33ae825 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -290,11 +290,6 @@ struct led_trigger {
#define led_trigger_get_led(dev) ((struct led_classdev *)dev_get_drvdata((dev)))
#define led_trigger_get_drvdata(dev) (led_get_trigger_data(led_trigger_get_led(dev)))
-ssize_t led_trigger_store(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count);
-ssize_t led_trigger_show(struct device *dev, struct device_attribute *attr,
- char *buf);
-
/* Registration functions for complex triggers */
extern int led_trigger_register(struct led_trigger *trigger);
extern void led_trigger_unregister(struct led_trigger *trigger);
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger
2019-09-12 14:39 ` [PATCH] leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger Akinobu Mita
@ 2019-09-12 17:15 ` Jacek Anaszewski
2019-09-13 0:34 ` Akinobu Mita
0 siblings, 1 reply; 5+ messages in thread
From: Jacek Anaszewski @ 2019-09-12 17:15 UTC (permalink / raw
To: Akinobu Mita, linux-leds, linux-kernel
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Pavel Machek, Dan Murphy
Hi Akinobu,
Please bump patch version each time you send an update
of the patch with the same subject.
Best regards,
Jacek Anaszewski
On 9/12/19 4:39 PM, Akinobu Mita wrote:
> Reading /sys/class/leds/<led>/trigger returns all available LED triggers.
> However, the size of this file is limited to PAGE_SIZE because of the
> limitation for sysfs attribute.
>
> Enabling LED CPU trigger on systems with thousands of CPUs easily hits
> PAGE_SIZE limit, and makes it impossible to see all available LED triggers
> and which trigger is currently activated.
>
> We work around it here by converting /sys/class/leds/<led>/trigger to
> binary attribute, which is not limited by length. This is _not_ good
> design, do not copy it.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger
2019-09-12 17:15 ` Jacek Anaszewski
@ 2019-09-13 0:34 ` Akinobu Mita
2019-09-13 5:00 ` Greg Kroah-Hartman
0 siblings, 1 reply; 5+ messages in thread
From: Akinobu Mita @ 2019-09-13 0:34 UTC (permalink / raw
To: Jacek Anaszewski
Cc: linux-leds, LKML, Greg Kroah-Hartman, Rafael J. Wysocki,
Pavel Machek, Dan Murphy
2019年9月13日(金) 2:15 Jacek Anaszewski <jacek.anaszewski@gmail.com>:
>
> Hi Akinobu,
>
> Please bump patch version each time you send an update
> of the patch with the same subject.
Oops, should I resend with the correct subject?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger
2019-09-13 0:34 ` Akinobu Mita
@ 2019-09-13 5:00 ` Greg Kroah-Hartman
0 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2019-09-13 5:00 UTC (permalink / raw
To: Akinobu Mita
Cc: Jacek Anaszewski, linux-leds, LKML, Rafael J. Wysocki,
Pavel Machek, Dan Murphy
On Fri, Sep 13, 2019 at 09:34:49AM +0900, Akinobu Mita wrote:
> 2019年9月13日(金) 2:15 Jacek Anaszewski <jacek.anaszewski@gmail.com>:
> >
> > Hi Akinobu,
> >
> > Please bump patch version each time you send an update
> > of the patch with the same subject.
>
> Oops, should I resend with the correct subject?
Yes please.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-13 5:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-12 14:39 [PATCH] leds: fix /sys/class/leds/<led>/trigger Akinobu Mita
2019-09-12 14:39 ` [PATCH] leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger Akinobu Mita
2019-09-12 17:15 ` Jacek Anaszewski
2019-09-13 0:34 ` Akinobu Mita
2019-09-13 5:00 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).