Skip to content

Commit

Permalink
V4L/DVB (13152): uvcvideo: Rely on videodev to reference-count the de…
Browse files Browse the repository at this point in the history 
…vice

The uvcvideo driver has a driver-wide lock and a reference count to protect
against a disconnect/open race. Now that videodev handles the race itself,
reference-counting in the driver can be removed.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
  • Loading branch information
Laurent Pinchart authored and Mauro Carvalho Chehab committed Dec 5, 2009
1 parent 1a969d9 commit 716fdee
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 104 deletions.
155 changes: 79 additions & 76 deletions drivers/media/video/uvc/uvc_driver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1530,23 +1530,93 @@ static int uvc_scan_device(struct uvc_device *dev)
* Video device registration and unregistration
*/

/*
* Delete the UVC device.
*
* Called by the kernel when the last reference to the uvc_device structure
* is released.
*
* As this function is called after or during disconnect(), all URBs have
* already been canceled by the USB core. There is no need to kill the
* interrupt URB manually.
*/
static void uvc_delete(struct uvc_device *dev)
{
struct list_head *p, *n;

usb_put_intf(dev->intf);
usb_put_dev(dev->udev);

uvc_status_cleanup(dev);
uvc_ctrl_cleanup_device(dev);

list_for_each_safe(p, n, &dev->chains) {
struct uvc_video_chain *chain;
chain = list_entry(p, struct uvc_video_chain, list);
kfree(chain);
}

list_for_each_safe(p, n, &dev->entities) {
struct uvc_entity *entity;
entity = list_entry(p, struct uvc_entity, list);
kfree(entity);
}

list_for_each_safe(p, n, &dev->streams) {
struct uvc_streaming *streaming;
streaming = list_entry(p, struct uvc_streaming, list);
usb_driver_release_interface(&uvc_driver.driver,
streaming->intf);
usb_put_intf(streaming->intf);
kfree(streaming->format);
kfree(streaming->header.bmaControls);
kfree(streaming);
}

kfree(dev);
}

static void uvc_release(struct video_device *vdev)
{
struct uvc_streaming *stream = video_get_drvdata(vdev);
struct uvc_device *dev = stream->dev;

video_device_release(vdev);

/* Decrement the registered streams count and delete the device when it
* reaches zero.
*/
if (atomic_dec_and_test(&dev->nstreams))
uvc_delete(dev);
}

/*
* Unregister the video devices.
*/
static void uvc_unregister_video(struct uvc_device *dev)
{
struct uvc_streaming *stream;

/* Unregistering all video devices might result in uvc_delete() being
* called from inside the loop if there's no open file handle. To avoid
* that, increment the stream count before iterating over the streams
* and decrement it when done.
*/
atomic_inc(&dev->nstreams);

list_for_each_entry(stream, &dev->streams, list) {
if (stream->vdev == NULL)
continue;

if (stream->vdev->minor == -1)
video_device_release(stream->vdev);
else
video_unregister_device(stream->vdev);
video_unregister_device(stream->vdev);
stream->vdev = NULL;
}

/* Decrement the stream count and call uvc_delete explicitly if there
* are no stream left.
*/
if (atomic_dec_and_test(&dev->nstreams))
uvc_delete(dev);
}

