diff mbox series

[5/5] leds: trigger: netdev: use mutex instead of spinlocks

Message ID 20230419210743.3594-6-ansuelsmth@gmail.com
State New
Headers
Series leds: trigger: netdev: fixup preparation for LEDs hw control |

Commit Message

Christian Marangi April 19, 2023, 9:07 p.m. UTC 
  Some LEDs may require to sleep while doing some operation like setting
brightness and other cleanup.

For this reason, using a spinlock will cause a sleep under spinlock
warning.

It should be safe to convert this to a sleepable lock since:
- sysfs read/write can sleep
- netdev_trig_work is a work queue and can sleep
- netdev _trig_notify can sleep

The spinlock was used when brightness didn't support sleeping, but this
changed and now it supported with brightness_set_blocking().

Convert to mutex lock to permit sleeping using brightness_set_blocking().

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/leds/trigger/ledtrig-netdev.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Andrew Lunn April 20, 2023, 12:27 a.m. UTC | #1 
On Wed, Apr 19, 2023 at 11:07:43PM +0200, Christian Marangi wrote:
> Some LEDs may require to sleep while doing some operation like setting
> brightness and other cleanup.
> 
> For this reason, using a spinlock will cause a sleep under spinlock
> warning.
> 
> It should be safe to convert this to a sleepable lock since:
> - sysfs read/write can sleep
> - netdev_trig_work is a work queue and can sleep
> - netdev _trig_notify can sleep
> 
> The spinlock was used when brightness didn't support sleeping, but this
> changed and now it supported with brightness_set_blocking().
> 
> Convert to mutex lock to permit sleeping using brightness_set_blocking().
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Lee Jones April 24, 2023, 1:45 p.m. UTC | #2 
On Wed, 19 Apr 2023, Christian Marangi wrote:

> Some LEDs may require to sleep while doing some operation like setting
> brightness and other cleanup.
> 
> For this reason, using a spinlock will cause a sleep under spinlock
> warning.
> 
> It should be safe to convert this to a sleepable lock since:
> - sysfs read/write can sleep
> - netdev_trig_work is a work queue and can sleep
> - netdev _trig_notify can sleep
> 
> The spinlock was used when brightness didn't support sleeping, but this
> changed and now it supported with brightness_set_blocking().
> 
> Convert to mutex lock to permit sleeping using brightness_set_blocking().
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  drivers/leds/trigger/ledtrig-netdev.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)

Applied, thanks
diff mbox series

Patch

diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
index 5a47913c813c..115f2bae9eee 100644
--- a/drivers/leds/trigger/ledtrig-netdev.c
+++ b/drivers/leds/trigger/ledtrig-netdev.c
@@ -20,7 +20,7 @@ 
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
-#include <linux/spinlock.h>
+#include <linux/mutex.h>
 #include <linux/timer.h>
 #include "../leds.h"
 
@@ -37,7 +37,7 @@ 
  */
 
 struct led_netdev_data {
-	spinlock_t lock;
+	struct mutex lock;
 
 	struct delayed_work work;
 	struct notifier_block notifier;
@@ -97,9 +97,9 @@  static ssize_t device_name_show(struct device *dev,
 	struct led_netdev_data *trigger_data = led_trigger_get_drvdata(dev);
 	ssize_t len;
 
-	spin_lock_bh(&trigger_data->lock);
+	mutex_lock(&trigger_data->lock);
 	len = sprintf(buf, "%s\n", trigger_data->device_name);
-	spin_unlock_bh(&trigger_data->lock);
+	mutex_unlock(&trigger_data->lock);
 
 	return len;
 }
@@ -115,7 +115,7 @@  static ssize_t device_name_store(struct device *dev,
 
 	cancel_delayed_work_sync(&trigger_data->work);
 
-	spin_lock_bh(&trigger_data->lock);
+	mutex_lock(&trigger_data->lock);
 
 	if (trigger_data->net_dev) {
 		dev_put(trigger_data->net_dev);
@@ -138,7 +138,7 @@  static ssize_t device_name_store(struct device *dev,
 	trigger_data->last_activity = 0;
 
 	set_baseline_state(trigger_data);
-	spin_unlock_bh(&trigger_data->lock);
+	mutex_unlock(&trigger_data->lock);
 
 	return size;
 }
@@ -279,7 +279,7 @@  static int netdev_trig_notify(struct notifier_block *nb,
 
 	cancel_delayed_work_sync(&trigger_data->work);
 
-	spin_lock_bh(&trigger_data->lock);
+	mutex_lock(&trigger_data->lock);
 
 	trigger_data->carrier_link_up = false;
 	switch (evt) {
@@ -304,7 +304,7 @@  static int netdev_trig_notify(struct notifier_block *nb,
 
 	set_baseline_state(trigger_data);
 
-	spin_unlock_bh(&trigger_data->lock);
+	mutex_unlock(&trigger_data->lock);
 
 	return NOTIFY_DONE;
 }
@@ -365,7 +365,7 @@  static int netdev_trig_activate(struct led_classdev *led_cdev)
 	if (!trigger_data)
 		return -ENOMEM;
 
-	spin_lock_init(&trigger_data->lock);
+	mutex_init(&trigger_data->lock);
 
 	trigger_data->notifier.notifier_call = netdev_trig_notify;
 	trigger_data->notifier.priority = 10;