[ALSA] snd_usb_caiaq: fix potential lockups locking

This patch fixes potential lockups in snd_usb_caiaq by refining the
locking mechanims and by using usb_kill_urb() in favor to
usb_unlink_urb().

Signed-off-by: Daniel Mack <daniel@caiaq.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This commit is contained in:
Daniel Mack 2008-04-14 15:39:14 +02:00 коммит произвёл Takashi Iwai
Родитель f57ab97e76
Коммит 8d048841e8
2 изменённых файлов: 32 добавлений и 45 удалений

Просмотреть файл

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2006,2007 Daniel Mack, Karsten Wiese * Copyright (c) 2006-2008 Daniel Mack, Karsten Wiese
* *
* This program is free software; you can redistribute it and/or modify * This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by * it under the terms of the GNU General Public License as published by
@ -77,10 +77,15 @@ static void
deactivate_substream(struct snd_usb_caiaqdev *dev, deactivate_substream(struct snd_usb_caiaqdev *dev,
struct snd_pcm_substream *sub) struct snd_pcm_substream *sub)
{ {
unsigned long flags;
spin_lock_irqsave(&dev->spinlock, flags);
if (sub->stream == SNDRV_PCM_STREAM_PLAYBACK) if (sub->stream == SNDRV_PCM_STREAM_PLAYBACK)
dev->sub_playback[sub->number] = NULL; dev->sub_playback[sub->number] = NULL;
else else
dev->sub_capture[sub->number] = NULL; dev->sub_capture[sub->number] = NULL;
spin_unlock_irqrestore(&dev->spinlock, flags);
} }
static int static int
@ -97,13 +102,13 @@ static int stream_start(struct snd_usb_caiaqdev *dev)
{ {
int i, ret; int i, ret;
debug("stream_start(%p)\n", dev); debug("%s(%p)\n", __func__, dev);
spin_lock_irq(&dev->spinlock);
if (dev->streaming) {
spin_unlock_irq(&dev->spinlock);
return -EINVAL;
}
if (dev->streaming)
return -EINVAL;
memset(dev->sub_playback, 0, sizeof(dev->sub_playback));
memset(dev->sub_capture, 0, sizeof(dev->sub_capture));
dev->input_panic = 0; dev->input_panic = 0;
dev->output_panic = 0; dev->output_panic = 0;
dev->first_packet = 1; dev->first_packet = 1;
@ -112,37 +117,35 @@ static int stream_start(struct snd_usb_caiaqdev *dev)
for (i = 0; i < N_URBS; i++) { for (i = 0; i < N_URBS; i++) {
ret = usb_submit_urb(dev->data_urbs_in[i], GFP_ATOMIC); ret = usb_submit_urb(dev->data_urbs_in[i], GFP_ATOMIC);
if (ret) { if (ret) {
log("unable to trigger initial read #%d! (ret = %d)\n", log("unable to trigger read #%d! (ret %d)\n", i, ret);
i, ret);
dev->streaming = 0; dev->streaming = 0;
spin_unlock_irq(&dev->spinlock);
return -EPIPE; return -EPIPE;
} }
} }
spin_unlock_irq(&dev->spinlock);
return 0; return 0;
} }
static void stream_stop(struct snd_usb_caiaqdev *dev) static void stream_stop(struct snd_usb_caiaqdev *dev)
{ {
int i; int i;
debug("stream_stop(%p)\n", dev); debug("%s(%p)\n", __func__, dev);
if (!dev->streaming) if (!dev->streaming)
return; return;
dev->streaming = 0; dev->streaming = 0;
for (i = 0; i < N_URBS; i++) { for (i = 0; i < N_URBS; i++) {
usb_unlink_urb(dev->data_urbs_in[i]); usb_kill_urb(dev->data_urbs_in[i]);
usb_unlink_urb(dev->data_urbs_out[i]); usb_kill_urb(dev->data_urbs_out[i]);
} }
} }
static int snd_usb_caiaq_substream_open(struct snd_pcm_substream *substream) static int snd_usb_caiaq_substream_open(struct snd_pcm_substream *substream)
{ {
struct snd_usb_caiaqdev *dev = snd_pcm_substream_chip(substream); struct snd_usb_caiaqdev *dev = snd_pcm_substream_chip(substream);
debug("snd_usb_caiaq_substream_open(%p)\n", substream); debug("%s(%p)\n", __func__, substream);
substream->runtime->hw = dev->pcm_info; substream->runtime->hw = dev->pcm_info;
snd_pcm_limit_hw_rates(substream->runtime); snd_pcm_limit_hw_rates(substream->runtime);
return 0; return 0;
@ -152,7 +155,7 @@ static int snd_usb_caiaq_substream_close(struct snd_pcm_substream *substream)
{ {
struct snd_usb_caiaqdev *dev = snd_pcm_substream_chip(substream); struct snd_usb_caiaqdev *dev = snd_pcm_substream_chip(substream);
debug("snd_usb_caiaq_substream_close(%p)\n", substream); debug("%s(%p)\n", __func__, substream);
if (all_substreams_zero(dev->sub_playback) && if (all_substreams_zero(dev->sub_playback) &&
all_substreams_zero(dev->sub_capture)) { all_substreams_zero(dev->sub_capture)) {
/* when the last client has stopped streaming, /* when the last client has stopped streaming,
@ -160,24 +163,22 @@ static int snd_usb_caiaq_substream_close(struct snd_pcm_substream *substream)
stream_stop(dev); stream_stop(dev);
dev->pcm_info.rates = dev->samplerates; dev->pcm_info.rates = dev->samplerates;
} }
return 0; return 0;
} }
static int snd_usb_caiaq_pcm_hw_params(struct snd_pcm_substream *sub, static int snd_usb_caiaq_pcm_hw_params(struct snd_pcm_substream *sub,
struct snd_pcm_hw_params *hw_params) struct snd_pcm_hw_params *hw_params)
{ {
debug("snd_usb_caiaq_pcm_hw_params(%p)\n", sub); debug("%s(%p)\n", __func__, sub);
return snd_pcm_lib_malloc_pages(sub, params_buffer_bytes(hw_params)); return snd_pcm_lib_malloc_pages(sub, params_buffer_bytes(hw_params));
} }
static int snd_usb_caiaq_pcm_hw_free(struct snd_pcm_substream *sub) static int snd_usb_caiaq_pcm_hw_free(struct snd_pcm_substream *sub)
{ {
struct snd_usb_caiaqdev *dev = snd_pcm_substream_chip(sub); struct snd_usb_caiaqdev *dev = snd_pcm_substream_chip(sub);
debug("snd_usb_caiaq_pcm_hw_free(%p)\n", sub); debug("%s(%p)\n", __func__, sub);
spin_lock_irq(&dev->spinlock);
deactivate_substream(dev, sub); deactivate_substream(dev, sub);
spin_unlock_irq(&dev->spinlock);
return snd_pcm_lib_free_pages(sub); return snd_pcm_lib_free_pages(sub);
} }
@ -196,7 +197,7 @@ static int snd_usb_caiaq_pcm_prepare(struct snd_pcm_substream *substream)
struct snd_usb_caiaqdev *dev = snd_pcm_substream_chip(substream); struct snd_usb_caiaqdev *dev = snd_pcm_substream_chip(substream);
struct snd_pcm_runtime *runtime = substream->runtime; struct snd_pcm_runtime *runtime = substream->runtime;
debug("snd_usb_caiaq_pcm_prepare(%p)\n", substream); debug("%s(%p)\n", __func__, substream);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
dev->audio_out_buf_pos[index] = BYTES_PER_SAMPLE + 1; dev->audio_out_buf_pos[index] = BYTES_PER_SAMPLE + 1;
@ -247,15 +248,11 @@ static int snd_usb_caiaq_pcm_trigger(struct snd_pcm_substream *sub, int cmd)
switch (cmd) { switch (cmd) {
case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
spin_lock(&dev->spinlock);
activate_substream(dev, sub); activate_substream(dev, sub);
spin_unlock(&dev->spinlock);
break; break;
case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH: case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
spin_lock(&dev->spinlock);
deactivate_substream(dev, sub); deactivate_substream(dev, sub);
spin_unlock(&dev->spinlock);
break; break;
default: default:
return -EINVAL; return -EINVAL;
@ -328,8 +325,6 @@ static void read_in_urb_mode0(struct snd_usb_caiaqdev *dev,
if (all_substreams_zero(dev->sub_capture)) if (all_substreams_zero(dev->sub_capture))
return; return;
spin_lock(&dev->spinlock);
for (i = 0; i < iso->actual_length;) { for (i = 0; i < iso->actual_length;) {
for (stream = 0; stream < dev->n_streams; stream++, i++) { for (stream = 0; stream < dev->n_streams; stream++, i++) {
sub = dev->sub_capture[stream]; sub = dev->sub_capture[stream];
@ -345,8 +340,6 @@ static void read_in_urb_mode0(struct snd_usb_caiaqdev *dev,
} }
} }
} }
spin_unlock(&dev->spinlock);
} }
static void read_in_urb_mode2(struct snd_usb_caiaqdev *dev, static void read_in_urb_mode2(struct snd_usb_caiaqdev *dev,
@ -358,8 +351,6 @@ static void read_in_urb_mode2(struct snd_usb_caiaqdev *dev,
struct snd_pcm_substream *sub; struct snd_pcm_substream *sub;
int stream, i; int stream, i;
spin_lock(&dev->spinlock);
for (i = 0; i < iso->actual_length;) { for (i = 0; i < iso->actual_length;) {
if (i % (dev->n_streams * BYTES_PER_SAMPLE_USB) == 0) { if (i % (dev->n_streams * BYTES_PER_SAMPLE_USB) == 0) {
for (stream = 0; for (stream = 0;
@ -393,8 +384,6 @@ static void read_in_urb_mode2(struct snd_usb_caiaqdev *dev,
} }
} }
} }
spin_unlock(&dev->spinlock);
} }
static void read_in_urb(struct snd_usb_caiaqdev *dev, static void read_in_urb(struct snd_usb_caiaqdev *dev,
@ -418,8 +407,6 @@ static void read_in_urb(struct snd_usb_caiaqdev *dev,
dev->input_panic ? "(input)" : "", dev->input_panic ? "(input)" : "",
dev->output_panic ? "(output)" : ""); dev->output_panic ? "(output)" : "");
} }
check_for_elapsed_periods(dev, dev->sub_capture);
} }
static void fill_out_urb(struct snd_usb_caiaqdev *dev, static void fill_out_urb(struct snd_usb_caiaqdev *dev,
@ -429,8 +416,6 @@ static void fill_out_urb(struct snd_usb_caiaqdev *dev,
unsigned char *usb_buf = urb->transfer_buffer + iso->offset; unsigned char *usb_buf = urb->transfer_buffer + iso->offset;
struct snd_pcm_substream *sub; struct snd_pcm_substream *sub;
int stream, i; int stream, i;
spin_lock(&dev->spinlock);
for (i = 0; i < iso->length;) { for (i = 0; i < iso->length;) {
for (stream = 0; stream < dev->n_streams; stream++, i++) { for (stream = 0; stream < dev->n_streams; stream++, i++) {
@ -456,9 +441,6 @@ static void fill_out_urb(struct snd_usb_caiaqdev *dev,
for (stream = 0; stream < dev->n_streams; stream++, i++) for (stream = 0; stream < dev->n_streams; stream++, i++)
usb_buf[i] = MAKE_CHECKBYTE(dev, stream, i); usb_buf[i] = MAKE_CHECKBYTE(dev, stream, i);
} }
spin_unlock(&dev->spinlock);
check_for_elapsed_periods(dev, dev->sub_playback);
} }
static void read_completed(struct urb *urb) static void read_completed(struct urb *urb)
@ -472,6 +454,7 @@ static void read_completed(struct urb *urb)
return; return;
dev = info->dev; dev = info->dev;
if (!dev->streaming) if (!dev->streaming)
return; return;
@ -489,8 +472,12 @@ static void read_completed(struct urb *urb)
out->iso_frame_desc[outframe].offset = BYTES_PER_FRAME * frame; out->iso_frame_desc[outframe].offset = BYTES_PER_FRAME * frame;
if (len > 0) { if (len > 0) {
spin_lock(&dev->spinlock);
fill_out_urb(dev, out, &out->iso_frame_desc[outframe]); fill_out_urb(dev, out, &out->iso_frame_desc[outframe]);
read_in_urb(dev, urb, &urb->iso_frame_desc[frame]); read_in_urb(dev, urb, &urb->iso_frame_desc[frame]);
spin_unlock(&dev->spinlock);
check_for_elapsed_periods(dev, dev->sub_playback);
check_for_elapsed_periods(dev, dev->sub_capture);
send_it = 1; send_it = 1;
} }
@ -696,7 +683,7 @@ int snd_usb_caiaq_audio_init(struct snd_usb_caiaqdev *dev)
void snd_usb_caiaq_audio_free(struct snd_usb_caiaqdev *dev) void snd_usb_caiaq_audio_free(struct snd_usb_caiaqdev *dev)
{ {
debug("snd_usb_caiaq_audio_free (%p)\n", dev); debug("%s(%p)\n", __func__, dev);
stream_stop(dev); stream_stop(dev);
free_urbs(dev->data_urbs_in); free_urbs(dev->data_urbs_in);
free_urbs(dev->data_urbs_out); free_urbs(dev->data_urbs_out);

Просмотреть файл

@ -42,7 +42,7 @@
#endif #endif
MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>"); MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>");
MODULE_DESCRIPTION("caiaq USB audio, version 1.3.2"); MODULE_DESCRIPTION("caiaq USB audio, version 1.3.4");
MODULE_LICENSE("GPL"); MODULE_LICENSE("GPL");
MODULE_SUPPORTED_DEVICE("{{Native Instruments, RigKontrol2}," MODULE_SUPPORTED_DEVICE("{{Native Instruments, RigKontrol2},"
"{Native Instruments, RigKontrol3}," "{Native Instruments, RigKontrol3},"
@ -456,7 +456,7 @@ static void snd_disconnect(struct usb_interface *intf)
struct snd_usb_caiaqdev *dev; struct snd_usb_caiaqdev *dev;
struct snd_card *card = dev_get_drvdata(&intf->dev); struct snd_card *card = dev_get_drvdata(&intf->dev);
debug("snd_disconnect(%p)\n", intf); debug("%s(%p)\n", __func__, intf);
if (!card) if (!card)
return; return;