Google
  Web www.spinics.net

Re: [hg:v4l-dvb] With the recent patch to v4l2 titled "v4l2: use register_chrdev_region

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


There is a small bug in this patch that was revealed during testing.
Please hold off on this until we have finished testing. There is now a
series of 3 patches that replaces this one.

> @@ -1478,7 +1437,10 @@ static void stk_camera_disconnect(struct
>        wake_up_interruptible(&dev->wait_frame);
>        stk_remove_sysfs_files(&dev->vdev);
>
> -       kref_put(&dev->kref, stk_camera_cleanup);
> +       STK_INFO("Syntek USB2.0 Camera release resources"
The bug in this patch is here ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
There should be a space after resources inside the quotes.

> +               "video device /dev/video%d\n", dev->vdev.minor);
> +
> +       video_unregister_device(&dev->vdev);
>  }

The replacement patches are as follows:
The first patch should fix the crash found by Jamie. usb_kill_urb
should not be called when the device is nolonger present. This is the
case when the device is physically disconnected while it is still
open. The subsequent close triggers a crash. This patch still has yet
to be tested. Please test this patch before applying. I don't own the
appropriate hardware and am therefore unable to do the appropriate
testing needed.

The last two replace the above patch. The division in the above patch
was made since simplifying access to the stk_camera struct is not
dependent on removing the reference counter and vice versa. The above
mentioned bug has been fixed.

Regards,

David Ellingsworth

------------------------------------------------------------------------------------------------------------
>From 2caae22f93469976bfd1266895da39bcf7bfbbe2 Mon Sep 17 00:00:00 2001
From: David Ellingsworth <david@xxxxxxxxxxxxxxxxx>
Date: Thu, 25 Sep 2008 20:59:34 -0400
Subject: [PATCH] stkwebcam: fix crash on disconnect before close


Signed-off-by: David Ellingsworth <david@xxxxxxxxxxxxxxxxx>
---
 drivers/media/video/stk-webcam.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/stk-webcam.c b/drivers/media/video/stk-webcam.c
index 8dda568..bbf3677 100644
--- a/drivers/media/video/stk-webcam.c
+++ b/drivers/media/video/stk-webcam.c
@@ -576,7 +576,7 @@ static void stk_clean_iso(struct stk_camera *dev)

 		urb = dev->isobufs[i].urb;
 		if (urb) {
-			if (atomic_read(&dev->urbs_used))
+			if (atomic_read(&dev->urbs_used) && is_present(dev))
 				usb_kill_urb(urb);
 			usb_free_urb(urb);
 		}
-- 
1.5.6

------------------------------------------------------------------------------------------------------------
>From bd669a31bc62761efd6c981887b61ecba6389850 Mon Sep 17 00:00:00 2001
From: David Ellingsworth <david@xxxxxxxxxxxxxxxxx>
Date: Fri, 26 Sep 2008 18:25:14 -0400
Subject: [PATCH] stkwebcam: free via video_device release callback


Signed-off-by: David Ellingsworth <david@xxxxxxxxxxxxxxxxx>
---
 drivers/media/video/stk-webcam.c |   44 +++++++++++++++----------------------
 drivers/media/video/stk-webcam.h |    2 -
 2 files changed, 18 insertions(+), 28 deletions(-)

diff --git a/drivers/media/video/stk-webcam.c b/drivers/media/video/stk-webcam.c
index bbf3677..0a3a2de 100644
--- a/drivers/media/video/stk-webcam.c
+++ b/drivers/media/video/stk-webcam.c
@@ -65,22 +65,6 @@ static struct usb_device_id stkwebcam_table[] = {
 };
 MODULE_DEVICE_TABLE(usb, stkwebcam_table);

-static void stk_camera_cleanup(struct kref *kref)
-{
-	struct stk_camera *dev = to_stk_camera(kref);
-
-	STK_INFO("Syntek USB2.0 Camera release resources"
-		" video device /dev/video%d\n", dev->vdev.minor);
-	video_unregister_device(&dev->vdev);
-	video_set_drvdata(&dev->vdev, NULL);
-
-	if (dev->sio_bufs != NULL || dev->isobufs != NULL)
-		STK_ERROR("We are leaking memory\n");
-	usb_put_intf(dev->interface);
-	kfree(dev);
-}
-
-
 /*
  * Basic stuff
  */
@@ -695,7 +679,6 @@ static int v4l_stk_open(struct inode *inode,
struct file *fp)
 		return -ENXIO;
 	}
 	fp->private_data = vdev;
