On Sun, Dec 12, 2010 at 05:13:47PM +0100, Torsten Kaiser wrote: > On Sun, Dec 12, 2010 at 2:15 PM, Dave Young <hidave.darkstar@xxxxxxxxx> wrote: > > oops happen in bttv_open while locking uninitialized mutex fh->cap.vb_lock > > add mutex_init before usage > > I have seen the same problem twice since I switched of the BKL in > 2.6.37-rc2, but only had the time today to search for the cause. > (It only happend on two boots out of probably more then 50, so I only > investigated after it happened a second time with -rc5) > > The cause was making this change to bttv_open() and radio_open() to > remove the BKL from them: > + mutex_lock(&fh->cap.vb_lock); > *fh = btv->init; > + mutex_unlock(&fh->cap.vb_lock); > > This is wrong because of two things: > First: The fh->cap.vb_lock has not been initialised, as you noted. > Second: The middle line will overwrite the mutex! > > Just adding mutex_init() will not really help the second problem and > seems to be wrong from the point that cap.vb_lock already has a > mutex_init() via videobuf_queue_sg_init(). Yes, you are right. I wonder if the above lock code in bttv_open and radio_open can be removed or just use btv->lock. > > I'm not sure what the correct fix is, but I would rather suggest this: > > * change &fh->cap.vb_lock in bttv_open() AND radio_open() to > &btv->init.cap.vb_lock > * add a mutex_init(&btv->init.cap.vb_lock) to the setup of init in bttv_probe() I will test this when the bttv card is available for me tommorrow. > > > Signed-off-by: Dave Young <hidave.darkstar@xxxxxxxxx> > > Tested-by: Chris Clayton <chris2553@xxxxxxxxxxxxxx> > > --- > > drivers/media/video/bt8xx/bttv-driver.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > --- linux-2.6.orig/drivers/media/video/bt8xx/bttv-driver.c 2010-11-27 11:21:30.000000000 +0800 > > +++ linux-2.6/drivers/media/video/bt8xx/bttv-driver.c 2010-12-12 16:31:39.633333338 +0800 > > @@ -3291,6 +3291,8 @@ static int bttv_open(struct file *file) > > fh = kmalloc(sizeof(*fh), GFP_KERNEL); > > if (unlikely(!fh)) > > return -ENOMEM; > > + > > + mutex_init(&fh->cap.vb_lock); > > file->private_data = fh; > > > > /* > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
