| Date | Thu, 11 Sep 2008 09:22:26 -0600 | From | Jonathan Corbet <> | Subject | Re: [PATCH 09/18] lirc driver for the Streamzap PC Receiver |
| |
[Yes, you said I shouldn't do more reviews of the posted code, but the following was already pretty much written. I really only found one thing in this driver, and I suspect it may not have been addressed yet.
This was set to be the last anyway, I'm pretty much out of time for now.]
jon
I'm curious about the following function:
> +static void stop_timer(struct usb_streamzap *sz) > +{ > + unsigned long flags; > + > + spin_lock_irqsave(&sz->timer_lock, flags); > + if (sz->timer_running) {
What happens if the timer fires right here?
> + sz->timer_running = 0; > + del_timer_sync(&sz->delay_timer); > + } > + spin_unlock_irqrestore(&sz->timer_lock, flags); > +}
The timer acquires the lock too, so this looks a bit like a deadlock to me. Probably del_timer_sync() should be called without the lock?
|