[v2] bttv: take correct lock in bttv_open()

Message ID 20101212165812.GG10623@bicker (mailing list archive)
State Superseded, archived
Headers

Commit Message

Dan Carpenter Dec. 12, 2010, 4:58 p.m. UTC
  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 

Signed-off-by: Dan Carpenter <error27@gmail.com>
---
Sergej could you test this one?

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Comments

Sergej Pupykin Dec. 12, 2010, 10:42 p.m. UTC | #1
On 12.12.2010 19:58, Dan Carpenter wrote:
> 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.
>
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Patch

diff --git a/drivers/media/video/bt8xx/bttv-driver.c b/drivers/media/video/bt8xx/bttv-driver.c
index a529619..6c8f4b0 100644
--- a/drivers/media/video/bt8xx/bttv-driver.c
+++ b/drivers/media/video/bt8xx/bttv-driver.c
@@ -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;