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@xxxxxxxxx> > > 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 > _______________________________________________ linux-dvb mailing list linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
