[linux-dvb] CX18 Oops

[Andy Walls]

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.


Regards,
Andy

> Regards,
> 
> 	Hans
>