bttv: take correct lock in bttv_open()

We're trying to make sure that no one is writing to the btv->init struct while we copy it over to the newly allocated "fh" struct. The original code doesn't make sense because "fh->cap.vb_lock" hasn't been initialized and no one else can be writing to it anyway.

Addresses: https://bugzilla.kernel.org/show_bug.cgi?id=24602

---
Sergej could you test this one?

index a529619..6c8f4b0 100644
@@ -3302,9 +3302,9 @@ static int bttv_open(struct file *file)
 	 * Let's first copy btv->init at fh, holding cap.vb_lock, and then work
 	 * with the rest of init, holding btv->lock.
 	 */
-	mutex_lock(&fh->cap.vb_lock);
+	mutex_lock(&btv->init.cap.vb_lock);
 	*fh = btv->init;
-	mutex_unlock(&fh->cap.vb_lock);
+	mutex_unlock(&btv->init.cap.vb_lock);
 
 	fh->type = type;
 	fh->ov.setup_ok = 0;

This patch also crashes the system. Unfortunately machine hangs, so I can not copy-paste trace. It was something about nosemaphore called from bttv_open. (something like previous reports)

I replace lock with btv->lock:

mutex_lock(&btv->lock);
*fh = btv->init;
mutex_unlock(&btv->lock);

Probably it is overkill and may be incorrect, but it starts working.

Also I found another issue: tvtime hangs on exit in D-state, so it looks like there is a problem near bttv_release function or something like this.

Mauro would be the one to know for sure.

Speaking of other bugs in this driver, I submitted a another fix that hasn't been merged yet. I've attached it. Don't know if it's related at all to the other bug you noticed but it can't hurt.

regards,
dan carpenter

Date: Thu, 18 Nov 2010 06:55:59 +0300
From: Dan Carpenter <[email protected]>
To: Mauro Carvalho Chehab <[email protected]>
Cc: [email protected], [email protected]
Subject: [patch] [media] bt8xx: missing unlock in bttv_overlay()
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=utf-8
Status: RO

There is a missing unlock here. This was introduced as part of BKL removal in c37db91fd0d4 "V4L/DVB: bttv: fix driver lock and remove explicit calls to BKL"

index 3da6e80..aca755c 100644
@@ -2779,16 +2779,14 @@ static int bttv_overlay(struct file *file, void *f, unsigned int on)
 		mutex_lock(&fh->cap.vb_lock);
 		/* verify args */
 		if (unlikely(!btv->fbuf.base)) {
-			mutex_unlock(&fh->cap.vb_lock);
-			return -EINVAL;
-		}
-		if (unlikely(!fh->ov.setup_ok)) {
+			retval = -EINVAL;
+		} else if (unlikely(!fh->ov.setup_ok)) {
 			dprintk("bttv%d: overlay: !setup_ok\n", btv->c.nr);
 			retval = -EINVAL;
 		}
+		mutex_unlock(&fh->cap.vb_lock);
 		if (retval)
 			return retval;
-		mutex_unlock(&fh->cap.vb_lock);
 	}
 
 	if (!check_alloc_btres_lock(btv, fh, RESOURCE_OVERLAY))

Em 14-12-2010 08:36, Dan Carpenter escreveu:

I'm preparing one machine in order to test bttv and try to fix the locking issues. Hopefully, I'll have something today.

Cheers,
Mauro