Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756120Ab0DIH3i (ORCPT ); Fri, 9 Apr 2010 03:29:38 -0400 Received: from cantor2.suse.de ([195.135.220.15]:49805 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755545Ab0DIH3f (ORCPT ); Fri, 9 Apr 2010 03:29:35 -0400 Date: Fri, 09 Apr 2010 09:29:33 +0200 Message-ID: From: Takashi Iwai To: Tvrtko Ursulin Cc: Clemens Ladisch , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org Subject: Re: Ooops when working with USB MIDI (2.6.33.1) In-Reply-To: <201004090751.35398.tvrtko@ursulin.net> References: <201004051433.47171.tvrtko@ursulin.net> <4BBDA92B.8010505@ladisch.de> <201004090751.35398.tvrtko@ursulin.net> User-Agent: Wanderlust/2.15.6 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.7 Emacs/23.1 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3856 Lines: 122 At Fri, 9 Apr 2010 07:51:35 +0100, Tvrtko Ursulin wrote: > > On Thursday 08 Apr 2010 13:22:36 Takashi Iwai wrote: > > > 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); > > } > > For the second hunk, do you think ep->out->... and so on? That would be more > in-line with code present in 2.6.33. Ah, crap. Sorry, that's just messing up. The revised (compiled but untested) patch is below. thanks, Takashi --- diff --git a/sound/usb/usbmidi.c b/sound/usb/usbmidi.c index 2c59afd..9e28b20 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. @@ -1123,14 +1125,21 @@ static int snd_usbmidi_in_endpoint_create(struct snd_usb_midi* umidi, * Frees an output endpoint. * May be called when ep hasn't been initialized completely. */ -static void snd_usbmidi_out_endpoint_delete(struct snd_usb_midi_out_endpoint* ep) +static void snd_usbmidi_out_endpoint_clear(struct snd_usb_midi_out_endpoint *ep) { unsigned int i; for (i = 0; i < OUTPUT_URBS; ++i) - if (ep->urbs[i].urb) + if (ep->urbs[i].urb) { free_urb_and_buffer(ep->umidi, ep->urbs[i].urb, ep->max_transfer); + ep->urbs[i].urb = NULL; + } +} + +static void snd_usbmidi_out_endpoint_delete(struct snd_usb_midi_out_endpoint *ep) +{ + snd_usbmidi_out_endpoint_clear(ep); kfree(ep); } @@ -1262,15 +1271,18 @@ void snd_usbmidi_disconnect(struct list_head* p) usb_kill_urb(ep->out->urbs[j].urb); if (umidi->usb_protocol_ops->finish_out_endpoint) umidi->usb_protocol_ops->finish_out_endpoint(ep->out); + ep->out->active_urbs = 0; + if (ep->out->drain_urbs) { + ep->out->drain_urbs = 0; + wake_up(&ep->out->drain_wait); + } } if (ep->in) for (j = 0; j < INPUT_URBS; ++j) usb_kill_urb(ep->in->urbs[j]); /* free endpoints here; later call can result in Oops */ - if (ep->out) { - snd_usbmidi_out_endpoint_delete(ep->out); - ep->out = NULL; - } + if (ep->out) + snd_usbmidi_out_endpoint_clear(ep->out); if (ep->in) { snd_usbmidi_in_endpoint_delete(ep->in); ep->in = NULL; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/