Panic in HVR-950q caused by changeset 11356

Panic in HVR-950q caused by changeset 11356

[Devin Heitmueller]

Hello Janne,

Ok, so now I need to better understand the nature of changeset 11356.
It turns up I spent the entire afternoon debugging a kernel panic on
usb disconnect, which ended up being due to this patch:

au0828: use usb_interface.dev for v4l2_device_register
http://linuxtv.org/hg/v4l-dvb/rev/33810c734a0d

The change to pass the interface->dev to v4l2_device_register()
effectively overwrote the interface data, so while I thought
usb_set_intfdata() was storing the au0828_dev *, in fact it was
holding a v4l2_device *.  When au0828_usb_disconnect() eventually gets
called, the call to usb_get_intfdata() returned the v4l2_device, and
presto - instant panic.

So, either I can roll back this change, or I can make the call to
usb_set_intfdata() *after* the call to v4l2_device_register().
However, I don't know what else that might screw up (I haven't traced
out everything in v4l2-device that might expect a v4l2_device * to be
stored there).

Suggestions?

Perhaps if you could provide some additional background as to what
prompted this change, it will help me better understand the correct
course of action at this point.

Devin

cc: Robert Krakora and Josh Watzman since they both independently
reported what I believe to be the exact same issue (the stack is
slightly different because in their case as it crashed in the
dvb_unregister portion of the usb_disconnect routine).

-- 
Devin J. Heitmueller
http://www.devinheitmueller.com
AIM: devinheitmueller

Re: Panic in HVR-950q caused by changeset 11356

[Robert Krakora]

On Mon, Apr 27, 2009 at 10:03 PM, Devin Heitmueller
<devin.heitmueller@gmail.com> wrote:
> Hello Janne,
>
> Ok, so now I need to better understand the nature of changeset 11356.
> It turns up I spent the entire afternoon debugging a kernel panic on
> usb disconnect, which ended up being due to this patch:
>
> au0828: use usb_interface.dev for v4l2_device_register
> http://linuxtv.org/hg/v4l-dvb/rev/33810c734a0d
>
> The change to pass the interface->dev to v4l2_device_register()
> effectively overwrote the interface data, so while I thought
> usb_set_intfdata() was storing the au0828_dev *, in fact it was
> holding a v4l2_device *.  When au0828_usb_disconnect() eventually gets
> called, the call to usb_get_intfdata() returned the v4l2_device, and
> presto - instant panic.
>
> So, either I can roll back this change, or I can make the call to
> usb_set_intfdata() *after* the call to v4l2_device_register().
> However, I don't know what else that might screw up (I haven't traced
> out everything in v4l2-device that might expect a v4l2_device * to be
> stored there).
>
> Suggestions?
>
> Perhaps if you could provide some additional background as to what
> prompted this change, it will help me better understand the correct
> course of action at this point.
>
> Devin
>
> cc: Robert Krakora and Josh Watzman since they both independently
> reported what I believe to be the exact same issue (the stack is
> slightly different because in their case as it crashed in the
> dvb_unregister portion of the usb_disconnect routine).
>
> --
> Devin J. Heitmueller
> http://www.devinheitmueller.com
> AIM: devinheitmueller
>

Devin:

I vote to roll it back until the ramifications of the changeset are
better understood.  ;-)

Best Regards,

-- 
Rob Krakora
Senior Software Engineer
MessageNet Systems
101 East Carmel Dr. Suite 105
Carmel, IN 46032
(317)566-1677 Ext. 206
(317)663-0808 Fax

Re: Panic in HVR-950q caused by changeset 11356

[Devin Heitmueller]

On Tue, Apr 28, 2009 at 12:42 PM, Robert Krakora <rob.krakora@gmail.com> wrote:
> Devin:
>
> I vote to roll it back until the ramifications of the changeset are
> better understood.  ;-)
>
> Best Regards,

Robert,

I discussed it at length with Janneg on #linuxtv this morning.  A pull
request has been submitted with the fix, marked high priority.

Thanks,

Devin

-- 
Devin J. Heitmueller
http://www.devinheitmueller.com
AIM: devinheitmueller

Re: Panic in HVR-950q caused by changeset 11356

[Josh Watzman]

Devin Heitmueller wrote:
> I discussed it at length with Janneg on #linuxtv this morning.  A pull
> request has been submitted with the fix, marked high priority.

I just pulled and built from the main hg tree, and in a simple test it
did not oops on disconnect! Thanks for everyone's work, and I'll
continue to test over the next week or so and let you know if it happens
again.

(Also, thanks for the CC, and please continue to do so on anything
related to this issue; I unsubscribed from the list a few days ago due
to the relatively high traffic.)

Josh Watzman