static int uvc_register_video(struct uvc_device *dev,
Expand Down Expand Up @@ -1580,7 +1650,7 @@ static int uvc_register_video(struct uvc_device *dev,
vdev->parent = &dev->intf->dev;
vdev->minor = -1;
vdev->fops = &uvc_fops;
vdev->release = video_device_release;
vdev->release = uvc_release;
strlcpy(vdev->name, dev->name, sizeof vdev->name);

/* Set the driver data before calling video_register_device, otherwise
Expand All @@ -1598,6 +1668,7 @@ static int uvc_register_video(struct uvc_device *dev,
return ret;
}

atomic_inc(&dev->nstreams);
return 0;
}

Expand Down Expand Up @@ -1653,61 +1724,6 @@ static int uvc_register_chains(struct uvc_device *dev)
* USB probe, disconnect, suspend and resume
*/

/*
* Delete the UVC device.
*
* Called by the kernel when the last reference to the uvc_device structure
* is released.
*
* Unregistering the video devices is done here because every opened instance
* must be closed before the device can be unregistered. An alternative would
* have been to use another reference count for uvc_v4l2_open/uvc_release, and
* unregister the video devices on disconnect when that reference count drops
* to zero.
*
* As this function is called after or during disconnect(), all URBs have
* already been canceled by the USB core. There is no need to kill the
* interrupt URB manually.
*/
void uvc_delete(struct kref *kref)
{
struct uvc_device *dev = container_of(kref, struct uvc_device, kref);
struct list_head *p, *n;

/* Unregister the video devices. */
uvc_unregister_video(dev);
usb_put_intf(dev->intf);
usb_put_dev(dev->udev);

uvc_status_cleanup(dev);
uvc_ctrl_cleanup_device(dev);

list_for_each_safe(p, n, &dev->chains) {
struct uvc_video_chain *chain;
chain = list_entry(p, struct uvc_video_chain, list);
kfree(chain);
}

list_for_each_safe(p, n, &dev->entities) {
struct uvc_entity *entity;
entity = list_entry(p, struct uvc_entity, list);
kfree(entity);
}

list_for_each_safe(p, n, &dev->streams) {
struct uvc_streaming *streaming;
streaming = list_entry(p, struct uvc_streaming, list);
usb_driver_release_interface(&uvc_driver.driver,
streaming->intf);
usb_put_intf(streaming->intf);
kfree(streaming->format);
kfree(streaming->header.bmaControls);
kfree(streaming);
}

kfree(dev);
}

static int uvc_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
Expand All @@ -1730,7 +1746,7 @@ static int uvc_probe(struct usb_interface *intf,
INIT_LIST_HEAD(&dev->entities);
INIT_LIST_HEAD(&dev->chains);
INIT_LIST_HEAD(&dev->streams);
kref_init(&dev->kref);
atomic_set(&dev->nstreams, 0);
atomic_set(&dev->users, 0);

dev->udev = usb_get_dev(udev);
Expand Down Expand Up @@ -1792,7 +1808,7 @@ static int uvc_probe(struct usb_interface *intf,
return 0;

error:
kref_put(&dev->kref, uvc_delete);
uvc_unregister_video(dev);
return -ENODEV;
}

Expand All @@ -1809,21 +1825,9 @@ static void uvc_disconnect(struct usb_interface *intf)
UVC_SC_VIDEOSTREAMING)
return;

/* uvc_v4l2_open() might race uvc_disconnect(). A static driver-wide
* lock is needed to prevent uvc_disconnect from releasing its
* reference to the uvc_device instance after uvc_v4l2_open() received
* the pointer to the device (video_devdata) but before it got the
* chance to increase the reference count (kref_get).
*
* Note that the reference can't be released with the lock held,
* otherwise a AB-BA deadlock can occur with videodev_lock that
* videodev acquires in videodev_open() and video_unregister_device().
*/
mutex_lock(&uvc_driver.open_mutex);
dev->state |= UVC_DEV_DISCONNECTED;
mutex_unlock(&uvc_driver.open_mutex);

kref_put(&dev->kref, uvc_delete);
uvc_unregister_video(dev);
}

static int uvc_suspend(struct usb_interface *intf, pm_message_t message)
Expand Down Expand Up @@ -2159,7 +2163,6 @@ static int __init uvc_init(void)

INIT_LIST_HEAD(&uvc_driver.devices);
INIT_LIST_HEAD(&uvc_driver.controls);
mutex_init(&uvc_driver.open_mutex);
mutex_init(&uvc_driver.ctrl_mutex);

uvc_ctrl_init();
Expand Down
32 changes: 8 additions & 24 deletions drivers/media/video/uvc/uvc_v4l2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,25 +376,18 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream,
*/
static int uvc_acquire_privileges(struct uvc_fh *handle)
{
int ret = 0;

/* Always succeed if the handle is already privileged. */
if (handle->state == UVC_HANDLE_ACTIVE)
return 0;

/* Check if the device already has a privileged handle. */
mutex_lock(&uvc_driver.open_mutex);
if (atomic_inc_return(&handle->stream->active) != 1) {
atomic_dec(&handle->stream->active);
ret = -EBUSY;
goto done;
return -EBUSY;
}

handle->state = UVC_HANDLE_ACTIVE;

done:
mutex_unlock(&uvc_driver.open_mutex);
return ret;
return 0;
}

static void uvc_dismiss_privileges(struct uvc_fh *handle)
Expand All @@ -421,24 +414,20 @@ static int uvc_v4l2_open(struct file *file)
int ret = 0;

uvc_trace(UVC_TRACE_CALLS, "uvc_v4l2_open\n");
mutex_lock(&uvc_driver.open_mutex);
stream = video_drvdata(file);

if (stream->dev->state & UVC_DEV_DISCONNECTED) {
ret = -ENODEV;
goto done;
}
if (stream->dev->state & UVC_DEV_DISCONNECTED)
return -ENODEV;

ret = usb_autopm_get_interface(stream->dev->intf);
if (ret < 0)
goto done;
return ret;

/* Create the device handle. */
handle = kzalloc(sizeof *handle, GFP_KERNEL);
if (handle == NULL) {
usb_autopm_put_interface(stream->dev->intf);
ret = -ENOMEM;
goto done;
return -ENOMEM;
}

if (atomic_inc_return(&stream->dev->users) == 1) {
Expand All @@ -447,7 +436,7 @@ static int uvc_v4l2_open(struct file *file)
usb_autopm_put_interface(stream->dev->intf);
atomic_dec(&stream->dev->users);
kfree(handle);
goto done;
return ret;
}
}

