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

[Andy W. <aw...@md...>]

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 <max...@gm...>
> ---
>  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);
> +
>  }
>  
>  /**