On Tue, 25 Aug 2015 19:15:23 +0200, Alexnader Kuleshov wrote:
Hello Takashi, just tested it on my linux box against 4.2.0-rc8+ and there is the same 'possible recursive locking detected', but another:
[ 13.422080] ============================================= [ 13.422081] [ INFO: possible recursive locking detected ] [ 13.422082] 4.2.0-rc8+ #61 Not tainted [ 13.422083] --------------------------------------------- [ 13.422084] systemd-udevd/533 is trying to acquire lock: [ 13.422085] (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0253da3>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio] [ 13.422094] but task is already holding lock: [ 13.422094] (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0257377>] snd_usb_mixer_set_ctl_value+0xd0/0x1ad [snd_usb_audio] [ 13.422100] other info that might help us debug this: [ 13.422101] Possible unsafe locking scenario:
[ 13.422102] CPU0 [ 13.422102] ---- [ 13.422103] lock(&chip->shutdown_rwsem); [ 13.422104] lock(&chip->shutdown_rwsem); [ 13.422105] *** DEADLOCK ***
[ 13.422106] May be due to missing lock nesting notation
[ 13.422107] 4 locks held by systemd-udevd/533: [ 13.422108] #0: (&dev->mutex){......}, at: [<ffffffff813b3414>] __driver_attach+0x30/0x80 [ 13.422112] #1: (&dev->mutex){......}, at: [<ffffffff813b342c>] __driver_attach+0x48/0x80 [ 13.422115] #2: (register_mutex#4){+.+.+.}, at: [<ffffffffa0253670>] usb_audio_probe+0xa3/0x7b9 [snd_usb_audio] [ 13.422120] #3: (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0257377>] snd_usb_mixer_set_ctl_value+0xd0/0x1ad [snd_usb_audio] [ 13.422125] stack backtrace: [ 13.422127] CPU: 7 PID: 533 Comm: systemd-udevd Not tainted 4.2.0-rc8+ #61 [ 13.422128] Hardware name: Gigabyte Technology Co., Ltd. Z97X-UD5H-BK/Z97X-UD5H-BK, BIOS F6 06/17/2014 [ 13.422129] 0000000000000000 0000000071d6ca78 ffff880407bf7538 ffffffff815ba622 [ 13.422131] 0000000000000000 ffffffff8239a680 ffff880407bf75f8 ffffffff810dd54e [ 13.422133] ffff880409db64a8 ffffffff00000000 0000000182000201 ffffffff8239a680 [ 13.422135] Call Trace: [ 13.422137] [<ffffffff815ba622>] dump_stack+0x4c/0x65 [ 13.422140] [<ffffffff810dd54e>] validate_chain.isra.9+0x75d/0xf59 [ 13.422142] [<ffffffff810de69c>] ? mark_held_locks+0x5f/0x76 [ 13.422144] [<ffffffff810e0964>] __lock_acquire+0x753/0xabf [ 13.422146] [<ffffffff810e1131>] lock_acquire+0x7b/0x9c [ 13.422151] [<ffffffffa0253da3>] ? snd_usb_autoresume+0x1d/0x52 [snd_usb_audio] [ 13.422153] [<ffffffff815c4896>] down_read+0x44/0x8d [ 13.422156] [<ffffffffa0253da3>] ? snd_usb_autoresume+0x1d/0x52 [snd_usb_audio] [ 13.422160] [<ffffffffa0253da3>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio] [ 13.422162] [<ffffffff815c48d6>] ? down_read+0x84/0x8d [ 13.422167] [<ffffffffa025738c>] snd_usb_mixer_set_ctl_value+0xe5/0x1ad [snd_usb_audio] [ 13.422171] [<ffffffffa025784a>] get_min_max_with_quirks+0x155/0x5e4 [snd_usb_audio] [ 13.422176] [<ffffffffa02581c1>] build_feature_ctl+0x33a/0x419 [snd_usb_audio] [ 13.422181] [<ffffffffa02586f9>] parse_audio_unit+0x459/0xa03 [snd_usb_audio] [ 13.422186] [<ffffffffa02590f4>] snd_usb_mixer_controls+0xe8/0x169 [snd_usb_audio] [ 13.422190] [<ffffffffa02593b4>] snd_usb_create_mixer+0xb4/0x261 [snd_usb_audio] [ 13.422194] [<ffffffffa02535ba>] ? snd_usb_create_stream+0x151/0x164 [snd_usb_audio] [ 13.422197] [<ffffffffa0253ccd>] usb_audio_probe+0x700/0x7b9 [snd_usb_audio] [ 13.422199] [<ffffffff81413bb7>] usb_probe_interface+0x139/0x1c3 [ 13.422201] [<ffffffff813b329a>] driver_probe_device+0xd0/0x21a [ 13.422203] [<ffffffff813b3441>] __driver_attach+0x5d/0x80 [ 13.422205] [<ffffffff813b33e4>] ? driver_probe_device+0x21a/0x21a [ 13.422207] [<ffffffff813b1850>] bus_for_each_dev+0x77/0xa5 [ 13.422210] [<ffffffff813b2f94>] driver_attach+0x19/0x1b [ 13.422212] [<ffffffff813b2a87>] bus_add_driver+0xe8/0x1da [ 13.422213] [<ffffffff813b3a26>] driver_register+0x86/0xc3 [ 13.422215] [<ffffffff81412b6c>] usb_register_driver+0xa8/0x146 [ 13.422216] [<ffffffffa0345000>] ? 0xffffffffa0345000 [ 13.422220] [<ffffffffa034501e>] usb_audio_driver_init+0x1e/0x20 [snd_usb_audio] [ 13.422222] [<ffffffff81000380>] do_one_initcall+0x17f/0x194 [ 13.422225] [<ffffffff810c31cd>] ? __might_sleep+0x71/0x79 [ 13.422228] [<ffffffff815b96ad>] do_init_module+0x56/0x1d7 [ 13.422230] [<ffffffff81109a8b>] load_module+0x17f5/0x1c1e [ 13.422234] [<ffffffff8117994b>] ? kernel_read+0x4b/0x6d [ 13.422236] [<ffffffff8110a066>] SyS_finit_module+0x6f/0x90 [ 13.422239] [<ffffffff815c6a57>] entry_SYSCALL_64_fastpath+0x12/0x6f
Hm, so this isn't so trivial to fix. I mostly give up for 4.2, but a big hammer change like below can go into 4.3 (if it really works). Please give it a try.
thanks,
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH v2] ALSA: usb-audio: Avoid nested autoresume calls
Since snd_usb_autoresume() invokes down_read() to shutdown_rwsem, this triggers lockdep warnings like ============================================= [ INFO: possible recursive locking detected ] 4.2.0-rc8+ #61 Not tainted --------------------------------------------- pulseaudio/980 is trying to acquire lock: (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio] but task is already holding lock: (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
Also, most of these calls are together with another down_read() for checking the chip->shutdown flag, and it may trigger the similar lockdep warning, too.
This patch rewrites the logic of snd_usb_autoresume() & co; namely, - The recursive call of autopm is avoided by the new refcount, chip->active. This is atomic_t, and it's also used for the old chip->probing by increasing/decreasing this refcount. - Instead of rwsem, another refcount, chip->usage_count, is introduced for tracking the period to delay the shutdown procedure. At decreasing this refcount, wake_up() to the shutdown waiter is called. - Two new helpers are introduced to simplify the management of these refcounts; snd_usb_lock_shutdown() increases the usage_count, checks the shutdown state, and does autoresume. snd_usb_unlock_shutdown() does the opposite. Most of mixer and other codes just need this, and simply returns an error if it receives an error from lock. - By these changes, chip->in_pm and chip->autosuspended flags become superfluous, so drop them.
Reported-by: Alexnader Kuleshov kuleshovmail@gmail.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/card.c | 107 +++++++++++++++++++++------------------- sound/usb/endpoint.c | 10 ++-- sound/usb/mixer.c | 32 ++++-------- sound/usb/mixer_quirks.c | 126 ++++++++++++++++++++--------------------------- sound/usb/pcm.c | 32 ++++++------ sound/usb/proc.c | 4 +- sound/usb/usbaudio.h | 12 +++-- 7 files changed, 149 insertions(+), 174 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c index 0450593980fd..ff8bf92dabab 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -365,13 +365,14 @@ static int snd_usb_audio_create(struct usb_interface *intf, }
mutex_init(&chip->mutex); - init_rwsem(&chip->shutdown_rwsem); + init_waitqueue_head(&chip->shutdown_wait); chip->index = idx; chip->dev = dev; chip->card = card; chip->setup = device_setup[idx]; chip->autoclock = autoclock; - chip->probing = 1; + atomic_set(&chip->active, 1); /* avoid autopm during probe */ + atomic_set(&chip->usage_count, 0);
chip->usb_id = USB_ID(le16_to_cpu(dev->descriptor.idVendor), le16_to_cpu(dev->descriptor.idProduct)); @@ -495,13 +496,13 @@ static int usb_audio_probe(struct usb_interface *intf, mutex_lock(®ister_mutex); for (i = 0; i < SNDRV_CARDS; i++) { if (usb_chip[i] && usb_chip[i]->dev == dev) { - if (usb_chip[i]->shutdown) { + if (atomic_read(&usb_chip[i]->shutdown)) { dev_err(&dev->dev, "USB device is in the shutdown state, cannot create a card instance\n"); err = -EIO; goto __error; } chip = usb_chip[i]; - chip->probing = 1; + atomic_inc(&chip->active); break; } } @@ -561,8 +562,8 @@ static int usb_audio_probe(struct usb_interface *intf,
usb_chip[chip->index] = chip; chip->num_interfaces++; - chip->probing = 0; usb_set_intfdata(intf, chip); + atomic_dec(&chip->active); mutex_unlock(®ister_mutex); return 0;
@@ -570,7 +571,7 @@ static int usb_audio_probe(struct usb_interface *intf, if (chip) { if (!chip->num_interfaces) snd_card_free(chip->card); - chip->probing = 0; + atomic_dec(&chip->active); } mutex_unlock(®ister_mutex); return err; @@ -585,23 +586,20 @@ static void usb_audio_disconnect(struct usb_interface *intf) struct snd_usb_audio *chip = usb_get_intfdata(intf); struct snd_card *card; struct list_head *p; - bool was_shutdown;
if (chip == (void *)-1L) return;
card = chip->card; - down_write(&chip->shutdown_rwsem); - was_shutdown = chip->shutdown; - chip->shutdown = 1; - up_write(&chip->shutdown_rwsem);
mutex_lock(®ister_mutex); - if (!was_shutdown) { + if (atomic_inc_return(&chip->shutdown) == 1) { struct snd_usb_stream *as; struct snd_usb_endpoint *ep; struct usb_mixer_interface *mixer;
+ wait_event(chip->shutdown_wait, + !atomic_read(&chip->usage_count)); snd_card_disconnect(card); /* release the pcm resources */ list_for_each_entry(as, &chip->pcm_list, list) { @@ -631,28 +629,48 @@ static void usb_audio_disconnect(struct usb_interface *intf) } }
-#ifdef CONFIG_PM - -int snd_usb_autoresume(struct snd_usb_audio *chip) +int snd_usb_lock_shutdown(struct snd_usb_audio *chip) { - int err = -ENODEV; + int err;
- down_read(&chip->shutdown_rwsem); - if (chip->probing || chip->in_pm) - err = 0; - else if (!chip->shutdown) - err = usb_autopm_get_interface(chip->pm_intf); - up_read(&chip->shutdown_rwsem); + atomic_inc(&chip->usage_count); + if (atomic_read(&chip->shutdown)) { + err = -EIO; + goto error; + } + err = snd_usb_autoresume(chip); + if (err < 0) + goto error; + return 0;
+ error: + if (atomic_dec_and_test(&chip->usage_count)) + wake_up(&chip->shutdown_wait); return err; }
+void snd_usb_unlock_shutdown(struct snd_usb_audio *chip) +{ + snd_usb_autosuspend(chip); + if (atomic_dec_and_test(&chip->usage_count)) + wake_up(&chip->shutdown_wait); +} + +#ifdef CONFIG_PM + +int snd_usb_autoresume(struct snd_usb_audio *chip) +{ + if (atomic_read(&chip->shutdown)) { + return -EIO; + if (atomic_inc_return(&chip->active) == 1) + return usb_autopm_get_interface(chip->pm_intf); + return 0; +} + void snd_usb_autosuspend(struct snd_usb_audio *chip) { - down_read(&chip->shutdown_rwsem); - if (!chip->shutdown && !chip->probing && !chip->in_pm) + if (atomic_dec_and_test(&chip->active)) usb_autopm_put_interface(chip->pm_intf); - up_read(&chip->shutdown_rwsem); }
static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message) @@ -665,25 +683,16 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message) if (chip == (void *)-1L) return 0;
- if (!PMSG_IS_AUTO(message)) { - snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot); - if (!chip->num_suspended_intf++) { - list_for_each_entry(as, &chip->pcm_list, list) { - snd_pcm_suspend_all(as->pcm); - as->substream[0].need_setup_ep = - as->substream[1].need_setup_ep = true; - } - list_for_each(p, &chip->midi_list) { - snd_usbmidi_suspend(p); - } + snd_power_change_state(chip->card, SNDRV_CTL_POWER_D3hot); + if (!chip->num_suspended_intf++) { + list_for_each_entry(as, &chip->pcm_list, list) { + snd_pcm_suspend_all(as->pcm); + as->substream[0].need_setup_ep = + as->substream[1].need_setup_ep = true; + } + list_for_each(p, &chip->midi_list) { + snd_usbmidi_suspend(p); } - } else { - /* - * otherwise we keep the rest of the system in the dark - * to keep this transparent - */ - if (!chip->num_suspended_intf++) - chip->autosuspended = 1; }
if (chip->num_suspended_intf == 1) @@ -705,7 +714,6 @@ static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume) if (--chip->num_suspended_intf) return 0;
- chip->in_pm = 1; /* * ALSA leaves material resumption to user space * we just notify and restart the mixers @@ -713,20 +721,15 @@ static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume) list_for_each_entry(mixer, &chip->mixer_list, list) { err = snd_usb_mixer_resume(mixer, reset_resume); if (err < 0) - goto err_out; + return err; }
list_for_each(p, &chip->midi_list) { snd_usbmidi_resume(p); }
- if (!chip->autosuspended) - snd_power_change_state(chip->card, SNDRV_CTL_POWER_D0); - chip->autosuspended = 0; - -err_out: - chip->in_pm = 0; - return err; + snd_power_change_state(chip->card, SNDRV_CTL_POWER_D0); + return 0; }
static int usb_audio_resume(struct usb_interface *intf) diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 03b074419964..e6f71894ecdc 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -355,8 +355,10 @@ static void snd_complete_urb(struct urb *urb) if (unlikely(urb->status == -ENOENT || /* unlinked */ urb->status == -ENODEV || /* device removed */ urb->status == -ECONNRESET || /* unlinked */ - urb->status == -ESHUTDOWN || /* device disabled */ - ep->chip->shutdown)) /* device disconnected */ + urb->status == -ESHUTDOWN)) /* device disabled */ + goto exit_clear; + /* device disconnected */ + if (unlikely(atomic_read(&ep->chip->shutdown))) goto exit_clear;
if (usb_pipeout(ep->pipe)) { @@ -529,7 +531,7 @@ static int deactivate_urbs(struct snd_usb_endpoint *ep, bool force) { unsigned int i;
- if (!force && ep->chip->shutdown) /* to be sure... */ + if (!force && atomic_read(&ep->chip->shutdown)) /* to be sure... */ return -EBADFD;
clear_bit(EP_FLAG_RUNNING, &ep->flags); @@ -868,7 +870,7 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep) int err; unsigned int i;
- if (ep->chip->shutdown) + if (atomic_read(&ep->chip->shutdown)) return -EBADFD;
/* already running? */ diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index c50790cb3f4d..fd5c49e94867 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -311,14 +311,11 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, int timeout = 10; int idx = 0, err;
- err = snd_usb_autoresume(chip); + err = snd_usb_lock_shutdown(chip); if (err < 0) return -EIO;
- down_read(&chip->shutdown_rwsem); while (timeout-- > 0) { - if (chip->shutdown) - break; idx = snd_usb_ctrl_intf(chip) | (cval->head.id << 8); if (snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), request, USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN, @@ -334,8 +331,7 @@ static int get_ctl_value_v1(struct usb_mixer_elem_info *cval, int request, err = -EINVAL;
out: - up_read(&chip->shutdown_rwsem); - snd_usb_autosuspend(chip); + snd_usb_unlock_shutdown(chip); return err; }
@@ -358,21 +354,15 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request,
memset(buf, 0, sizeof(buf));
- ret = snd_usb_autoresume(chip) ? -EIO : 0; + ret = snd_usb_lock_shutdown(chip) ? -EIO : 0; if (ret) goto error;
- down_read(&chip->shutdown_rwsem); - if (chip->shutdown) { - ret = -ENODEV; - } else { - idx = snd_usb_ctrl_intf(chip) | (cval->head.id << 8); - ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), bRequest, + idx = snd_usb_ctrl_intf(chip) | (cval->head.id << 8); + ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), bRequest, USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_IN, validx, idx, buf, size); - } - up_read(&chip->shutdown_rwsem); - snd_usb_autosuspend(chip); + snd_usb_unlock_shutdown(chip);
if (ret < 0) { error: @@ -485,13 +475,12 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval, buf[1] = (value_set >> 8) & 0xff; buf[2] = (value_set >> 16) & 0xff; buf[3] = (value_set >> 24) & 0xff; - err = snd_usb_autoresume(chip); + + err = snd_usb_lock_shutdown(chip); if (err < 0) return -EIO; - down_read(&chip->shutdown_rwsem); + while (timeout-- > 0) { - if (chip->shutdown) - break; idx = snd_usb_ctrl_intf(chip) | (cval->head.id << 8); if (snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), request, @@ -506,8 +495,7 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval, err = -EINVAL;
out: - up_read(&chip->shutdown_rwsem); - snd_usb_autosuspend(chip); + snd_usb_unlock_shutdown(chip); return err; }
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index 337c317ead6f..d3608c0a29f3 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -308,11 +308,10 @@ static int snd_audigy2nx_led_update(struct usb_mixer_interface *mixer, struct snd_usb_audio *chip = mixer->chip; int err;
- down_read(&chip->shutdown_rwsem); - if (chip->shutdown) { - err = -ENODEV; - goto out; - } + err = snd_usb_lock_shutdown(chip); + if (err < 0) + return err; + if (chip->usb_id == USB_ID(0x041e, 0x3042)) err = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), 0x24, @@ -329,8 +328,7 @@ static int snd_audigy2nx_led_update(struct usb_mixer_interface *mixer, usb_sndctrlpipe(chip->dev, 0), 0x24, USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER, value, index + 2, NULL, 0); - out: - up_read(&chip->shutdown_rwsem); + snd_usb_unlock_shutdown(chip); return err; }
@@ -441,16 +439,15 @@ static void snd_audigy2nx_proc_read(struct snd_info_entry *entry,
for (i = 0; jacks[i].name; ++i) { snd_iprintf(buffer, "%s: ", jacks[i].name); - down_read(&mixer->chip->shutdown_rwsem); - if (mixer->chip->shutdown) - err = 0; - else - err = snd_usb_ctl_msg(mixer->chip->dev, + err = snd_usb_lock_shutdown(mixer->chip); + if (err < 0) + return; + err = snd_usb_ctl_msg(mixer->chip->dev, usb_rcvctrlpipe(mixer->chip->dev, 0), UAC_GET_MEM, USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE, 0, jacks[i].unitid << 8, buf, 3); - up_read(&mixer->chip->shutdown_rwsem); + snd_usb_unlock_shutdown(mixer->chip); if (err == 3 && (buf[0] == 3 || buf[0] == 6)) snd_iprintf(buffer, "%02x %02x\n", buf[1], buf[2]); else @@ -481,11 +478,9 @@ static int snd_emu0204_ch_switch_update(struct usb_mixer_interface *mixer, int err; unsigned char buf[2];
- down_read(&chip->shutdown_rwsem); - if (mixer->chip->shutdown) { - err = -ENODEV; - goto out; - } + err = snd_usb_lock_shutdown(chip); + if (err < 0) + return err;
buf[0] = 0x01; buf[1] = value ? 0x02 : 0x01; @@ -493,8 +488,7 @@ static int snd_emu0204_ch_switch_update(struct usb_mixer_interface *mixer, usb_sndctrlpipe(chip->dev, 0), UAC_SET_CUR, USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT, 0x0400, 0x0e00, buf, 2); - out: - up_read(&chip->shutdown_rwsem); + snd_usb_unlock_shutdown(chip); return err; }
@@ -554,15 +548,14 @@ static int snd_xonar_u1_switch_update(struct usb_mixer_interface *mixer, struct snd_usb_audio *chip = mixer->chip; int err;
- down_read(&chip->shutdown_rwsem); - if (chip->shutdown) - err = -ENODEV; - else - err = snd_usb_ctl_msg(chip->dev, + err = snd_usb_lock_shutdown(chip); + if (err < 0) + return err; + err = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), 0x08, USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_OTHER, 50, 0, &status, 1); - up_read(&chip->shutdown_rwsem); + snd_usb_unlock_shutdown(chip); return err; }
@@ -623,11 +616,9 @@ static int snd_mbox1_switch_update(struct usb_mixer_interface *mixer, int val) int err; unsigned char buff[3];
- down_read(&chip->shutdown_rwsem); - if (chip->shutdown) { - err = -ENODEV; - goto err; - } + err = snd_usb_lock_shutdown(chip); + if (err < 0) + return err;
/* Prepare for magic command to toggle clock source */ err = snd_usb_ctl_msg(chip->dev, @@ -683,7 +674,7 @@ static int snd_mbox1_switch_update(struct usb_mixer_interface *mixer, int val) goto err;
err: - up_read(&chip->shutdown_rwsem); + snd_usb_unlock_shutdown(chip); return err; }
@@ -778,15 +769,14 @@ static int snd_ni_update_cur_val(struct usb_mixer_elem_list *list) unsigned int pval = list->kctl->private_value; int err;
- down_read(&chip->shutdown_rwsem); - if (chip->shutdown) - err = -ENODEV; - else - err = usb_control_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), - (pval >> 16) & 0xff, - USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, - pval >> 24, pval & 0xffff, NULL, 0, 1000); - up_read(&chip->shutdown_rwsem); + err = snd_usb_lock_shutdown(chip); + if (err < 0) + return err; + err = usb_control_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), + (pval >> 16) & 0xff, + USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, + pval >> 24, pval & 0xffff, NULL, 0, 1000); + snd_usb_unlock_shutdown(chip); return err; }
@@ -944,18 +934,17 @@ static int snd_ftu_eff_switch_update(struct usb_mixer_elem_list *list) value[0] = pval >> 24; value[1] = 0;
- down_read(&chip->shutdown_rwsem); - if (chip->shutdown) - err = -ENODEV; - else - err = snd_usb_ctl_msg(chip->dev, - usb_sndctrlpipe(chip->dev, 0), - UAC_SET_CUR, - USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT, - pval & 0xff00, - snd_usb_ctrl_intf(chip) | ((pval & 0xff) << 8), - value, 2); - up_read(&chip->shutdown_rwsem); + err = snd_usb_lock_shutdown(chip); + if (err < 0) + return err; + err = snd_usb_ctl_msg(chip->dev, + usb_sndctrlpipe(chip->dev, 0), + UAC_SET_CUR, + USB_RECIP_INTERFACE | USB_TYPE_CLASS | USB_DIR_OUT, + pval & 0xff00, + snd_usb_ctrl_intf(chip) | ((pval & 0xff) << 8), + value, 2); + snd_usb_unlock_shutdown(chip); return err; }
@@ -1519,11 +1508,9 @@ static int snd_microii_spdif_default_get(struct snd_kcontrol *kcontrol, unsigned char data[3]; int rate;
- down_read(&chip->shutdown_rwsem); - if (chip->shutdown) { - err = -ENODEV; - goto end; - } + err = snd_usb_lock_shutdown(chip); + if (err < 0) + return err;
ucontrol->value.iec958.status[0] = kcontrol->private_value & 0xff; ucontrol->value.iec958.status[1] = (kcontrol->private_value >> 8) & 0xff; @@ -1551,7 +1538,7 @@ static int snd_microii_spdif_default_get(struct snd_kcontrol *kcontrol,
err = 0; end: - up_read(&chip->shutdown_rwsem); + snd_usb_unlock_shutdown(chip); return err; }
@@ -1562,11 +1549,9 @@ static int snd_microii_spdif_default_update(struct usb_mixer_elem_list *list) u8 reg; int err;
- down_read(&chip->shutdown_rwsem); - if (chip->shutdown) { - err = -ENODEV; - goto end; - } + err = snd_usb_lock_shutdown(chip); + if (err < 0) + return err;
reg = ((pval >> 4) & 0xf0) | (pval & 0x0f); err = snd_usb_ctl_msg(chip->dev, @@ -1594,7 +1579,7 @@ static int snd_microii_spdif_default_update(struct usb_mixer_elem_list *list) goto end;
end: - up_read(&chip->shutdown_rwsem); + snd_usb_unlock_shutdown(chip); return err; }
@@ -1650,11 +1635,9 @@ static int snd_microii_spdif_switch_update(struct usb_mixer_elem_list *list) u8 reg = list->kctl->private_value; int err;
- down_read(&chip->shutdown_rwsem); - if (chip->shutdown) { - err = -ENODEV; - goto end; - } + err = snd_usb_lock_shutdown(chip); + if (err < 0) + return err;
err = snd_usb_ctl_msg(chip->dev, usb_sndctrlpipe(chip->dev, 0), @@ -1665,8 +1648,7 @@ static int snd_microii_spdif_switch_update(struct usb_mixer_elem_list *list) NULL, 0);
- end: - up_read(&chip->shutdown_rwsem); + snd_usb_unlock_shutdown(chip); return err; }
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 30797269d5aa..cdac5179db3f 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -80,7 +80,7 @@ static snd_pcm_uframes_t snd_usb_pcm_pointer(struct snd_pcm_substream *substream unsigned int hwptr_done;
subs = (struct snd_usb_substream *)substream->runtime->private_data; - if (subs->stream->chip->shutdown) + if (atomic_read(&subs->stream->chip->shutdown)) return SNDRV_PCM_POS_XRUN; spin_lock(&subs->lock); hwptr_done = subs->hwptr_done; @@ -735,12 +735,11 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }
- down_read(&subs->stream->chip->shutdown_rwsem); - if (subs->stream->chip->shutdown) - ret = -ENODEV; - else - ret = set_format(subs, fmt); - up_read(&subs->stream->chip->shutdown_rwsem); + ret = snd_usb_lock_shutdown(subs->stream->chip); + if (ret < 0) + return ret; + ret = set_format(subs, fmt); + snd_usb_unlock_shutdown(subs->stream->chip); if (ret < 0) return ret;
@@ -763,13 +762,12 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream) subs->cur_audiofmt = NULL; subs->cur_rate = 0; subs->period_bytes = 0; - down_read(&subs->stream->chip->shutdown_rwsem); - if (!subs->stream->chip->shutdown) { + if (!snd_usb_lock_shutdown(subs->stream->chip)) { stop_endpoints(subs, true); snd_usb_endpoint_deactivate(subs->sync_endpoint); snd_usb_endpoint_deactivate(subs->data_endpoint); + snd_usb_unlock_shutdown(subs->stream->chip); } - up_read(&subs->stream->chip->shutdown_rwsem); return snd_pcm_lib_free_vmalloc_buffer(substream); }
@@ -791,11 +789,9 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) return -ENXIO; }
- down_read(&subs->stream->chip->shutdown_rwsem); - if (subs->stream->chip->shutdown) { - ret = -ENODEV; - goto unlock; - } + ret = snd_usb_lock_shutdown(subs->stream->chip); + if (ret < 0) + return ret; if (snd_BUG_ON(!subs->data_endpoint)) { ret = -EIO; goto unlock; @@ -844,7 +840,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) ret = start_endpoints(subs, true);
unlock: - up_read(&subs->stream->chip->shutdown_rwsem); + snd_usb_unlock_shutdown(subs->stream->chip); return ret; }
@@ -1246,9 +1242,11 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
stop_endpoints(subs, true);
- if (!as->chip->shutdown && subs->interface >= 0) { + if (subs->interface >= 0 && + !snd_usb_lock_shutdown(subs->stream->chip)) { usb_set_interface(subs->dev, subs->interface, 0); subs->interface = -1; + snd_usb_unlock_shutdown(subs->stream->chip); }
subs->pcm_substream = NULL; diff --git a/sound/usb/proc.c b/sound/usb/proc.c index 5f761ab34c01..0ac89e294d31 100644 --- a/sound/usb/proc.c +++ b/sound/usb/proc.c @@ -46,14 +46,14 @@ static inline unsigned get_high_speed_hz(unsigned int usb_rate) static void proc_audio_usbbus_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer) { struct snd_usb_audio *chip = entry->private_data; - if (!chip->shutdown) + if (!atomic_read(&chip->shutdown)) snd_iprintf(buffer, "%03d/%03d\n", chip->dev->bus->busnum, chip->dev->devnum); }
static void proc_audio_usbid_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer) { struct snd_usb_audio *chip = entry->private_data; - if (!chip->shutdown) + if (!atomic_read(&chip->shutdown)) snd_iprintf(buffer, "%04x:%04x\n", USB_ID_VENDOR(chip->usb_id), USB_ID_PRODUCT(chip->usb_id)); diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index 91d0380431b4..46cf8d14415f 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -37,11 +37,10 @@ struct snd_usb_audio { struct usb_interface *pm_intf; u32 usb_id; struct mutex mutex; - struct rw_semaphore shutdown_rwsem; - unsigned int shutdown:1; - unsigned int probing:1; - unsigned int in_pm:1; - unsigned int autosuspended:1; + atomic_t active; + atomic_t shutdown; + atomic_t usage_count; + wait_queue_head_t shutdown_wait; unsigned int txfr_quirk:1; /* Subframe boundaries on transfers */ int num_interfaces; @@ -115,4 +114,7 @@ struct snd_usb_audio_quirk { #define combine_triple(s) (combine_word(s) | ((unsigned int)(s)[2] << 16)) #define combine_quad(s) (combine_triple(s) | ((unsigned int)(s)[3] << 24))
+int snd_usb_lock_shutdown(struct snd_usb_audio *chip); +void snd_usb_unlock_shutdown(struct snd_usb_audio *chip); + #endif /* __USBAUDIO_H */