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

Takashi Iwai
2013-01-10T10:21:30Z

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++;