Re: Oops in sound/usb/pcm.c:match_endpoint_audioformats() in current -git

[Takashi Iwai <tiwai@suse.de>]

At Thu, 10 Jan 2013 09:23:48 +0100,
Jens Axboe wrote:
> 
> Hi,
> 
> I have a USB DAC that I use for music. Just upgraded my workstation from
> 3.7-rc7 to 3.8-rc3 this morning, and when the DAC is switched on, I get
> an immediate oops. I have attached the picture. It's crashing here:
> 
> (gdb) l *snd_usb_pcm_prepare+0x153
> 0x161c is in snd_usb_pcm_prepare (sound/usb/pcm.c:470).
> 465		snd_pcm_format_t pcm_format)
> 466	{
> 467		int i;
> 468		int score = 0;
> 469	
> 470		if (fp->channels < 1) {
> 471			snd_printdd("%s: (fmt @%p) no channels\n", __func__, fp);
> 472			return 0;
> 473		}
> 474	
> 
> 'fp' is clearly NULL.

Hmm... it's a function called only from a single place in
configure_sync_endpoint() and it's in list_for_each_entry():

	list_for_each_entry(fp, &sync_subs->fmt_list, list) {
		int score = match_endpoint_audioformats(fp, subs->cur_audiofmt,
			subs->cur_rate, subs->pcm_format);

so it must be hardly NULL in normal situations.
The only scenario I can imagine right now is that the whole structure
is still uninitialized while it's called.  If so, it's a race problem,
and shouldn't be new to 3.8.  A patch like below might paper over the
problem although it's far from perfect.


> Let me know if you want a bisect.

Yeah, that'll be really helpful.

Or, at least, could you try to copy sound/usb/* from 3.7 to 3.8 and
see whether it's really a regression there?


thanks,

Takashi

---
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index ad181d5..3bfc564 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -81,8 +81,7 @@ static void snd_usb_audio_pcm_free(struct snd_pcm *pcm)
  */
 
 static void snd_usb_init_substream(struct snd_usb_stream *as,
-				   int stream,
-				   struct audioformat *fp)
+				   int stream)
 {
 	struct snd_usb_substream *subs = &as->substream[stream];
 
@@ -96,6 +95,13 @@ static void snd_usb_init_substream(struct snd_usb_stream *as,
 	subs->speed = snd_usb_get_speed(subs->dev);
 
 	snd_usb_set_pcm_ops(as->pcm, stream);
+}
+
+static void snd_usb_add_substream_format(struct snd_usb_stream *as,
+				   int stream,
+				   struct audioformat *fp)
+{
+	struct snd_usb_substream *subs = &as->substream[stream];
 
 	list_add_tail(&fp->list, &subs->fmt_list);
 	subs->formats |= fp->formats;
@@ -342,7 +348,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 		err = snd_pcm_new_stream(as->pcm, stream, 1);
 		if (err < 0)
 			return err;
-		snd_usb_init_substream(as, stream, fp);
+		snd_usb_add_substream_format(as, stream, fp);
 		return add_chmap(as->pcm, stream, subs);
 	}
 
@@ -370,7 +376,9 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 	else
 		strcpy(pcm->name, "USB Audio");
 
-	snd_usb_init_substream(as, stream, fp);
+	snd_usb_init_substream(as, SNDRV_PCM_STREAM_PLAYBACK);
+	snd_usb_init_substream(as, SNDRV_PCM_STREAM_CAPTURE);
+	snd_usb_add_substream_format(as, stream, fp);
 
 	list_add(&as->list, &chip->pcm_list);
 	chip->pcm_devs++;