[linux-dvb] CX18 Oops
awalls at radix.net
Mon Aug 18 04:53:58 CEST 2008
On Sun, 2008-08-17 at 22:01 +0200, Hans Verkuil wrote:
> On Sunday 17 August 2008 21:12:50 Andy Walls wrote:
> > On Sun, 2008-08-17 at 11:41 +0200, Hans Verkuil wrote:
> > > On Sunday 17 August 2008 04:13:24 Andy Walls wrote:
> > > > On Mon, 2008-08-11 at 17:33 -0400, Brandon Jenkins wrote:
> > > > > On Fri, Aug 8, 2008 at 10:18 AM, Andy Walls <awalls at radix.net>
> > See my concern above. In brief, AFAICT, a system call on one
> > processor concurrent with interrupt service on another processor
> > requires the irq handler to obtain the proper lock before mucking
> > with the shared data structure.
> You are completely right and I stand corrected. cx18_queue_find_buf (aka
> cx18_queue_get_buf_irq) must have a spin_lock. So that spin_lock in
> ivtv wasn't bogus either :-)
> Damn, it's so easy to get confused with locking, even you've implemented
> it several times already.
Yup. And I found that "reading the code" without any sort of design
paperwork, design description, or reference textbooks makes locking
problems hard to spot.
I spent ~6 years on $BIG_PROJECT on a team maintaining highly
multithreaded applications that ran on an SMP machine. The apps used
spinlocks, mutexes (with and without condition variables), semaphores,
rendezvous, etc. A peer review of any significant code change usually
revealed at least one locking problem.
> That's a serious bug which needs to go into 2.6.27 (and probably to the
> 2.6.26 stable series as well).
> Andy, can you make a patch that adds the spin_lock to
> cx18_queue_find_buf(). It's better to do it there rather than in the
> interrupt routine.
> Then that patch can go into v4l-dvb and from there to 2.6.27. The other
> changes can come later.
Done. Pull request submitted.
> Apologies for probably confusing you. I certainly confused myself.
No big deal. I wasn't confused, but I did think I had missed something.
More information about the linux-dvb