Skip to contentSkip to conversation

leds: fix /sys/class/leds/<led>/trigger

PATCH Applied Akinobu Mita 3 participants 4 messages Sep 29, 2019 – Oct 1, 2019 LED

lore
Share & tools
lore
#0 lore raw

v3 cover leds: fix /sys/class/leds/<led>/trigger

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. The second version dropped it because there have been a number of problems and it turns out that the new api should be submitted separately.

* v3
- Remove "query" parameters from led_trigger_snprintf() and led_trigger_format()
- Return -ENOMEM immediately if memory allocation fails
- Drop Acked-by: tag due to a certain amount of changes

* 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, 78 insertions(+), 31 deletions(-)
Cc Greg Kroah-Hartman <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Jacek Anaszewski <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Dan Murphy <[email protected]>
Signature · 1 line
2.7.4
#1 lore raw

PATCH v3 1/1 leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger

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 <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Jacek Anaszewski <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Dan Murphy <[email protected]>
---
 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, 78 insertions(+), 31 deletions(-)
drivers/leds/led-class.c +4 -4
index 647b126..3f04334 100644
@@ -74,13 +74,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
 
drivers/leds/led-triggers.c +68 -22

122 lines not on this page · raw message

drivers/leds/leds.h +6 -0
index 0b577ce..2d9eb48 100644
@@ -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;
include/linux/leds.h +0 -5
index 5c8f3c5..da78b27 100644
@@ -361,11 +361,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);
Signature · 1 line
2.7.4
Reviewed-by on v3 1/1
#2 lore raw

Re: [PATCH v3 1/1] leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger

Quoted 12 lines · Akinobu Mita

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.

Quoted 6 lines
Cc Greg Kroah-Hartman <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Jacek Anaszewski <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Dan Murphy <[email protected]>
#3 lore raw

Re: [PATCH v3 1/1] leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger

In reply to Akinobu Mita

Hi Akinobu,

Thank you for your work on this patch.

Quoted 12 lines · Akinobu Mita

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.

Quoted 6 lines
Cc Greg Kroah-Hartman <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Jacek Anaszewski <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Dan Murphy <[email protected]>

Applied to the for-next branch of linux-leds.git

Signature · 2 lines
Best regards,
Jacek Anaszewski