* Re: [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
[not found] <200811151218.45664.m.kozlowski@tuxland.pl>
@ 2008-11-15 18:48 ` Jean-Francois Moine
[not found] ` <200811162224.47885.m.kozlowski@tuxland.pl>
0 siblings, 1 reply; 13+ messages in thread
From: Jean-Francois Moine @ 2008-11-15 18:48 UTC (permalink / raw)
To: Mariusz Kozlowski; +Cc: video4linux-list, v4l-dvb-maintainer
On Sat, 2008-11-15 at 12:18 +0100, Mariusz Kozlowski wrote:
> Hi,
>
> Recently I bought one of these cheap usb cameras. This would be
> Logitech e1000 identified as 046d:08af Logitech, Inc. It doesn't quite
> work but that's another story. On unplug though it causes oops
> while /dev/video is open.
>
> Steps to reproduce:
>
> a) plug the camera in (zc3xx as a module)
> b) wait for it to settle down
> c) cat /dev/video > /dev/null
> d) unplug the camera
Hi Mariusz,
Do you have the same oops when streaming by mmap (most apps) or by
userptr (svv)?
Cheers.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
[not found] ` <200811162224.47885.m.kozlowski@tuxland.pl>
@ 2008-11-18 18:57 ` Jean-Francois Moine
[not found] ` <200811182219.38925.m.kozlowski@tuxland.pl>
0 siblings, 1 reply; 13+ messages in thread
From: Jean-Francois Moine @ 2008-11-18 18:57 UTC (permalink / raw)
To: Mariusz Kozlowski; +Cc: video4linux-list, v4l-dvb-maintainer
On Sun, 2008-11-16 at 22:24 +0100, Mariusz Kozlowski wrote:
> > > Steps to reproduce:
> > >
> > > a) plug the camera in (zc3xx as a module)
> > > b) wait for it to settle down
> > > c) cat /dev/video > /dev/null
> > > d) unplug the camera
Hi Mariusz,
Thank you for the traces. I found the problem and I updated my
repository (http://linuxtv.org/hg/~jfrancois/gspca/).
May you try if everything works now?
Cheers.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
[not found] ` <200811182219.38925.m.kozlowski@tuxland.pl>
@ 2008-11-19 10:32 ` Jean-Francois Moine
2008-11-19 13:52 ` David Ellingsworth
0 siblings, 1 reply; 13+ messages in thread
From: Jean-Francois Moine @ 2008-11-19 10:32 UTC (permalink / raw)
To: Mariusz Kozlowski; +Cc: video4linux-list, v4l-dvb-maintainer
[-- Attachment #1: Type: text/plain, Size: 902 bytes --]
On Tue, 2008-11-18 at 22:19 +0100, Mariusz Kozlowski wrote:
> and it didn't fix it. It didn't apply cleanly - there
> was some offset during patching if that matters.
[snip]
> If you could provide patches against some mainline kernel versions
> like 2.6.28-rc5 that would be great - and please specify which bits
> exactly should I patch to avoid confusion.
>
> BTW. Can you reproduce the oops I'm seeing?
Hi Mariusz,
You have the oops thanks to poison and it is not enabled in my kernel.
I found the real bug: the device structure was part of the gspca device
and it was freed on close after webcam unplug while streaming.
I join a patch I merged from the linux-2.6.28-rc5 source (not compiled -
the original patch is the last one in my mercurial repository).
Thanks again.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
[-- Attachment #2: gspca.patch --]
[-- Type: text/x-patch, Size: 3546 bytes --]
--- linux-2.26.8-rc5/drivers/media/video/gspca/gspca.h.orig 2008-11-19 11:10:11.000000000 +0100
+++ linux-2.26.8-rc5/drivers/media/video/gspca/gspca.h 2008-11-19 11:16:58.000000000 +0100
@@ -120,8 +120,8 @@
};
struct gspca_dev {
- struct video_device vdev; /* !! must be the first item */
- struct file_operations fops;
+ struct video_device *vdev;
+ struct module *module; /* subdriver handling the device */
struct usb_device *dev;
struct kref kref;
struct file *capt_file; /* file doing video capture */
--- linux-2.26.8-rc5/drivers/media/video/gspca/gspca.c.orig 2008-11-19 11:10:02.000000000 +0100
+++ linux-2.26.8-rc5/drivers/media/video/gspca/gspca.c 2008-11-19 11:19:57.000000000 +0100
@@ -863,7 +863,7 @@
int ret;
PDEBUG(D_STREAM, "%s open", current->comm);
- gspca_dev = (struct gspca_dev *) video_devdata(file);
+ gspca_dev = video_drvdata(file);
if (mutex_lock_interruptible(&gspca_dev->queue_lock))
return -ERESTARTSYS;
if (!gspca_dev->present) {
@@ -875,6 +875,13 @@
ret = -EBUSY;
goto out;
}
+
+ /* protect the subdriver against rmmod */
+ if (!try_module_get(gspca_dev->module)) {
+ ret = -ENODEV;
+ goto out;
+ }
+
gspca_dev->users++;
/* one more user */
@@ -884,10 +891,10 @@
#ifdef GSPCA_DEBUG
/* activate the v4l2 debug */
if (gspca_debug & D_V4L2)
- gspca_dev->vdev.debug |= V4L2_DEBUG_IOCTL
+ gspca_dev->vdev->debug |= V4L2_DEBUG_IOCTL
| V4L2_DEBUG_IOCTL_ARG;
else
- gspca_dev->vdev.debug &= ~(V4L2_DEBUG_IOCTL
+ gspca_dev->vdev->debug &= ~(V4L2_DEBUG_IOCTL
| V4L2_DEBUG_IOCTL_ARG);
#endif
ret = 0;
@@ -921,6 +928,7 @@
gspca_dev->memory = GSPCA_MEMORY_NO;
}
file->private_data = NULL;
+ module_put(gspca_dev->module);
mutex_unlock(&gspca_dev->queue_lock);
PDEBUG(D_STREAM, "close done");
@@ -1748,11 +1756,6 @@
return ret;
}
-static void dev_release(struct video_device *vfd)
-{
- /* nothing */
-}
-
static struct file_operations dev_fops = {
.owner = THIS_MODULE,
.open = dev_open,
@@ -1800,7 +1803,7 @@
.name = "gspca main driver",
.fops = &dev_fops,
.ioctl_ops = &dev_ioctl_ops,
- .release = dev_release, /* mandatory */
+ .release = video_device_release,
.minor = -1,
};
@@ -1869,17 +1872,18 @@
init_waitqueue_head(&gspca_dev->wq);
/* init video stuff */
- memcpy(&gspca_dev->vdev, &gspca_template, sizeof gspca_template);
- gspca_dev->vdev.parent = &dev->dev;
- memcpy(&gspca_dev->fops, &dev_fops, sizeof gspca_dev->fops);
- gspca_dev->vdev.fops = &gspca_dev->fops;
- gspca_dev->fops.owner = module; /* module protection */
+ gspca_dev->vdev = video_device_alloc();
+ memcpy(gspca_dev->vdev, &gspca_template, sizeof gspca_template);
+ gspca_dev->vdev->parent = &dev->dev;
+ gspca_dev->module = module;
gspca_dev->present = 1;
- ret = video_register_device(&gspca_dev->vdev,
+ video_set_drvdata(gspca_dev->vdev, gspca_dev);
+ ret = video_register_device(gspca_dev->vdev,
VFL_TYPE_GRABBER,
video_nr);
if (ret < 0) {
err("video_register_device err %d", ret);
+ video_device_release(gspca_dev->vdev);
goto out;
}
@@ -1887,7 +1891,8 @@
PDEBUG(D_PROBE, "probe ok");
return 0;
out:
- kref_put(&gspca_dev->kref, gspca_delete);
+ kfree(gspca_dev->usb_buf);
+ kfree(gspca_dev);
return ret;
}
EXPORT_SYMBOL(gspca_dev_probe);
@@ -1905,7 +1910,7 @@
usb_set_intfdata(intf, NULL);
/* We don't want people trying to open up the device */
- video_unregister_device(&gspca_dev->vdev);
+ video_unregister_device(gspca_dev->vdev);
gspca_dev->present = 0;
gspca_dev->streaming = 0;
[-- Attachment #3: Type: text/plain, Size: 164 bytes --]
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
2008-11-19 10:32 ` Jean-Francois Moine
@ 2008-11-19 13:52 ` David Ellingsworth
[not found] ` <492439AE.1070903@redhat.com>
0 siblings, 1 reply; 13+ messages in thread
From: David Ellingsworth @ 2008-11-19 13:52 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: v4l-dvb-maintainer, Mariusz Kozlowski, video4linux-list
> Hi Mariusz,
>
> You have the oops thanks to poison and it is not enabled in my kernel.
>
> I found the real bug: the device structure was part of the gspca device
> and it was freed on close after webcam unplug while streaming.
Jean-Francois,
I reviewed your patch and in my opinion it is the wrong thing to do.
With the recent modifications to v4l2 it is very safe and practical to
embed the video_device struct within a driver struct. The containing
structure however should not be freed until the release callback in
the video_device structure is called. This callback is called after
all open handles have been closed, it is no longer called immediately
after video_unregister_device is called.
The v4l2 subsystem was changed since every driver using v4l2 would
have needed to implement a reference count in order to properly insure
any structure containing the video_device struct was not freed at
inappropriate times. Removing this responsibility from every
sub-driver was a very practical thing to do since it helped reduce
redundant code and increase readability.
For an example of how this should be done, please review the
stk-webcam driver in the v4l-dvb hg repository. I updated it not to
long ago to take advantage of the changes made to the v4l2 subsystem.
The net effect of the changes was a reduction of about 80 lines of
code from the stk-webcam driver, while far less than that were needed
in the v4l2 subsystem.
Regards,
David Ellingsworth
>
> I join a patch I merged from the linux-2.6.28-rc5 source (not compiled -
> the original patch is the last one in my mercurial repository).
>
> Thanks again.
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [v4l-dvb-maintainer] [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
[not found] ` <492439AE.1070903@redhat.com>
@ 2008-11-19 20:20 ` Jean-Francois Moine
[not found] ` <200811192256.09361.m.kozlowski@tuxland.pl>
1 sibling, 0 replies; 13+ messages in thread
From: Jean-Francois Moine @ 2008-11-19 20:20 UTC (permalink / raw)
To: Hans de Goede
Cc: video4linux-list, v4l-dvb-maintainer, David Ellingsworth,
Mariusz Kozlowski
On Wed, 2008-11-19 at 17:07 +0100, Hans de Goede wrote:
> Here is a patch fixing this by using the ref counting already built
> into the
> v4l2-core. Jean-Francois, this is to be applied after reverting your
> fix for this.
Done.
Thanks.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [v4l-dvb-maintainer] [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
[not found] ` <200811192256.09361.m.kozlowski@tuxland.pl>
@ 2008-11-20 18:19 ` Jean-Francois Moine
2008-11-20 18:57 ` David Ellingsworth
2008-11-22 12:41 ` Jean-Francois Moine
1 sibling, 1 reply; 13+ messages in thread
From: Jean-Francois Moine @ 2008-11-20 18:19 UTC (permalink / raw)
To: Hans de Goede; +Cc: Mariusz Kozlowski, David Ellingsworth, video4linux-list
On Wed, 2008-11-19 at 22:56 +0100, Mariusz Kozlowski wrote:
> > Here is a patch fixing this by using the ref counting already built
> > into the
> > v4l2-core. Jean-Francois, this is to be applied after reverting your
> > fix for this.
>
> Not sure I understand what should be applied where. I applied your -
> Hans - patch to
> 2.6.28-rc5-00117-g7f0f598. As you see my HEAD in linux-2.6 is at
> 7f0f598a0069d1ab072375965a4b69137233169c and I can reproduce the oops
> easily.
> I turned on all possible debuging in gspca as well. If it should be
> applied to
> some other tree which contains some more fixes for this - my fault.
> Please let me know.
Hi Hans (de Goede) and Hans (Verkuil),
As you saw, the patch does not work.
Looking at the modules, when a webcam is streaming, the module refcount
of the gspca_main is 3: 1 for the subdriver dependancies, and 2 for one
open. Why 2?
I did not look carefully at the I/O system, but it seems there are two
objects / operations associated to the device. When a disconnection
occurs while the device is opened, at close time, there is:
- a first object put of the device which makes it to be released,
- this release should do a first module_put and then
- calls the gspca_release (see the patch) which frees the gspca device
(and also the video device which is embedded),
- then, the close job is not finished: a second module_put is called
with the fops of the device,
- as this one is in a non allocated memory and as the slab debug is
active: oops!
All this is may be found in the function __fput of fs/file_table.c.
I was wondering if the gspca device could not be freed by the release of
the video device, i.e. what happens if there is no 'kfree(gspca_dev)' in
the gspca_release()?
Cheers.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [v4l-dvb-maintainer] [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
2008-11-20 18:19 ` Jean-Francois Moine
@ 2008-11-20 18:57 ` David Ellingsworth
2008-11-20 19:03 ` Jean-Francois Moine
0 siblings, 1 reply; 13+ messages in thread
From: David Ellingsworth @ 2008-11-20 18:57 UTC (permalink / raw)
To: Jean-Francois Moine; +Cc: Hans de Goede, Mariusz Kozlowski, video4linux-list
On Thu, Nov 20, 2008 at 1:19 PM, Jean-Francois Moine <moinejf@free.fr> wrote:
> On Wed, 2008-11-19 at 22:56 +0100, Mariusz Kozlowski wrote:
>> > Here is a patch fixing this by using the ref counting already built
>> > into the
>> > v4l2-core. Jean-Francois, this is to be applied after reverting your
>> > fix for this.
>>
>> Not sure I understand what should be applied where. I applied your -
>> Hans - patch to
>> 2.6.28-rc5-00117-g7f0f598. As you see my HEAD in linux-2.6 is at
>> 7f0f598a0069d1ab072375965a4b69137233169c and I can reproduce the oops
>> easily.
>> I turned on all possible debuging in gspca as well. If it should be
>> applied to
>> some other tree which contains some more fixes for this - my fault.
>> Please let me know.
>
> Hi Hans (de Goede) and Hans (Verkuil),
>
> As you saw, the patch does not work.
>
> Looking at the modules, when a webcam is streaming, the module refcount
> of the gspca_main is 3: 1 for the subdriver dependancies, and 2 for one
> open. Why 2?
>
> I did not look carefully at the I/O system, but it seems there are two
> objects / operations associated to the device. When a disconnection
> occurs while the device is opened, at close time, there is:
> - a first object put of the device which makes it to be released,
> - this release should do a first module_put and then
> - calls the gspca_release (see the patch) which frees the gspca device
> (and also the video device which is embedded),
> - then, the close job is not finished: a second module_put is called
> with the fops of the device,
> - as this one is in a non allocated memory and as the slab debug is
> active: oops!
>
> All this is may be found in the function __fput of fs/file_table.c.
>
> I was wondering if the gspca device could not be freed by the release of
> the video device, i.e. what happens if there is no 'kfree(gspca_dev)' in
> the gspca_release()?
I'm not entirely sure what's going on in the gspca driver. It seems as
though the module count is wrong. Unfortunately, I don't have a camera
which uses this driver so it's a little hard for me to do any
debugging with it at this time. Technically though, freeing the
gspca_dev in the release callback of the video_device struct should be
possible and that is how it was intended to be used. The stk-webcam
driver has no issues using it this way either.
Regards,
David Ellingsworth
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [v4l-dvb-maintainer] [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
2008-11-20 18:57 ` David Ellingsworth
@ 2008-11-20 19:03 ` Jean-Francois Moine
2008-11-21 0:24 ` leandro Costantino
2008-11-21 14:54 ` David Ellingsworth
0 siblings, 2 replies; 13+ messages in thread
From: Jean-Francois Moine @ 2008-11-20 19:03 UTC (permalink / raw)
To: David Ellingsworth; +Cc: Hans de Goede, Mariusz Kozlowski, video4linux-list
On Thu, 2008-11-20 at 13:57 -0500, David Ellingsworth wrote:
> I'm not entirely sure what's going on in the gspca driver. It seems as
> though the module count is wrong. Unfortunately, I don't have a camera
No, the module count is correct, the problem is that it is incremented /
decremented by 2 at each open / close. Don't you have the same behaviour
with stk-webcam?
> which uses this driver so it's a little hard for me to do any
> debugging with it at this time. Technically though, freeing the
> gspca_dev in the release callback of the video_device struct should be
> possible and that is how it was intended to be used. The stk-webcam
> driver has no issues using it this way either.
I looked at your code, and the only difference I see is that I
increment / decrement explicitly the subdriver module count (OK, step 1
- this module is not the main driver which has the file operations and
the problem!).
Did you activate the slab debug and check the disconnect while
streaming?
Regards.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [v4l-dvb-maintainer] [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
2008-11-20 19:03 ` Jean-Francois Moine
@ 2008-11-21 0:24 ` leandro Costantino
2008-11-21 14:54 ` David Ellingsworth
1 sibling, 0 replies; 13+ messages in thread
From: leandro Costantino @ 2008-11-21 0:24 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: Hans de Goede, Mariusz Kozlowski, David Ellingsworth,
video4linux-list
On Thu, Nov 20, 2008 at 4:03 PM, Jean-Francois Moine <moinejf@free.fr>wrote:
> On Thu, 2008-11-20 at 13:57 -0500, David Ellingsworth wrote:
> > I'm not entirely sure what's going on in the gspca driver. It seems as
> > though the module count is wrong. Unfortunately, I don't have a camera
>
> No, the module count is correct, the problem is that it is incremented /
> decremented by 2 at each open / close. Don't you have the same behaviour
> with stk-webcam?
>
> [SNIP]
>
>
Just for the record. I tested my webcam (zc3xx) usb id on logs. Kernel
2.6.27-rc6 original (gspca 2.3.x) and with lastest hg gspca.
And couldn't reproduce the bug. Worked as expected.
Could this be related to another thing?
***2.6.27-rc6 original***
gspca: main v2.3.0
[ 2903.937468] rxwebcam[23513]: segfault at b6895008 ip 0805e8d0 sp bf970d40
error 6 in rxwebcam[8048000+36000]
[ 3441.672080] wlan0: no IPv6 routers present
[ 3462.933359] ath5k phy0: bf=ed835a40 bf_skb=00000000
[ 3462.933379] wlan0: Selected IBSS BSSID 1e:dc:34:3a:83:17 based on
configured SSID
[ 3462.964934] ath5k phy0: bf=ed835a40 bf_skb=00000000
[ 3462.990417] ath5k phy0: bf=ed835a40 bf_skb=00000000
[ 4314.256080] usb 6-1: USB disconnect, address 2
[ 4314.257781] gspca: urb status: -108
[ 4314.257787] gspca: urb status: -108
[ 4314.260939] gspca: disconnect complete
[ 4345.288071] usb 6-2: new full speed USB device using uhci_hcd and address
3
[ 4345.480872] usb 6-2: configuration #1 chosen from 1 choice
[ 4345.482644] gspca: probing 0ac8:301b
[ 4347.103398] zc3xx: probe 2wr ov vga 0x0000
[ 4347.222391] zc3xx: probe 3wr vga 1 0x8000
[ 4347.227395] zc3xx: probe sensor -> 14
[ 4347.227398] zc3xx: Find Sensor CS2102K?. Chip revision 8000
[ 4347.233597] gspca: probe ok
[ 4378.492090] usb 6-2: USB disconnect, address 3
[ 4378.493391] gspca: urb status: -108
[ 4378.493398] gspca: urb status: -108
[ 4378.495599] gspca: disconnect complete
***2.6.27-rc6 + gspca-902cc23a6723 (lastest) ****
[ 4656.965493] usbcore: deregistering interface driver zc3xx
[ 4656.965615] zc3xx: deregistered
[ 4659.333068] usbcore: deregistering interface driver ALi m5602
[ 4659.333394] gspca: disconnect complete
[ 4659.333538] ALi m5602: deregistered
[ 4659.338052] gspca: main deregistered
[ 4675.576083] usb 6-2: new full speed USB device using uhci_hcd and address
4
[ 4675.768602] usb 6-2: configuration #1 chosen from 1 choice
[ 4675.783522] Linux video capture interface: v2.00
[ 4675.785721] gspca: main v2.4.0 registered
[ 4675.786911] gspca: probing 0ac8:301b
[ 4677.419313] zc3xx: probe 2wr ov vga 0x0000
[ 4677.542335] zc3xx: probe 3wr vga 1 0x8000
[ 4677.547304] zc3xx: probe sensor -> 14
[ 4677.547310] zc3xx: Find Sensor CS2102K?. Chip revision 8000
[ 4677.552456] gspca: probe ok
[ 4677.552478] usbcore: registered new interface driver zc3xx
[ 4677.552482] zc3xx: registered
[ 4704.608077] usb 6-2: USB disconnect, address 4
[ 4704.609288] gspca: urb status: -108
[ 4704.609296] gspca: urb status: -108
[ 4704.610951] gspca: disconnect complete
I will take further look later.
Best Regards.
Costantino Leandro
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [v4l-dvb-maintainer] [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
2008-11-20 19:03 ` Jean-Francois Moine
2008-11-21 0:24 ` leandro Costantino
@ 2008-11-21 14:54 ` David Ellingsworth
2008-11-21 15:03 ` David Ellingsworth
1 sibling, 1 reply; 13+ messages in thread
From: David Ellingsworth @ 2008-11-21 14:54 UTC (permalink / raw)
To: Jean-Francois Moine; +Cc: Hans de Goede, Mariusz Kozlowski, video4linux-list
On Thu, Nov 20, 2008 at 2:03 PM, Jean-Francois Moine <moinejf@free.fr> wrote:
> On Thu, 2008-11-20 at 13:57 -0500, David Ellingsworth wrote:
>> I'm not entirely sure what's going on in the gspca driver. It seems as
>> though the module count is wrong. Unfortunately, I don't have a camera
>
> No, the module count is correct, the problem is that it is incremented /
> decremented by 2 at each open / close. Don't you have the same behaviour
> with stk-webcam?
>
>> which uses this driver so it's a little hard for me to do any
>> debugging with it at this time. Technically though, freeing the
>> gspca_dev in the release callback of the video_device struct should be
>> possible and that is how it was intended to be used. The stk-webcam
>> driver has no issues using it this way either.
>
> I looked at your code, and the only difference I see is that I
> increment / decrement explicitly the subdriver module count (OK, step 1
> - this module is not the main driver which has the file operations and
> the problem!).
The v4l2-core uses a cdev struct, which is embedded in the
video_device struct. The cdev struct has a reference count that is
incremented during video_register_device and all calls to open. This
reference count is then decremented during video_unregister_device and
all calls to close. Once the reference count reaches 0 the
video_device release callback is called to free the structure as the
device is no longer in use. This is the exact same behavior given by
the kref that gspca implemented and the reason that it could be
removed.
>From looking at your repository, it appears you didn't entirely remove
your previous patch. This may in fact be the cause of the problem
since the cdev struct embedded in the video_device struct uses the
video_device's fops->owner. Before your patch this value pointed to
the gspca sub-module, it now refers to the gspca module. I don't
believe this is the right behavior since the gspca is more or less a
supporting driver that provides a set of functions for other drivers.
The sub-module is the true owner of the file_operations since it owns
the device being operated on. This may be the cause of the issue.
>
> Did you activate the slab debug and check the disconnect while
> streaming?
I didn't debug the changes to stk-webcam that I made, the driver's
maintainer did. You'll have to defer this question to him to receive
an answer. However, I don't believe stk-webcam has any issues at this
time for the reasons stated above as to how the v4l2-core works.
The ibmcam driver I had been working on uses the same implementation
seen in stk-webcam and I haven't experienced any issues with slab
debug on and disconnecting the device while it was streaming.
Regards,
David Ellingsworth
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [v4l-dvb-maintainer] [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
2008-11-21 14:54 ` David Ellingsworth
@ 2008-11-21 15:03 ` David Ellingsworth
0 siblings, 0 replies; 13+ messages in thread
From: David Ellingsworth @ 2008-11-21 15:03 UTC (permalink / raw)
To: Jean-Francois Moine; +Cc: Hans de Goede, Mariusz Kozlowski, video4linux-list
[snip]
>> No, the module count is correct, the problem is that it is incremented /
>> decremented by 2 at each open / close. Don't you have the same behaviour
>> with stk-webcam?
Sorry I missed this one. No we do not have this issue with stk-webcam.
The usage count is only incremented once to my knowledge.
Regards,
David Ellingsworth
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [v4l-dvb-maintainer] [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
[not found] ` <200811192256.09361.m.kozlowski@tuxland.pl>
2008-11-20 18:19 ` Jean-Francois Moine
@ 2008-11-22 12:41 ` Jean-Francois Moine
[not found] ` <200811221421.50818.m.kozlowski@tuxland.pl>
1 sibling, 1 reply; 13+ messages in thread
From: Jean-Francois Moine @ 2008-11-22 12:41 UTC (permalink / raw)
To: Mariusz Kozlowski
Cc: Hans de Goede, v4l-dvb-maintainer, David Ellingsworth,
video4linux-list
[-- Attachment #1: Type: text/plain, Size: 782 bytes --]
On Wed, 2008-11-19 at 22:56 +0100, Mariusz Kozlowski wrote:
> Hi,
Hi Mariusz,
> Not sure I understand what should be applied where. I applied your -
> Hans - patch to
> 2.6.28-rc5-00117-g7f0f598. As you see my HEAD in linux-2.6 is at
> 7f0f598a0069d1ab072375965a4b69137233169c and I can reproduce the oops
> easily.
> I turned on all possible debuging in gspca as well. If it should be
> applied to
> some other tree which contains some more fixes for this - my fault.
> Please let me know.
I think Hans's patch was good.
Well, Leandro Costantino found an other bug. Here is a new patch to be
applied to 2.26.8-rc5 (not compiled). May you check it?
Cheers.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
[-- Attachment #2: gspca.patch --]
[-- Type: text/x-patch, Size: 3795 bytes --]
--- ../linux-2.26.8-rc5/drivers/media/video/gspca/gspca.h.orig 2008-11-19 11:10:11.000000000 +0100
+++ ../linux-2.26.8-rc5/drivers/media/video/gspca/gspca.h 2008-11-22 13:31:41.000000000 +0100
@@ -121,9 +121,8 @@
struct gspca_dev {
struct video_device vdev; /* !! must be the first item */
- struct file_operations fops;
+ struct module *module; /* subdriver handling the device */
struct usb_device *dev;
- struct kref kref;
struct file *capt_file; /* file doing video capture */
struct cam cam; /* device information */
--- ../linux-2.26.8-rc5/drivers/media/video/gspca/gspca.c.orig 2008-11-19 11:10:02.000000000 +0100
+++ ../linux-2.26.8-rc5/drivers/media/video/gspca/gspca.c 2008-11-22 13:36:44.000000000 +0100
@@ -30,7 +30,6 @@
#include <linux/string.h>
#include <linux/pagemap.h>
#include <linux/io.h>
-#include <linux/kref.h>
#include <asm/page.h>
#include <linux/uaccess.h>
#include <linux/jiffies.h>
@@ -847,11 +846,11 @@
return ret;
}
-static void gspca_delete(struct kref *kref)
+static void gspca_release(struct video_device *vfd)
{
- struct gspca_dev *gspca_dev = container_of(kref, struct gspca_dev, kref);
+ struct gspca_dev *gspca_dev = container_of(vfd, struct gspca_dev, vdev);
- PDEBUG(D_STREAM, "device deleted");
+ PDEBUG(D_STREAM, "device released");
kfree(gspca_dev->usb_buf);
kfree(gspca_dev);
@@ -875,10 +874,14 @@
ret = -EBUSY;
goto out;
}
- gspca_dev->users++;
- /* one more user */
- kref_get(&gspca_dev->kref);
+ /* protect the subdriver against rmmod */
+ if (!try_module_get(gspca_dev->module)) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ gspca_dev->users++;
file->private_data = gspca_dev;
#ifdef GSPCA_DEBUG
@@ -921,12 +924,11 @@
gspca_dev->memory = GSPCA_MEMORY_NO;
}
file->private_data = NULL;
+ module_put(gspca_dev->module);
mutex_unlock(&gspca_dev->queue_lock);
PDEBUG(D_STREAM, "close done");
- kref_put(&gspca_dev->kref, gspca_delete);
-
return 0;
}
@@ -1748,11 +1750,6 @@
return ret;
}
-static void dev_release(struct video_device *vfd)
-{
- /* nothing */
-}
-
static struct file_operations dev_fops = {
.owner = THIS_MODULE,
.open = dev_open,
@@ -1800,7 +1797,7 @@
.name = "gspca main driver",
.fops = &dev_fops,
.ioctl_ops = &dev_ioctl_ops,
- .release = dev_release, /* mandatory */
+ .release = gspca_release,
.minor = -1,
};
@@ -1838,7 +1835,6 @@
err("couldn't kzalloc gspca struct");
return -ENOMEM;
}
- kref_init(&gspca_dev->kref);
gspca_dev->usb_buf = kmalloc(USB_BUF_SZ, GFP_KERNEL);
if (!gspca_dev->usb_buf) {
err("out of memory");
@@ -1871,9 +1867,7 @@
/* init video stuff */
memcpy(&gspca_dev->vdev, &gspca_template, sizeof gspca_template);
gspca_dev->vdev.parent = &dev->dev;
- memcpy(&gspca_dev->fops, &dev_fops, sizeof gspca_dev->fops);
- gspca_dev->vdev.fops = &gspca_dev->fops;
- gspca_dev->fops.owner = module; /* module protection */
+ gspca_dev->module = module;
gspca_dev->present = 1;
ret = video_register_device(&gspca_dev->vdev,
VFL_TYPE_GRABBER,
@@ -1887,7 +1881,8 @@
PDEBUG(D_PROBE, "probe ok");
return 0;
out:
- kref_put(&gspca_dev->kref, gspca_delete);
+ kfree(gspca_dev->usb_buf);
+ kfree(gspca_dev);
return ret;
}
EXPORT_SYMBOL(gspca_dev_probe);
@@ -1902,15 +1897,14 @@
{
struct gspca_dev *gspca_dev = usb_get_intfdata(intf);
- usb_set_intfdata(intf, NULL);
-
-/* We don't want people trying to open up the device */
- video_unregister_device(&gspca_dev->vdev);
-
gspca_dev->present = 0;
gspca_dev->streaming = 0;
- kref_put(&gspca_dev->kref, gspca_delete);
+ usb_set_intfdata(intf, NULL);
+
+ /* release the device */
+ /* (this will call gspca_release() immediatly or on last close) */
+ video_unregister_device(&gspca_dev->vdev);
PDEBUG(D_PROBE, "disconnect complete");
}
[-- Attachment #3: Type: text/plain, Size: 164 bytes --]
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [v4l-dvb-maintainer] [BUG] zc3xx oopses on unplug: unable to handle kernel paging request
[not found] ` <200811221421.50818.m.kozlowski@tuxland.pl>
@ 2008-11-23 17:43 ` Jean-Francois Moine
0 siblings, 0 replies; 13+ messages in thread
From: Jean-Francois Moine @ 2008-11-23 17:43 UTC (permalink / raw)
To: Mariusz Kozlowski
Cc: Hans de Goede, v4l-dvb-maintainer, David Ellingsworth,
video4linux-list
On Sat, 2008-11-22 at 14:21 +0100, Mariusz Kozlowski wrote:
> Hello,
Hello Mariusz,
[snip]
> and no oops is observed. Thanks for fixing it. Feel free to add
>
> Reported-by: Mariusz Kozlowski <m.kozlowski@tuxland.pl>
> Tested-by: Mariusz Kozlowski <m.kozlowski@tuxland.pl>
Sorry for I forgot to do it :(
Many thanks for your report and tests.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2008-11-23 17:43 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <200811151218.45664.m.kozlowski@tuxland.pl>
2008-11-15 18:48 ` [BUG] zc3xx oopses on unplug: unable to handle kernel paging request Jean-Francois Moine
[not found] ` <200811162224.47885.m.kozlowski@tuxland.pl>
2008-11-18 18:57 ` Jean-Francois Moine
[not found] ` <200811182219.38925.m.kozlowski@tuxland.pl>
2008-11-19 10:32 ` Jean-Francois Moine
2008-11-19 13:52 ` David Ellingsworth
[not found] ` <492439AE.1070903@redhat.com>
2008-11-19 20:20 ` [v4l-dvb-maintainer] " Jean-Francois Moine
[not found] ` <200811192256.09361.m.kozlowski@tuxland.pl>
2008-11-20 18:19 ` Jean-Francois Moine
2008-11-20 18:57 ` David Ellingsworth
2008-11-20 19:03 ` Jean-Francois Moine
2008-11-21 0:24 ` leandro Costantino
2008-11-21 14:54 ` David Ellingsworth
2008-11-21 15:03 ` David Ellingsworth
2008-11-22 12:41 ` Jean-Francois Moine
[not found] ` <200811221421.50818.m.kozlowski@tuxland.pl>
2008-11-23 17:43 ` Jean-Francois Moine
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox