Re: ALSA/usb-audio: snd_usb_autoresume possible recursive locking detected

[Takashi Iwai]

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@xxxxxxx>
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@xxxxxxxxx>
Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
---
 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(&register_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(&register_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(&register_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(&register_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 */
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/