-	kref_get(&dev->kref);
 	usb_autopm_get_interface(dev->interface);
 	unlock_kernel();

@@ -720,7 +703,6 @@ static int v4l_stk_release(struct inode *inode,
struct file *fp)

 	if (dev->owner != fp) {
 		usb_autopm_put_interface(dev->interface);
-		kref_put(&dev->kref, stk_camera_cleanup);
 		return 0;
 	}

@@ -731,7 +713,6 @@ static int v4l_stk_release(struct inode *inode,
struct file *fp)
 	dev->owner = NULL;

 	usb_autopm_put_interface(dev->interface);
-	kref_put(&dev->kref, stk_camera_cleanup);

 	return 0;
 }
@@ -1359,6 +1340,12 @@ static const struct v4l2_ioctl_ops v4l_stk_ioctl_ops = {

 static void stk_v4l_dev_release(struct video_device *vd)
 {
+	struct stk_camera *dev = vdev_to_camera(vd);
+
+	if (dev->sio_bufs != NULL || dev->isobufs != NULL)
+		STK_ERROR("We are leaking memory\n");
+	usb_put_intf(dev->interface);
+	kfree(dev);
 }

 static struct video_device stk_v4l_data = {
@@ -1396,7 +1383,7 @@ static int stk_camera_probe(struct usb_interface
*interface,
 		const struct usb_device_id *id)
 {
 	int i;
-	int err;
+	int err = 0;

 	struct stk_camera *dev = NULL;
 	struct usb_device *udev = interface_to_usbdev(interface);
@@ -1409,7 +1396,6 @@ static int stk_camera_probe(struct usb_interface
*interface,
 		return -ENOMEM;
 	}

-	kref_init(&dev->kref);
 	spin_lock_init(&dev->spinlock);
 	init_waitqueue_head(&dev->wait_frame);

@@ -1442,8 +1428,8 @@ static int stk_camera_probe(struct usb_interface
*interface,
 	}
 	if (!dev->isoc_ep) {
 		STK_ERROR("Could not find isoc-in endpoint");
-		kref_put(&dev->kref, stk_camera_cleanup);
-		return -ENODEV;
+		err = -ENODEV;
+		goto error;
 	}
 	dev->vsettings.brightness = 0x7fff;
 	dev->vsettings.palette = V4L2_PIX_FMT_RGB565;
@@ -1457,14 +1443,17 @@ static int stk_camera_probe(struct
usb_interface *interface,

 	err = stk_register_video_device(dev);
 	if (err) {
-		kref_put(&dev->kref, stk_camera_cleanup);
-		return err;
+		goto error;
 	}

 	stk_create_sysfs_files(&dev->vdev);
 	usb_autopm_enable(dev->interface);

 	return 0;
+
+error:
+	kfree(dev);
+	return err;
 }

 static void stk_camera_disconnect(struct usb_interface *interface)
@@ -1477,7 +1466,10 @@ static void stk_camera_disconnect(struct
usb_interface *interface)
 	wake_up_interruptible(&dev->wait_frame);
 	stk_remove_sysfs_files(&dev->vdev);

-	kref_put(&dev->kref, stk_camera_cleanup);
+	STK_INFO("Syntek USB2.0 Camera release resources "
+		"video device /dev/video%d\n", dev->vdev.minor);
+
+	video_unregister_device(&dev->vdev);
 }

 #ifdef CONFIG_PM
diff --git a/drivers/media/video/stk-webcam.h b/drivers/media/video/stk-webcam.h
index df4dfef..084a85b 100644
--- a/drivers/media/video/stk-webcam.h
+++ b/drivers/media/video/stk-webcam.h
@@ -99,7 +99,6 @@ struct stk_camera {

 	u8 isoc_ep;

-	struct kref kref;
 	/* Not sure if this is right */
 	atomic_t urbs_used;

@@ -121,7 +120,6 @@ struct stk_camera {
 	unsigned sequence;
 };

-#define to_stk_camera(d) container_of(d, struct stk_camera, kref)
 #define vdev_to_camera(d) container_of(d, struct stk_camera, vdev)

 void stk_camera_delete(struct kref *);
-- 
1.5.6

-------------------------------------------------------------------------------------------------
>From b5886307fe11ed3e7684137cfef2766dc6b11ec4 Mon Sep 17 00:00:00 2001
From: David Ellingsworth <david@xxxxxxxxxxxxxxxxx>
Date: Thu, 25 Sep 2008 21:00:57 -0400
Subject: [PATCH] stkwebcam: simplify access to stk_camera struct


Signed-off-by: David Ellingsworth <david@xxxxxxxxxxxxxxxxx>
---
 drivers/media/video/stk-webcam.c |   40 ++++---------------------------------
 1 files changed, 5 insertions(+), 35 deletions(-)

diff --git a/drivers/media/video/stk-webcam.c b/drivers/media/video/stk-webcam.c
index 7ad97c7..c3e7d66 100644
--- a/drivers/media/video/stk-webcam.c
+++ b/drivers/media/video/stk-webcam.c
@@ -678,7 +678,7 @@ static int v4l_stk_open(struct inode *inode,
struct file *fp)
 		unlock_kernel();
 		return -ENXIO;
 	}
-	fp->private_data = vdev;
+	fp->private_data = dev;
 	usb_autopm_get_interface(dev->interface);
 	unlock_kernel();

@@ -687,19 +687,7 @@ static int v4l_stk_open(struct inode *inode,
struct file *fp)

 static int v4l_stk_release(struct inode *inode, struct file *fp)
 {
-	struct stk_camera *dev;
-	struct video_device *vdev;
-
-	vdev = video_devdata(fp);
-	if (vdev == NULL) {
-		STK_ERROR("v4l_release called w/o video devdata\n");
-		return -EFAULT;
-	}
-	dev = vdev_to_camera(vdev);
-	if (dev == NULL) {
-		STK_ERROR("v4l_release called on removed device\n");
-		return -ENODEV;
-	}
+	struct stk_camera *dev = fp->private_data;

 	if (dev->owner != fp) {
 		usb_autopm_put_interface(dev->interface);
@@ -723,14 +711,8 @@ static ssize_t v4l_stk_read(struct file *fp, char
__user *buf,
 	int i;
 	int ret;
 	unsigned long flags;
-	struct stk_camera *dev;
-	struct video_device *vdev;
 	struct stk_sio_buffer *sbuf;
-
-	vdev = video_devdata(fp);
-	if (vdev == NULL)
-		return -EFAULT;
-	dev = vdev_to_camera(vdev);
+	struct stk_camera *dev = fp->private_data;

 	if (dev == NULL)
 		return -EIO;
@@ -789,15 +771,8 @@ static ssize_t v4l_stk_read(struct file *fp, char
__user *buf,

 static unsigned int v4l_stk_poll(struct file *fp, poll_table *wait)
 {
-	struct stk_camera *dev;
-	struct video_device *vdev;
-
-	vdev = video_devdata(fp);
-
-	if (vdev == NULL)
-		return -EFAULT;
+	struct stk_camera *dev = fp->private_data;

-	dev = vdev_to_camera(vdev);
 	if (dev == NULL)
 		return -ENODEV;

@@ -835,16 +810,12 @@ static int v4l_stk_mmap(struct file *fp, struct
vm_area_struct *vma)
 	unsigned int i;
 	int ret;
 	unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
-	struct stk_camera *dev;
-	struct video_device *vdev;
+	struct stk_camera *dev = fp->private_data;
 	struct stk_sio_buffer *sbuf = NULL;

 	if (!(vma->vm_flags & VM_WRITE) || !(vma->vm_flags & VM_SHARED))
 		return -EINVAL;

-	vdev = video_devdata(fp);
-	dev = vdev_to_camera(vdev);
-
 	for (i = 0; i < dev->n_sbufs; i++) {
 		if (dev->sio_bufs[i].v4lbuf.m.offset == offset) {
 			sbuf = dev->sio_bufs + i;
@@ -1366,7 +1337,6 @@ static int stk_register_video_device(struct
stk_camera *dev)
 	dev->vdev = stk_v4l_data;
 	dev->vdev.debug = debug;
 	dev->vdev.parent = &dev->interface->dev;
-	video_set_drvdata(&dev->vdev, dev);
 	err = video_register_device(&dev->vdev, VFL_TYPE_GRABBER, -1);
 	if (err)
 		STK_ERROR("v4l registration failed\n");
-- 
1.5.6
_______________________________________________
v4l-dvb-maintainer mailing list
v4l-dvb-maintainer@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer

[Linux Media]     [Older V4L]     [Linux DVB]     [Video Disk Recorder]     [Asterisk]     [Photo]     [DCCP]     [Netdev]     [Xorg]     [Util Linux NG]     [Xfree86]     [Free Photo Albums]     [Fedora Users]     [Fedora Women]     [ALSA Users]     [ALSA Devel]     [SSH]     [Linux USB]

-->
Add to Google Powered by Linux

Google PageRank Checking tool