Em Fri, 04 Apr 2014 01:34:38 +0200 David Härdeman <[email protected]> escreveu:
> Simplify and improve the locking in rc-ir-raw by making better use of > the existing kfifo functionality and by using RCU where possible. Please rebase. > > Signed-off-by: David Härdeman <[email protected]> > --- > 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 [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
