On Tuesday 03 February 2009, Jean-Francois Moine wrote: > Indeed, the problem is there! You must have only one process reading the > webcam! I do not see how this can work with these 2 processes... Although 2 processes are created only one ever gets used. create_singlethread_workqueue would therefore be less wasteful but make no other difference. Google searches for create_freezeable_workqueue bring up suggestions to avoid it so I think I will although I guess we ought to check at some point how well the camera copes with suspend while streaming. At 19:53:31 Alan Stern wrote > If a camera is unplugged while it is in use then > waiting until close() is no good -- it will cause destroy_urbs() to > pass a stale pointer to usb_buffer_free(). That's the reason for the > oops. I'd go further and suggest that gspca_disconnect should, after calling destroy_urbs() do gspca_dev->dev = NULL; Any use of that pointer past that point is a bug (the downside is that doing so would have turned the current bug into a hard to spot memory leak). At 03:37:35 Alan Stern wrote > On Mon, 2 Feb 2009, Adam Baker wrote: >> This shouldn't be a problem for sq905 (which doesn't use these URBs) or >> isochronous cameras (which don't need to resubmit URBs) but the finepix >> driver (the other supported bulk device) will need some careful >> consideration to avoid a race between killing the URB and resubmitting it. > > You shouldn't need to take any special action. usb_kill_urb() solves > these races for you; it guarantees not to return until the URB has been > unlinked and the completion handler has finished, and it guarantees > that attempts by the completion handler to resubmit the URB will fail. The sq905 driver doesn't use the URBs provided by gspca, it uses usb_control_msg and usb_bulk_msg which I presume do the right thing internally. There would be a tiny window in between when it checks the dev->streaming flag and when it sends a new USB msg for the disconnect to occur and invalidate the dev pointer. That could be fixed by holding gspca_dev->usb_lock in gspca_disconnect when it sets gspca_dev->present = 0. That would also address the race between open and disconnect. Unfortunately the finepix driver sometimes uses calls to schedule_delayed_work in the completion handler which then makes the call to usb_submit_urb. Fixing that will require someone with access to a suitable camera to test it otherwise there is a significant risk of adding deadlocks. It already suffers from this bug so we aren't making it worse. Calling destroy_urbs from dev_close rather than gspca_disconnect could also trigger the same bug on isochronous cameras. I haven't looked at them closely but they probably also have the race between open and disconnect which taking the lock in disconnect is likely to fix. I'll do some testing and post a patch if it looks promising. Adam -- 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
