Simplify and improve the locking in rc-ir-raw by making better use of
the existing kfifo functionality and by using RCU where possible.
Signed-off-by: David Härdeman <david@xxxxxxxxxxx>
---
drivers/media/rc/rc-core-priv.h | 6 +-
drivers/media/rc/rc-ir-raw.c | 124 ++++++++++++++++-----------------------
2 files changed, 55 insertions(+), 75 deletions(-)
diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
index 0b32ef8..c3de26b 100644
--- a/drivers/media/rc/rc-core-priv.h
+++ b/drivers/media/rc/rc-core-priv.h
@@ -37,11 +37,13 @@ struct ir_raw_handler {
int (*raw_unregister)(struct rc_dev *dev);
};
+/* max number of pulse/space transitions to buffer */
+#define RC_MAX_IR_EVENTS 512
+
struct ir_raw_event_ctrl {
struct list_head list; /* to keep track of raw clients */
struct task_struct *thread;
- spinlock_t lock;
- struct kfifo_rec_ptr_1 kfifo; /* fifo for the pulse/space durations */
+ DECLARE_KFIFO(kfifo, struct ir_raw_event, RC_MAX_IR_EVENTS); /* for pulse/space durations */
ktime_t last_event; /* when last event occurred */
enum raw_event_type last_type; /* last event type */
struct rc_dev *dev; /* pointer to the parent rc_dev */
diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index 86f5aa7..9631825 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -21,15 +21,12 @@
#include <linux/freezer.h>
#include "rc-core-priv.h"
-/* Define the max number of pulse/space transitions to buffer */
-#define MAX_IR_EVENT_SIZE 512
-
-/* Used to keep track of IR raw clients, protected by ir_raw_handler_lock */
+/* IR raw clients/handlers, writers synchronize with ir_raw_mutex */
+static DEFINE_MUTEX(ir_raw_mutex);
static LIST_HEAD(ir_raw_client_list);
-
-/* Used to handle IR raw handler extensions */
-static DEFINE_MUTEX(ir_raw_handler_lock);
static LIST_HEAD(ir_raw_handler_list);
+
+/* protocols supported by the currently loaded decoders */
static u64 available_protocols;
static int ir_raw_event_thread(void *data)
@@ -37,32 +34,19 @@ static int ir_raw_event_thread(void *data)
struct ir_raw_event ev;
struct ir_raw_handler *handler;
struct ir_raw_event_ctrl *raw = (struct ir_raw_event_ctrl *)data;
- int retval;
while (!kthread_should_stop()) {
-
- spin_lock_irq(&raw->lock);
- retval = kfifo_len(&raw->kfifo);
-
- if (retval < sizeof(ev)) {
+ if (kfifo_out(&raw->kfifo, &ev, 1) == 0) {
set_current_state(TASK_INTERRUPTIBLE);
-
- if (kthread_should_stop())
- set_current_state(TASK_RUNNING);
-
- spin_unlock_irq(&raw->lock);
schedule();
continue;
}
- retval = kfifo_out(&raw->kfifo, &ev, sizeof(ev));
- spin_unlock_irq(&raw->lock);
-
- mutex_lock(&ir_raw_handler_lock);
- list_for_each_entry(handler, &ir_raw_handler_list, list)
+ rcu_read_lock();
+ list_for_each_entry_rcu(handler, &ir_raw_handler_list, list)
handler->decode(raw->dev, ev);
+ rcu_read_unlock();
raw->prev_ev = ev;
- mutex_unlock(&ir_raw_handler_lock);
}
return 0;
@@ -76,7 +60,8 @@ static int ir_raw_event_thread(void *data)
* This routine (which may be called from an interrupt context) stores a
* pulse/space duration for the raw ir decoding state machines. Pulses are
* signalled as positive values and spaces as negative values. A zero value
- * will reset the decoding state machines.
+ * will reset the decoding state machines. Drivers are responsible for
+ * synchronizing calls to this function.
*/
int ir_raw_event_store(struct rc_dev *dev, struct ir_raw_event *ev)
{
@@ -86,7 +71,7 @@ int ir_raw_event_store(struct rc_dev *dev, struct ir_raw_event *ev)
IR_dprintk(2, "sample: (%05dus %s)\n",
TO_US(ev->duration), TO_STR(ev->pulse));
- if (kfifo_in(&dev->raw->kfifo, ev, sizeof(*ev)) != sizeof(*ev))
+ if (kfifo_in(&dev->raw->kfifo, ev, 1) != 1)
return -ENOMEM;
return 0;
@@ -258,14 +243,8 @@ EXPORT_SYMBOL_GPL(ir_raw_get_tx_event);
*/
void ir_raw_event_handle(struct rc_dev *dev)
{
- unsigned long flags;
-
- if (!dev->raw)
- return;
-
- spin_lock_irqsave(&dev->raw->lock, flags);
- wake_up_process(dev->raw->thread);
- spin_unlock_irqrestore(&dev->raw->lock, flags);
+ if (dev->raw)
+ wake_up_process(dev->raw->thread);
}
EXPORT_SYMBOL_GPL(ir_raw_event_handle);
@@ -273,9 +252,9 @@ EXPORT_SYMBOL_GPL(ir_raw_event_handle);
static u64 ir_raw_get_allowed_protocols(struct rc_dev *dev)
{
u64 protocols;
- mutex_lock(&ir_raw_handler_lock);
+ mutex_lock(&ir_raw_mutex);
protocols = available_protocols;
- mutex_unlock(&ir_raw_handler_lock);
+ mutex_unlock(&ir_raw_mutex);
return protocols;
}
@@ -290,52 +269,49 @@ static int change_protocol(struct rc_dev *dev, u64 *rc_type) {
int rc_register_ir_raw_device(struct rc_dev *dev)
{
int rc;
+ struct ir_raw_event_ctrl *raw;
struct ir_raw_handler *handler;
if (!dev)
return -EINVAL;
- dev->raw = kzalloc(sizeof(*dev->raw), GFP_KERNEL);
- if (!dev->raw)
+ raw = kzalloc(sizeof(*raw), GFP_KERNEL);
+ if (!raw)
return -ENOMEM;
- dev->raw->dev = dev;
- dev->enabled_protocols = ~0;
- dev->get_protocols = ir_raw_get_allowed_protocols;
- dev->driver_type = RC_DRIVER_IR_RAW;
- dev->change_protocol = change_protocol;
- spin_lock_init(&dev->raw->lock);
- rc = kfifo_alloc(&dev->raw->kfifo,
- sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE,
- GFP_KERNEL);
- if (rc < 0)
- goto out;
-
- dev->raw->thread = kthread_run(ir_raw_event_thread, dev->raw,
- dev_name(&dev->dev));
+ raw->dev = dev;
+ INIT_KFIFO(raw->kfifo);
+ raw->thread = kthread_run(ir_raw_event_thread, dev->raw,
+ dev_name(&dev->dev));
- if (IS_ERR(dev->raw->thread)) {
- rc = PTR_ERR(dev->raw->thread);
+ if (IS_ERR(raw->thread)) {
+ rc = PTR_ERR(raw->thread);
goto out;
}
+ dev->raw = raw;
+ dev->enabled_protocols = ~0;
+ dev->get_protocols = ir_raw_get_allowed_protocols;
+ dev->driver_type = RC_DRIVER_IR_RAW;
+ dev->change_protocol = change_protocol;
rc = rc_register_device(dev);
if (rc < 0)
goto out_thread;
- mutex_lock(&ir_raw_handler_lock);
- list_add_tail(&dev->raw->list, &ir_raw_client_list);
- list_for_each_entry(handler, &ir_raw_handler_list, list)
+ mutex_lock(&ir_raw_mutex);
+ list_add_tail_rcu(&dev->raw->list, &ir_raw_client_list);
+ list_for_each_entry_rcu(handler, &ir_raw_handler_list, list)
if (handler->raw_register)
handler->raw_register(dev);
- mutex_unlock(&ir_raw_handler_lock);
+ mutex_unlock(&ir_raw_mutex);
+ synchronize_rcu();
return 0;
out_thread:
- kthread_stop(dev->raw->thread);
+ kthread_stop(raw->thread);
out:
- kfree(dev->raw);
+ kfree(raw);
dev->raw = NULL;
return rc;
}
@@ -350,14 +326,14 @@ void rc_unregister_ir_raw_device(struct rc_dev *dev)
kthread_stop(dev->raw->thread);
- mutex_lock(&ir_raw_handler_lock);
- list_del(&dev->raw->list);
- list_for_each_entry(handler, &ir_raw_handler_list, list)
+ mutex_lock(&ir_raw_mutex);
+ list_del_rcu(&dev->raw->list);
+ list_for_each_entry_rcu(handler, &ir_raw_handler_list, list)
if (handler->raw_unregister)
handler->raw_unregister(dev);
- mutex_unlock(&ir_raw_handler_lock);
+ mutex_unlock(&ir_raw_mutex);
+ synchronize_rcu();
- kfifo_free(&dev->raw->kfifo);
kfree(dev->raw);
dev->raw = NULL;
rc_unregister_device(dev);
@@ -372,13 +348,14 @@ int ir_raw_handler_register(struct ir_raw_handler *ir_raw_handler)
{
struct ir_raw_event_ctrl *raw;
- mutex_lock(&ir_raw_handler_lock);
- list_add_tail(&ir_raw_handler->list, &ir_raw_handler_list);
+ mutex_lock(&ir_raw_mutex);
+ list_add_tail_rcu(&ir_raw_handler->list, &ir_raw_handler_list);
if (ir_raw_handler->raw_register)
- list_for_each_entry(raw, &ir_raw_client_list, list)
+ list_for_each_entry_rcu(raw, &ir_raw_client_list, list)
ir_raw_handler->raw_register(raw->dev);
available_protocols |= ir_raw_handler->protocols;
- mutex_unlock(&ir_raw_handler_lock);
+ mutex_unlock(&ir_raw_mutex);
+ synchronize_rcu();
return 0;
}
@@ -388,13 +365,14 @@ void ir_raw_handler_unregister(struct ir_raw_handler *ir_raw_handler)
{
struct ir_raw_event_ctrl *raw;
- mutex_lock(&ir_raw_handler_lock);
- list_del(&ir_raw_handler->list);
+ mutex_lock(&ir_raw_mutex);
+ list_del_rcu(&ir_raw_handler->list);
if (ir_raw_handler->raw_unregister)
- list_for_each_entry(raw, &ir_raw_client_list, list)
+ list_for_each_entry_rcu(raw, &ir_raw_client_list, list)
ir_raw_handler->raw_unregister(raw->dev);
available_protocols &= ~ir_raw_handler->protocols;
- mutex_unlock(&ir_raw_handler_lock);
+ mutex_unlock(&ir_raw_mutex);
+ synchronize_rcu();
}
EXPORT_SYMBOL(ir_raw_handler_unregister);
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
