Re: Ooops when working with USB MIDI (2.6.33.1)

Takashi Iwai
2010-04-08T12:22:39Z

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);
}