[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>

diff --git a/sound/usb/caiaq/caiaq-audio.c b/sound/usb/caiaq/caiaq-audio.c
index 9cc4cd8283f91561d744641b0afbead6e8b0495a..1aa927942cc6f559988e064ba9fe08676a02531c 100644 (file)
--- a/sound/usb/caiaq/caiaq-audio.c
+++ b/sound/usb/caiaq/caiaq-audio.c
@@ -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
  *   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,
                     struct snd_pcm_substream *sub)
 {
+       unsigned long flags;
+       spin_lock_irqsave(&dev->spinlock, flags);
+
        if (sub->stream == SNDRV_PCM_STREAM_PLAYBACK)
                dev->sub_playback[sub->number] = NULL;
        else
                dev->sub_capture[sub->number] = NULL;
+
+       spin_unlock_irqrestore(&dev->spinlock, flags);
 }
 
 static int
@@ -97,13 +102,13 @@ static int stream_start(struct snd_usb_caiaqdev *dev)
 {
        int i, ret;
 
-       debug("stream_start(%p)\n", dev);
-       spin_lock_irq(&dev->spinlock);
-       if (dev->streaming) {
-               spin_unlock_irq(&dev->spinlock);
+       debug("%s(%p)\n", __func__, dev);
+
+       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->output_panic = 0;
        dev->first_packet = 1;
@@ -112,37 +117,35 @@ static int stream_start(struct snd_usb_caiaqdev *dev)
        for (i = 0; i < N_URBS; i++) {
                ret = usb_submit_urb(dev->data_urbs_in[i], GFP_ATOMIC);
                if (ret) {
-                       log("unable to trigger initial read #%d! (ret = %d)\n",
-                               i, ret);
+                       log("unable to trigger read #%d! (ret %d)\n", i, ret);
                        dev->streaming = 0;
-                       spin_unlock_irq(&dev->spinlock);
                        return -EPIPE;
                }
        }
        
-       spin_unlock_irq(&dev->spinlock);
        return 0;
 }
 
 static void stream_stop(struct snd_usb_caiaqdev *dev)
 {
        int i;
-       
-       debug("stream_stop(%p)\n", dev);
+
+       debug("%s(%p)\n", __func__, dev);
        if (!dev->streaming)
                return;
        
        dev->streaming = 0;
+
        for (i = 0; i < N_URBS; i++) {
-               usb_unlink_urb(dev->data_urbs_in[i]);
-               usb_unlink_urb(dev->data_urbs_out[i]);
+               usb_kill_urb(dev->data_urbs_in[i]);
+               usb_kill_urb(dev->data_urbs_out[i]);
        }
 }
 
 static int snd_usb_caiaq_substream_open(struct snd_pcm_substream *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;
        snd_pcm_limit_hw_rates(substream->runtime);
        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);
 
-       debug("snd_usb_caiaq_substream_close(%p)\n", substream);
+       debug("%s(%p)\n", __func__, substream);
        if (all_substreams_zero(dev->sub_playback) &&
            all_substreams_zero(dev->sub_capture)) {
                /* 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);
                dev->pcm_info.rates = dev->samplerates;
        }
-       
+
        return 0;
 }
 
 static int snd_usb_caiaq_pcm_hw_params(struct snd_pcm_substream *sub,
                                        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));
 }
 
 static int snd_usb_caiaq_pcm_hw_free(struct snd_pcm_substream *sub)
 {
        struct snd_usb_caiaqdev *dev = snd_pcm_substream_chip(sub);
-       debug("snd_usb_caiaq_pcm_hw_free(%p)\n", sub);
-       spin_lock_irq(&dev->spinlock);
+       debug("%s(%p)\n", __func__, sub);
        deactivate_substream(dev, sub);
-       spin_unlock_irq(&dev->spinlock);
        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_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)
                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) {
        case SNDRV_PCM_TRIGGER_START:
        case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-               spin_lock(&dev->spinlock);
                activate_substream(dev, sub);
-               spin_unlock(&dev->spinlock);
                break;
        case SNDRV_PCM_TRIGGER_STOP:
        case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-               spin_lock(&dev->spinlock);
                deactivate_substream(dev, sub);
-               spin_unlock(&dev->spinlock);
                break;
        default:
                return -EINVAL;
@@ -328,8 +325,6 @@ static void read_in_urb_mode0(struct snd_usb_caiaqdev *dev,
        if (all_substreams_zero(dev->sub_capture))
                return;
 
-       spin_lock(&dev->spinlock);
-       
        for (i = 0; i < iso->actual_length;) {
                for (stream = 0; stream < dev->n_streams; stream++, i++) {
                        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,
@@ -358,8 +351,6 @@ static void read_in_urb_mode2(struct snd_usb_caiaqdev *dev,
        struct snd_pcm_substream *sub;
        int stream, i;
 
-       spin_lock(&dev->spinlock);
-       
        for (i = 0; i < iso->actual_length;) {
                if (i % (dev->n_streams * BYTES_PER_SAMPLE_USB) == 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,
@@ -418,8 +407,6 @@ static void read_in_urb(struct snd_usb_caiaqdev *dev,
                                dev->input_panic ? "(input)" : "",
                                dev->output_panic ? "(output)" : "");
        }
-
-       check_for_elapsed_periods(dev, dev->sub_capture);
 }
 
 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;
        struct snd_pcm_substream *sub;
        int stream, i;
-
-       spin_lock(&dev->spinlock);
        
        for (i = 0; i < iso->length;) {
                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++)
                        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)
@@ -472,6 +454,7 @@ static void read_completed(struct urb *urb)
                return;
 
        dev = info->dev;
+
        if (!dev->streaming)
                return;
 
@@ -489,8 +472,12 @@ static void read_completed(struct urb *urb)
                out->iso_frame_desc[outframe].offset = BYTES_PER_FRAME * frame;
                
                if (len > 0) {
+                       spin_lock(&dev->spinlock);
                        fill_out_urb(dev, out, &out->iso_frame_desc[outframe]);
                        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;
                }
 
@@ -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)
 {
-       debug("snd_usb_caiaq_audio_free (%p)\n", dev);
+       debug("%s(%p)\n", __func__, dev);
        stream_stop(dev);
        free_urbs(dev->data_urbs_in);
        free_urbs(dev->data_urbs_out);

diff --git a/sound/usb/caiaq/caiaq-device.c b/sound/usb/caiaq/caiaq-device.c
index 7c44a2c7f96366d4e2eedbb408f6b6d5b8f360cb..73c08b40cc5fe70509156421536703cd6840d3d0 100644 (file)
--- a/sound/usb/caiaq/caiaq-device.c
+++ b/sound/usb/caiaq/caiaq-device.c
@@ -42,7 +42,7 @@
 #endif
 
 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_SUPPORTED_DEVICE("{{Native Instruments, RigKontrol2},"
                         "{Native Instruments, RigKontrol3},"
@@ -456,7 +456,7 @@ static void snd_disconnect(struct usb_interface *intf)
        struct snd_usb_caiaqdev *dev;
        struct snd_card *card = dev_get_drvdata(&intf->dev);
 
-       debug("snd_disconnect(%p)\n", intf);
+       debug("%s(%p)\n", __func__, intf);
 
        if (!card)
                return;