Re: [PATCH FOR 2.6.37] Revert "V4L/DVB: v4l2-dev: remove unnecessary lock around atomic clear_bit"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tuesday, November 23, 2010 01:21:45 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Tuesday 23 November 2010 01:14:28 Hans Verkuil wrote:
> > On Monday, November 22, 2010 12:20:25 Hans Verkuil wrote:
> > > On Monday, November 22, 2010 11:21:27 Laurent Pinchart wrote:
> > > > Removing the mutex_lock/unlock around clear_bit allowed
> > > > device_unregister() to race with v4l2_open(). The device can be
> > > > unregistered  between the video_devdata() and video_get() calls.
> > > > 
> > > > Revert the patch to fix the problem.
> > > > 
> > > > This reverts commit dd0daf2a6fb6bec436a3ef68bd585ea09a2a54b7.
> > > > 
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > 
> > > Acked-by: Hans Verkuil <hverkuil@xxxxxxxxx>
> > 
> > Change of mind, I nack this.
> > 
> > I realized that the mutex should be around device_unregister, not around
> > the clear_bit. The mutex protects access to vdev->dev and not vdev->flags
> > since that's atomic anyway.
> 
> device_unregister() will end up calling v4l2_device_release(), which also 
> locks videodev_lock. That will result in a deadlock.

True. (Note to myself: don't post past midnight...)

But a mutex around clear_bit doesn't help either. Unless v4l2_open will test
for this bit just before the video_get(). E.g. by replacing 'if (vdev == NULL)'
with 'if (vdev == NULL || !video_is_registered(vdev))'.

I think that will do the trick.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco
--
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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux