Re: [PATCH] bttv: fix mutex use before init

[Brandon Philips]

On 19:45 Wed 15 Dec 2010, Mauro Carvalho Chehab wrote:
> Em 15-12-2010 16:44, Chris Clayton escreveu:
> > On Tuesday 14 December 2010, Brandon Philips wrote:
> >> On 17:13 Sun 12 Dec 2010, Torsten Kaiser wrote:
> >>>  * 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()
> >>
> >> That seems like a reasonable suggestion. An openSUSE user submitted
> >> this bug to our tracker too. Here is the patch I am having him
> >> test.
> >>
> >> Would you mind testing it?
> >>
> >> From 456dc0ce36db523c4c0c8a269f4eec43a72de1dc Mon Sep 17 00:00:00
> >> 2001 From: Brandon Philips <bphilips@xxxxxxx> Date: Mon, 13 Dec
> >> 2010 16:21:55 -0800 Subject: [PATCH] bttv: fix locking for
> >> btv->init
> >>
> >> Fix locking for the btv->init by using btv->init.cap.vb_lock and in
> >> the process fix uninitialized deref introduced in c37db91fd0d.
> >>
> >> Signed-off-by: Brandon Philips <bphilips@xxxxxxx>
> 
> While your patch fixes the issue, it has some other troubles, like to
> the presence of lock code at free_btres_lock(). It is possible to fix,
> but the better is to just use the core-assisted locking schema. This
> way, V4L2 core will serialize access to all
> ioctl's/open/close/mmap/read/poll operations, avoiding to have two
> processes accessing the hardware at the same time. Also, as there's
> just one lock, instead of 3, there's no risk of dead locks.

Thanks, but, why wasn't this done instead of c37db91f?

Will this make it in before 2.6.37 is released? Otherwise 2.6.37 will
need to be fixed in -stable immediatly after release.

> The net result is a cleaner code, with just one lock.

Could you take this patch to remove all of the comments about locking
order with btv->lock since it doesn't seem to matter any longer.

Cheers,

	Brandon

P.S. Your mail client creates really long lines- somewhere around 90
characters. Could you fix that?