Re: Ooops when working with USB MIDI (2.6.33.1)

[Takashi Iwai <tiwai@suse.de>]

At Thu, 08 Apr 2010 12:00:11 +0200,
Clemens Ladisch wrote:
> 
> Tvrtko Ursulin wrote:
> > I had TuxGuitar running and an external USB MIDI device, which I then
> > turned off, on, and exited TuxGuitar at which point there was this oops.
> > ...
> > [321182.618342] IP: [<ffffffff81375d65>] _raw_spin_lock_irq+0x15/0x30                                                                               
> > [321182.618449]  [<ffffffffa01e2fca>] ? snd_usbmidi_output_drain+0x7a/0x100 [snd_usb_lib]
> > [321182.618462]  [<ffffffffa01b7699>] ? snd_rawmidi_drain_output+0x119/0x180 [snd_rawmidi]
> 
> I think that bug was introduced by this commit:
> http://git.kernel.org/linus/7a17daae8ed71bf3259d905a4fc48a5b424fa935
> which causes the driver to free many internal data structures when
> they might still be in use by userspace.
> 
> Please try to revert it.
> 
> I think a proper fix would be to free the URBs/buffers but not the
> driver's data structures.
> 
> Takashi, do you remember what the original problem was?

Well, I have only a vague memory -- it's a similar scenario that some app
still accessing after disconnection.  The URB can't be handled after
the disconnection is finished.

I think the patch below might fix in this case.  You can try it
instead of reverting the commit above.


thanks,

Takashi

---
diff --git a/sound/usb/usbmidi.c b/sound/usb/usbmidi.c
index 2c59afd..81c8d85 100644
--- a/sound/usb/usbmidi.c
+++ b/sound/usb/usbmidi.c
@@ -986,6 +986,8 @@ static void snd_usbmidi_output_drain(struct snd_rawmidi_substream *substream)
 	DEFINE_WAIT(wait);
 	long timeout = msecs_to_jiffies(50);
 
+	if (ep->umidi->disconnected)
+		return;
 	/*
 	 * The substream buffer is empty, but some data might still be in the
 	 * currently active URBs, so we have to wait for those to complete.
@@ -1275,6 +1277,11 @@ void snd_usbmidi_disconnect(struct list_head* p)
 			snd_usbmidi_in_endpoint_delete(ep->in);
 			ep->in = NULL;
 		}
+		ep->active_urbs = 0;
+		if (ep->drain_urbs) {
+			ep->drain_urbs = 0;
+			wake_up(&ep->drain_wait);
+		}
 	}
 	del_timer_sync(&umidi->error_timer);
 }