Expand All @@ -456,11 +445,7 @@ static int uvc_v4l2_open(struct file *file)
handle->state = UVC_HANDLE_PASSIVE;
file->private_data = handle;

kref_get(&stream->dev->kref);

done:
mutex_unlock(&uvc_driver.open_mutex);
return ret;
return 0;
}

static int uvc_v4l2_release(struct file *file)
Expand Down Expand Up @@ -490,7 +475,6 @@ static int uvc_v4l2_release(struct file *file)
uvc_status_stop(stream->dev);

usb_autopm_put_interface(stream->dev->intf);
kref_put(&stream->dev->kref, uvc_delete);
return 0;
}

Expand Down
5 changes: 1 addition & 4 deletions drivers/media/video/uvc/uvcvideo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,6 @@ struct uvc_device {
char name[32];

enum uvc_device_state state;
struct kref kref;
struct list_head list;
atomic_t users;

Expand All @@ -488,6 +487,7 @@ struct uvc_device {

/* Video Streaming interfaces */
struct list_head streams;
atomic_t nstreams;

/* Status Interrupt Endpoint */
struct usb_host_endpoint *int_ep;
Expand All @@ -511,8 +511,6 @@ struct uvc_fh {
struct uvc_driver {
struct usb_driver driver;

struct mutex open_mutex; /* protects from open/disconnect race */

struct list_head devices; /* struct uvc_device list */
struct list_head controls; /* struct uvc_control_info list */
struct mutex ctrl_mutex; /* protects controls and devices
Expand Down Expand Up @@ -572,7 +570,6 @@ extern unsigned int uvc_trace_param;

/* Core driver */
extern struct uvc_driver uvc_driver;
extern void uvc_delete(struct kref *kref);

/* Video buffers queue management. */
extern void uvc_queue_init(struct uvc_video_queue *queue,
Expand Down

0 comments on commit 716fdee

Please sign in to comment.