Re: [PATCH 04/13] IR: fix locking in ir_raw_event_work

Andy Walls
2010-07-30T02:42:29Z

On Fri, 2010-07-30 at 05:17 +0300, Maxim Levitsky wrote:
> It is prefectly possible to have ir_raw_event_work
> running concurently on two cpus, thus we must protect
> it from that situation.

Yup, the work is marked as not pending (and hence reschedulable) just
before the work handler is run.

> Maybe better solution is to ditch the workqueue at all
> and use good 'ol thread per receiver, and just wake it up...

I suppose you could also use a single threaded workqueue instead of a
mutex, and let a bit test provide exclusivity. With the mutex, when the
second thread finally obtains the lock, there will likely not be
anything for it to do.

Regards,
Andy

> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> ---
> drivers/media/IR/ir-raw-event.c | 7 +++++--
> 1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c
> index 9d5c029..4098748 100644
> --- a/drivers/media/IR/ir-raw-event.c
> +++ b/drivers/media/IR/ir-raw-event.c
> @@ -40,13 +40,16 @@ static void ir_raw_event_work(struct work_struct *work)
> struct ir_raw_event_ctrl *raw =
> container_of(work, struct ir_raw_event_ctrl, rx_work);
>
> + mutex_lock(&ir_raw_handler_lock);
> +
> while (kfifo_out(&raw->kfifo, &ev, sizeof(ev)) == sizeof(ev)) {
> - mutex_lock(&ir_raw_handler_lock);
> list_for_each_entry(handler, &ir_raw_handler_list, list)
> handler->decode(raw->input_dev, ev);
> - mutex_unlock(&ir_raw_handler_lock);
> raw->prev_ev = ev;
> }
> +
> + mutex_unlock(&ir_raw_handler_lock);
> +
> }
>
> /**