ALSA/usb-audio: snd_usb_autoresume possible recursive locking detected

ALSA/usb-audio: snd_usb_autoresume possible recursive locking detected

[Alexnader Kuleshov]

Hello all,

Today I've update to 4.2.0-rc8+ and started to get following lines in the
dmesg output:

[   13.884092] =============================================
[   13.884092] [ INFO: possible recursive locking detected ]
[   13.884094] 4.2.0-rc8+ #61 Not tainted
[   13.884094] ---------------------------------------------
[   13.884095] pulseaudio/980 is trying to acquire lock:
[   13.884096]  (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
[   13.884103] 
               but task is already holding lock:
[   13.884104]  (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
[   13.884107] 
               other info that might help us debug this:
[   13.884108]  Possible unsafe locking scenario:

[   13.884109]        CPU0
[   13.884110]        ----
[   13.884110]   lock(&chip->shutdown_rwsem);
[   13.884111]   lock(&chip->shutdown_rwsem);
[   13.884112] 
                *** DEADLOCK ***

[   13.884113]  May be due to missing lock nesting notation

[   13.884114] 2 locks held by pulseaudio/980:
[   13.884115]  #0:  (&pcm->open_mutex){+.+.+.}, at: [<ffffffffa02ac9bc>] snd_pcm_open+0xa9/0x1f8 [snd_pcm]
[   13.884120]  #1:  (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
[   13.884124] 
               stack backtrace:
[   13.884125] CPU: 0 PID: 980 Comm: pulseaudio Not tainted 4.2.0-rc8+ #61
[   13.884126] Hardware name: Gigabyte Technology Co., Ltd. Z97X-UD5H-BK/Z97X-UD5H-BK, BIOS F6 06/17/2014
[   13.884127]  0000000000000000 000000003d4c66e6 ffff88040897b4d8 ffffffff815ba622
[   13.884129]  0000000000000000 ffffffff82399320 ffff88040897b598 ffffffff810dd54e
[   13.884130]  ffff88040ac486a8 0000000000000000 0000000000000001 ffffffff82399320
[   13.884132] Call Trace:
[   13.884134]  [<ffffffff815ba622>] dump_stack+0x4c/0x65
[   13.884137]  [<ffffffff810dd54e>] validate_chain.isra.9+0x75d/0xf59
[   13.884139]  [<ffffffff815c62b3>] ? _raw_spin_unlock_irq+0x28/0x34
[   13.884140]  [<ffffffff810e0964>] __lock_acquire+0x753/0xabf
[   13.884142]  [<ffffffff81163819>] ? kfree+0xb8/0x10d
[   13.884143]  [<ffffffff810e1131>] lock_acquire+0x7b/0x9c
[   13.884146]  [<ffffffffa0355dac>] ? snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
[   13.884147]  [<ffffffff815c4896>] down_read+0x44/0x8d
[   13.884150]  [<ffffffffa0355dac>] ? snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
[   13.884152]  [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
[   13.884153]  [<ffffffff815c37e5>] ? __mutex_unlock_slowpath+0x110/0x11b
[   13.884156]  [<ffffffffa0359338>] snd_usb_mixer_set_ctl_value+0x9a/0x16d [snd_usb_audio]
[   13.884159]  [<ffffffffa035957e>] snd_usb_set_cur_mix_value+0x4d/0x77 [snd_usb_audio]
[   13.884161]  [<ffffffffa0359db9>] restore_mixer_value+0x74/0x87 [snd_usb_audio]
[   13.884164]  [<ffffffffa035b581>] snd_usb_mixer_resume+0x64/0x70 [snd_usb_audio]
[   13.884166]  [<ffffffffa035505c>] __usb_audio_resume+0x5c/0xd8 [snd_usb_audio]
[   13.884169]  [<ffffffff81413ca4>] ? usb_runtime_suspend+0x63/0x63
[   13.884171]  [<ffffffffa03550e6>] usb_audio_reset_resume+0xe/0x10 [snd_usb_audio]
[   13.884172]  [<ffffffff81412fde>] usb_resume_interface.isra.1+0x6c/0xbd
[   13.884174]  [<ffffffff814130cf>] usb_resume_both+0xa0/0xc0
[   13.884175]  [<ffffffff81413cb9>] usb_runtime_resume+0x15/0x17
[   13.884178]  [<ffffffff813b92b0>] __rpm_callback+0x35/0x5d
[   13.884179]  [<ffffffff81413ca4>] ? usb_runtime_suspend+0x63/0x63
[   13.884180]  [<ffffffff813b9330>] rpm_callback+0x58/0x77
[   13.884182]  [<ffffffff81413ca4>] ? usb_runtime_suspend+0x63/0x63
[   13.884183]  [<ffffffff813ba0db>] rpm_resume+0x370/0x412
[   13.884184]  [<ffffffff813ba06c>] rpm_resume+0x301/0x412
[   13.884186]  [<ffffffff813ba1e3>] __pm_runtime_resume+0x66/0x82
[   13.884187]  [<ffffffff81412e10>] usb_autopm_get_interface+0x1e/0x48
[   13.884190]  [<ffffffffa0355dcc>] snd_usb_autoresume+0x3d/0x52 [snd_usb_audio]
[   13.884191]  [<ffffffff810d84b9>] ? __init_waitqueue_head+0x37/0x4b
[   13.884194]  [<ffffffffa035e765>] snd_usb_pcm_open+0x181/0x3bb [snd_usb_audio]
[   13.884197]  [<ffffffffa035e9ad>] snd_usb_capture_open+0xe/0x10 [snd_usb_audio]
[   13.884200]  [<ffffffffa02ac8c6>] snd_pcm_open_substream+0x50/0x9d [snd_pcm]
[   13.884203]  [<ffffffffa02ac9ce>] snd_pcm_open+0xbb/0x1f8 [snd_pcm]
[   13.884204]  [<ffffffff810de87d>] ? trace_hardirqs_on+0xd/0xf
[   13.884206]  [<ffffffff810c5550>] ? wake_up_q+0x53/0x53
[   13.884209]  [<ffffffffa02acb49>] snd_pcm_capture_open+0x3e/0x61 [snd_pcm]
[   13.884212]  [<ffffffffa028232d>] snd_open+0x130/0x13f [snd]
[   13.884214]  [<ffffffff811788c5>] chrdev_open+0x13b/0x175
[   13.884215]  [<ffffffff8117878a>] ? cdev_put+0x20/0x20
[   13.884217]  [<ffffffff81172f31>] do_dentry_open.isra.1+0x1d2/0x2c9
[   13.884218]  [<ffffffff8117d0e4>] ? __inode_permission+0x84/0x95
[   13.884219]  [<ffffffff81173bf6>] vfs_open+0x50/0x54
[   13.884221]  [<ffffffff811815e0>] path_openat+0xa90/0xd0f
[   13.884222]  [<ffffffff810e0900>] ? __lock_acquire+0x6ef/0xabf
[   13.884223]  [<ffffffff8118269e>] do_filp_open+0x48/0xac
[   13.884225]  [<ffffffff815c6203>] ? _raw_spin_unlock+0x23/0x2e
[   13.884227]  [<ffffffff8118f5b7>] ? __alloc_fd+0x144/0x156
[   13.884228]  [<ffffffff81173f3a>] do_sys_open+0x154/0x1f7
[   13.884229]  [<ffffffff81173ff6>] SyS_open+0x19/0x1b
[   13.884231]  [<ffffffff815c6a57>] entry_SYSCALL_64_fastpath+0x12/0x6f

Previous version of the kernel was 4.2.0-rc6+.

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

[Takashi Iwai]

On Tue, 25 Aug 2015 15:32:56 +0200,
Alexnader Kuleshov wrote:
> 
> Hello all,
> 
> Today I've update to 4.2.0-rc8+ and started to get following lines in the
> dmesg output:
> 
> [   13.884092] =============================================
> [   13.884092] [ INFO: possible recursive locking detected ]
> [   13.884094] 4.2.0-rc8+ #61 Not tainted
> [   13.884094] ---------------------------------------------
> [   13.884095] pulseaudio/980 is trying to acquire lock:
> [   13.884096]  (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
> [   13.884103] 
>                but task is already holding lock:
> [   13.884104]  (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
> [   13.884107] 
>                other info that might help us debug this:
> [   13.884108]  Possible unsafe locking scenario:
> 
> [   13.884109]        CPU0
> [   13.884110]        ----
> [   13.884110]   lock(&chip->shutdown_rwsem);
> [   13.884111]   lock(&chip->shutdown_rwsem);
> [   13.884112] 
>                 *** DEADLOCK ***
> 
> [   13.884113]  May be due to missing lock nesting notation
> 
> [   13.884114] 2 locks held by pulseaudio/980:
> [   13.884115]  #0:  (&pcm->open_mutex){+.+.+.}, at: [<ffffffffa02ac9bc>] snd_pcm_open+0xa9/0x1f8 [snd_pcm]
> [   13.884120]  #1:  (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
> [   13.884124] 
>                stack backtrace:
> [   13.884125] CPU: 0 PID: 980 Comm: pulseaudio Not tainted 4.2.0-rc8+ #61
> [   13.884126] Hardware name: Gigabyte Technology Co., Ltd. Z97X-UD5H-BK/Z97X-UD5H-BK, BIOS F6 06/17/2014
> [   13.884127]  0000000000000000 000000003d4c66e6 ffff88040897b4d8 ffffffff815ba622
> [   13.884129]  0000000000000000 ffffffff82399320 ffff88040897b598 ffffffff810dd54e
> [   13.884130]  ffff88040ac486a8 0000000000000000 0000000000000001 ffffffff82399320
> [   13.884132] Call Trace:
> [   13.884134]  [<ffffffff815ba622>] dump_stack+0x4c/0x65
> [   13.884137]  [<ffffffff810dd54e>] validate_chain.isra.9+0x75d/0xf59
> [   13.884139]  [<ffffffff815c62b3>] ? _raw_spin_unlock_irq+0x28/0x34
> [   13.884140]  [<ffffffff810e0964>] __lock_acquire+0x753/0xabf
> [   13.884142]  [<ffffffff81163819>] ? kfree+0xb8/0x10d
> [   13.884143]  [<ffffffff810e1131>] lock_acquire+0x7b/0x9c
> [   13.884146]  [<ffffffffa0355dac>] ? snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
> [   13.884147]  [<ffffffff815c4896>] down_read+0x44/0x8d
> [   13.884150]  [<ffffffffa0355dac>] ? snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
> [   13.884152]  [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
> [   13.884153]  [<ffffffff815c37e5>] ? __mutex_unlock_slowpath+0x110/0x11b
> [   13.884156]  [<ffffffffa0359338>] snd_usb_mixer_set_ctl_value+0x9a/0x16d [snd_usb_audio]
> [   13.884159]  [<ffffffffa035957e>] snd_usb_set_cur_mix_value+0x4d/0x77 [snd_usb_audio]
> [   13.884161]  [<ffffffffa0359db9>] restore_mixer_value+0x74/0x87 [snd_usb_audio]
> [   13.884164]  [<ffffffffa035b581>] snd_usb_mixer_resume+0x64/0x70 [snd_usb_audio]
> [   13.884166]  [<ffffffffa035505c>] __usb_audio_resume+0x5c/0xd8 [snd_usb_audio]
> [   13.884169]  [<ffffffff81413ca4>] ? usb_runtime_suspend+0x63/0x63
> [   13.884171]  [<ffffffffa03550e6>] usb_audio_reset_resume+0xe/0x10 [snd_usb_audio]
> [   13.884172]  [<ffffffff81412fde>] usb_resume_interface.isra.1+0x6c/0xbd
> [   13.884174]  [<ffffffff814130cf>] usb_resume_both+0xa0/0xc0
> [   13.884175]  [<ffffffff81413cb9>] usb_runtime_resume+0x15/0x17
> [   13.884178]  [<ffffffff813b92b0>] __rpm_callback+0x35/0x5d
> [   13.884179]  [<ffffffff81413ca4>] ? usb_runtime_suspend+0x63/0x63
> [   13.884180]  [<ffffffff813b9330>] rpm_callback+0x58/0x77
> [   13.884182]  [<ffffffff81413ca4>] ? usb_runtime_suspend+0x63/0x63
> [   13.884183]  [<ffffffff813ba0db>] rpm_resume+0x370/0x412
> [   13.884184]  [<ffffffff813ba06c>] rpm_resume+0x301/0x412
> [   13.884186]  [<ffffffff813ba1e3>] __pm_runtime_resume+0x66/0x82
> [   13.884187]  [<ffffffff81412e10>] usb_autopm_get_interface+0x1e/0x48
> [   13.884190]  [<ffffffffa0355dcc>] snd_usb_autoresume+0x3d/0x52 [snd_usb_audio]
> [   13.884191]  [<ffffffff810d84b9>] ? __init_waitqueue_head+0x37/0x4b
> [   13.884194]  [<ffffffffa035e765>] snd_usb_pcm_open+0x181/0x3bb [snd_usb_audio]
> [   13.884197]  [<ffffffffa035e9ad>] snd_usb_capture_open+0xe/0x10 [snd_usb_audio]
> [   13.884200]  [<ffffffffa02ac8c6>] snd_pcm_open_substream+0x50/0x9d [snd_pcm]
> [   13.884203]  [<ffffffffa02ac9ce>] snd_pcm_open+0xbb/0x1f8 [snd_pcm]
> [   13.884204]  [<ffffffff810de87d>] ? trace_hardirqs_on+0xd/0xf
> [   13.884206]  [<ffffffff810c5550>] ? wake_up_q+0x53/0x53
> [   13.884209]  [<ffffffffa02acb49>] snd_pcm_capture_open+0x3e/0x61 [snd_pcm]
> [   13.884212]  [<ffffffffa028232d>] snd_open+0x130/0x13f [snd]
> [   13.884214]  [<ffffffff811788c5>] chrdev_open+0x13b/0x175
> [   13.884215]  [<ffffffff8117878a>] ? cdev_put+0x20/0x20
> [   13.884217]  [<ffffffff81172f31>] do_dentry_open.isra.1+0x1d2/0x2c9
> [   13.884218]  [<ffffffff8117d0e4>] ? __inode_permission+0x84/0x95
> [   13.884219]  [<ffffffff81173bf6>] vfs_open+0x50/0x54
> [   13.884221]  [<ffffffff811815e0>] path_openat+0xa90/0xd0f
> [   13.884222]  [<ffffffff810e0900>] ? __lock_acquire+0x6ef/0xabf
> [   13.884223]  [<ffffffff8118269e>] do_filp_open+0x48/0xac
> [   13.884225]  [<ffffffff815c6203>] ? _raw_spin_unlock+0x23/0x2e
> [   13.884227]  [<ffffffff8118f5b7>] ? __alloc_fd+0x144/0x156
> [   13.884228]  [<ffffffff81173f3a>] do_sys_open+0x154/0x1f7
> [   13.884229]  [<ffffffff81173ff6>] SyS_open+0x19/0x1b
> [   13.884231]  [<ffffffff815c6a57>] entry_SYSCALL_64_fastpath+0x12/0x6f
> 
> Previous version of the kernel was 4.2.0-rc6+.

This should be a false positive warning, a side-effect now by fix of
runtime PM.  That is, it proves that the runtime PM is activated.
I'll consider to reduce this later.


thanks,

Takashi

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

[Takashi Iwai]

On Tue, 25 Aug 2015 15:48:58 +0200,
Takashi Iwai wrote:
> 
> On Tue, 25 Aug 2015 15:32:56 +0200,
> Alexnader Kuleshov wrote:
> > 
> > Hello all,
> > 
> > Today I've update to 4.2.0-rc8+ and started to get following lines in the
> > dmesg output:
> > 
> > [   13.884092] =============================================
> > [   13.884092] [ INFO: possible recursive locking detected ]
> > [   13.884094] 4.2.0-rc8+ #61 Not tainted
> > [   13.884094] ---------------------------------------------
> > [   13.884095] pulseaudio/980 is trying to acquire lock:
> > [   13.884096]  (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
> > [   13.884103] 
> >                but task is already holding lock:
> > [   13.884104]  (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
> > [   13.884107] 
> >                other info that might help us debug this:
> > [   13.884108]  Possible unsafe locking scenario:
> > 
> > [   13.884109]        CPU0
> > [   13.884110]        ----
> > [   13.884110]   lock(&chip->shutdown_rwsem);
> > [   13.884111]   lock(&chip->shutdown_rwsem);
> > [   13.884112] 
> >                 *** DEADLOCK ***
> > 
> > [   13.884113]  May be due to missing lock nesting notation
> > 
> > [   13.884114] 2 locks held by pulseaudio/980:
> > [   13.884115]  #0:  (&pcm->open_mutex){+.+.+.}, at: [<ffffffffa02ac9bc>] snd_pcm_open+0xa9/0x1f8 [snd_pcm]
> > [   13.884120]  #1:  (&chip->shutdown_rwsem){.+.+.+}, at: [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
> > [   13.884124] 
> >                stack backtrace:
> > [   13.884125] CPU: 0 PID: 980 Comm: pulseaudio Not tainted 4.2.0-rc8+ #61
> > [   13.884126] Hardware name: Gigabyte Technology Co., Ltd. Z97X-UD5H-BK/Z97X-UD5H-BK, BIOS F6 06/17/2014
> > [   13.884127]  0000000000000000 000000003d4c66e6 ffff88040897b4d8 ffffffff815ba622
> > [   13.884129]  0000000000000000 ffffffff82399320 ffff88040897b598 ffffffff810dd54e
> > [   13.884130]  ffff88040ac486a8 0000000000000000 0000000000000001 ffffffff82399320
> > [   13.884132] Call Trace:
> > [   13.884134]  [<ffffffff815ba622>] dump_stack+0x4c/0x65
> > [   13.884137]  [<ffffffff810dd54e>] validate_chain.isra.9+0x75d/0xf59
> > [   13.884139]  [<ffffffff815c62b3>] ? _raw_spin_unlock_irq+0x28/0x34
> > [   13.884140]  [<ffffffff810e0964>] __lock_acquire+0x753/0xabf
> > [   13.884142]  [<ffffffff81163819>] ? kfree+0xb8/0x10d
> > [   13.884143]  [<ffffffff810e1131>] lock_acquire+0x7b/0x9c
> > [   13.884146]  [<ffffffffa0355dac>] ? snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
> > [   13.884147]  [<ffffffff815c4896>] down_read+0x44/0x8d
> > [   13.884150]  [<ffffffffa0355dac>] ? snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
> > [   13.884152]  [<ffffffffa0355dac>] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio]
> > [   13.884153]  [<ffffffff815c37e5>] ? __mutex_unlock_slowpath+0x110/0x11b
> > [   13.884156]  [<ffffffffa0359338>] snd_usb_mixer_set_ctl_value+0x9a/0x16d [snd_usb_audio]
> > [   13.884159]  [<ffffffffa035957e>] snd_usb_set_cur_mix_value+0x4d/0x77 [snd_usb_audio]
> > [   13.884161]  [<ffffffffa0359db9>] restore_mixer_value+0x74/0x87 [snd_usb_audio]
> > [   13.884164]  [<ffffffffa035b581>] snd_usb_mixer_resume+0x64/0x70 [snd_usb_audio]
> > [   13.884166]  [<ffffffffa035505c>] __usb_audio_resume+0x5c/0xd8 [snd_usb_audio]
> > [   13.884169]  [<ffffffff81413ca4>] ? usb_runtime_suspend+0x63/0x63
> > [   13.884171]  [<ffffffffa03550e6>] usb_audio_reset_resume+0xe/0x10 [snd_usb_audio]
> > [   13.884172]  [<ffffffff81412fde>] usb_resume_interface.isra.1+0x6c/0xbd
> > [   13.884174]  [<ffffffff814130cf>] usb_resume_both+0xa0/0xc0
> > [   13.884175]  [<ffffffff81413cb9>] usb_runtime_resume+0x15/0x17
> > [   13.884178]  [<ffffffff813b92b0>] __rpm_callback+0x35/0x5d
> > [   13.884179]  [<ffffffff81413ca4>] ? usb_runtime_suspend+0x63/0x63
> > [   13.884180]  [<ffffffff813b9330>] rpm_callback+0x58/0x77
> > [   13.884182]  [<ffffffff81413ca4>] ? usb_runtime_suspend+0x63/0x63
> > [   13.884183]  [<ffffffff813ba0db>] rpm_resume+0x370/0x412
> > [   13.884184]  [<ffffffff813ba06c>] rpm_resume+0x301/0x412
> > [   13.884186]  [<ffffffff813ba1e3>] __pm_runtime_resume+0x66/0x82
> > [   13.884187]  [<ffffffff81412e10>] usb_autopm_get_interface+0x1e/0x48
> > [   13.884190]  [<ffffffffa0355dcc>] snd_usb_autoresume+0x3d/0x52 [snd_usb_audio]
> > [   13.884191]  [<ffffffff810d84b9>] ? __init_waitqueue_head+0x37/0x4b
> > [   13.884194]  [<ffffffffa035e765>] snd_usb_pcm_open+0x181/0x3bb [snd_usb_audio]
> > [   13.884197]  [<ffffffffa035e9ad>] snd_usb_capture_open+0xe/0x10 [snd_usb_audio]
> > [   13.884200]  [<ffffffffa02ac8c6>] snd_pcm_open_substream+0x50/0x9d [snd_pcm]
> > [   13.884203]  [<ffffffffa02ac9ce>] snd_pcm_open+0xbb/0x1f8 [snd_pcm]
> > [   13.884204]  [<ffffffff810de87d>] ? trace_hardirqs_on+0xd/0xf
> > [   13.884206]  [<ffffffff810c5550>] ? wake_up_q+0x53/0x53
> > [   13.884209]  [<ffffffffa02acb49>] snd_pcm_capture_open+0x3e/0x61 [snd_pcm]
> > [   13.884212]  [<ffffffffa028232d>] snd_open+0x130/0x13f [snd]
> > [   13.884214]  [<ffffffff811788c5>] chrdev_open+0x13b/0x175
> > [   13.884215]  [<ffffffff8117878a>] ? cdev_put+0x20/0x20
> > [   13.884217]  [<ffffffff81172f31>] do_dentry_open.isra.1+0x1d2/0x2c9
> > [   13.884218]  [<ffffffff8117d0e4>] ? __inode_permission+0x84/0x95
> > [   13.884219]  [<ffffffff81173bf6>] vfs_open+0x50/0x54
> > [   13.884221]  [<ffffffff811815e0>] path_openat+0xa90/0xd0f
> > [   13.884222]  [<ffffffff810e0900>] ? __lock_acquire+0x6ef/0xabf
> > [   13.884223]  [<ffffffff8118269e>] do_filp_open+0x48/0xac
> > [   13.884225]  [<ffffffff815c6203>] ? _raw_spin_unlock+0x23/0x2e
> > [   13.884227]  [<ffffffff8118f5b7>] ? __alloc_fd+0x144/0x156
> > [   13.884228]  [<ffffffff81173f3a>] do_sys_open+0x154/0x1f7
> > [   13.884229]  [<ffffffff81173ff6>] SyS_open+0x19/0x1b
> > [   13.884231]  [<ffffffff815c6a57>] entry_SYSCALL_64_fastpath+0x12/0x6f
> > 
> > Previous version of the kernel was 4.2.0-rc6+.
> 
> This should be a false positive warning, a side-effect now by fix of
> runtime PM.  That is, it proves that the runtime PM is activated.
> I'll consider to reduce this later.

Could you try the patch below?


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] 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]

One could avoid this with down_read_nested().  But a quicker solution
is just to skip snd_usb_autoresume() call and relevant down_read()
inside the resume path.  This is already marked via chip->in_pm flag,
so we can check it easily.

This patch adds the workaround in the regular resume path (via
snd_usb_mixer_set_ctl_value()).  A more comprehensive coverage to all
resume paths will follow later.

Reported-by: Alexnader Kuleshov <kuleshovmail@gmail.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/mixer.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index c50790cb3f4d..dd9caac4998a 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -463,6 +463,7 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
 	struct snd_usb_audio *chip = cval->head.mixer->chip;
 	unsigned char buf[4];
 	int idx = 0, val_len, err, timeout = 10;
+	bool autoresume;
 
 	validx += cval->idx_off;
 
@@ -485,10 +486,16 @@ 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);
-	if (err < 0)
-		return -EIO;
-	down_read(&chip->shutdown_rwsem);
+
+	/* do autoresume and lock only when invoked from non-resume path */
+	autoresume = !chip->in_pm;
+	if (autoresume) {
+		err = snd_usb_autoresume(chip);
+		if (err < 0)
+			return -EIO;
+		down_read(&chip->shutdown_rwsem);
+	}
+
 	while (timeout-- > 0) {
 		if (chip->shutdown)
 			break;
@@ -506,8 +513,10 @@ 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);
+	if (autoresume) {
+		up_read(&chip->shutdown_rwsem);
+		snd_usb_autosuspend(chip);
+	}
 	return err;
 }
 
-- 
2.5.0

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

[Alexnader Kuleshov]

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


Thank you.

On 08-25-15, Takashi Iwai wrote:
> 
> Could you try the patch below?
> 
> 
> Takashi
> 
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] 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]
> 
> One could avoid this with down_read_nested().  But a quicker solution
> is just to skip snd_usb_autoresume() call and relevant down_read()
> inside the resume path.  This is already marked via chip->in_pm flag,
> so we can check it easily.
> 
> This patch adds the workaround in the regular resume path (via
> snd_usb_mixer_set_ctl_value()).  A more comprehensive coverage to all
> resume paths will follow later.
> 
> Reported-by: Alexnader Kuleshov <kuleshovmail@gmail.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/usb/mixer.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index c50790cb3f4d..dd9caac4998a 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -463,6 +463,7 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval,
>  	struct snd_usb_audio *chip = cval->head.mixer->chip;
>  	unsigned char buf[4];
>  	int idx = 0, val_len, err, timeout = 10;
> +	bool autoresume;
>  
>  	validx += cval->idx_off;
>  
> @@ -485,10 +486,16 @@ 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);
> -	if (err < 0)
> -		return -EIO;
> -	down_read(&chip->shutdown_rwsem);
> +
> +	/* do autoresume and lock only when invoked from non-resume path */
> +	autoresume = !chip->in_pm;
> +	if (autoresume) {
> +		err = snd_usb_autoresume(chip);
> +		if (err < 0)
> +			return -EIO;
> +		down_read(&chip->shutdown_rwsem);
> +	}
> +
>  	while (timeout-- > 0) {
>  		if (chip->shutdown)
>  			break;
> @@ -506,8 +513,10 @@ 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);
> +	if (autoresume) {
> +		up_read(&chip->shutdown_rwsem);
> +		snd_usb_autosuspend(chip);
> +	}
>  	return err;
>  }
>  
> -- 
> 2.5.0
> 

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@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(&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

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

[Alexander Kuleshov]

I've applied this patch agains your for-next tree
(https//git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git), but
it does not
compile.

Compilation output: http://pastebin.com/hrN196Zs

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

[Takashi Iwai]

On Tue, 25 Aug 2015 21:31:27 +0200,
Alexander Kuleshov wrote:
> 
> I've applied this patch agains your for-next tree
> (https//git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git), but
> it does not
> compile.

Sorry, a wrong file was attached.  Below is the correct one.


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..fc8cb60cc7c6 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

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

[Alexander Kuleshov]

On 08-25-15, Takashi Iwai wrote:
> Sorry, a wrong file was attached.  Below is the correct one.

I've applied last patch against your for-next tree and seems that
it brings more problems. I see following messages:

1. http://pastebin.com/ggC1Nm6X

2. http://pastebin.com/F4cpyzjm

And moreover some userspace apps hangs and I can't reboot with it
only hard reset.

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

[Takashi Iwai]

On Wed, 26 Aug 2015 09:40:02 +0200,
Alexander Kuleshov wrote:
> 
> On 08-25-15, Takashi Iwai wrote:
> > Sorry, a wrong file was attached.  Below is the correct one.
> 
> I've applied last patch against your for-next tree and seems that
> it brings more problems. I see following messages:
> 
> 1. http://pastebin.com/ggC1Nm6X
> 
> 2. http://pastebin.com/F4cpyzjm
> 
> And moreover some userspace apps hangs and I can't reboot with it
> only hard reset.

Ah, sorry, this was the missing refcount increment at resume.
Below is the revised patch.  In the final form, it'll be split to a
few parts, but now it's all folded for ease of testing.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] 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.
- 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.

Fixes: 9003ebb13f61 ('ALSA: usb-audio: Fix runtime PM unbalance')
Reported-by: Alexnader Kuleshov <kuleshovmail@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/card.c         | 106 ++++++++++++++++++++-------------------
 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(+), 173 deletions(-)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 0450593980fd..80c99fde234b 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 probing */
+	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); /* avoid autopm */
 			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,30 +683,18 @@ 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;
 		}
-	} 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)
+		list_for_each(p, &chip->midi_list)
+			snd_usbmidi_suspend(p);
 		list_for_each_entry(mixer, &chip->mixer_list, list)
 			snd_usb_mixer_suspend(mixer);
+	}
 
 	return 0;
 }
@@ -705,7 +711,7 @@ static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume)
 	if (--chip->num_suspended_intf)
 		return 0;
 
-	chip->in_pm = 1;
+	atomic_inc(&chip->active);
 	/*
 	 * ALSA leaves material resumption to user space
 	 * we just notify and restart the mixers
@@ -720,12 +726,10 @@ static int __usb_audio_resume(struct usb_interface *intf, bool reset_resume)
 		snd_usbmidi_resume(p);
 	}
 
-	if (!chip->autosuspended)
-		snd_power_change_state(chip->card, SNDRV_CTL_POWER_D0);
-	chip->autosuspended = 0;
+	snd_power_change_state(chip->card, SNDRV_CTL_POWER_D0);
 
 err_out:
-	chip->in_pm = 0;
+	atomic_dec(&chip->active);
 	return err;
 }
 
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

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

[Alexander Kuleshov]

On 08-26-15, Takashi Iwai wrote:
> 
> Ah, sorry, this was the missing refcount increment at resume.
> Below is the revised patch.  In the final form, it'll be split to a
> few parts, but now it's all folded for ease of testing.
> 

Hello Takashi,

sorry for delay, just made a test with the last patch and finally
dmesg is clear.

Thank you.

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

[Takashi Iwai]

On Wed, 26 Aug 2015 15:16:43 +0200,
Alexander Kuleshov wrote:
> 
> On 08-26-15, Takashi Iwai wrote:
> > 
> > Ah, sorry, this was the missing refcount increment at resume.
> > Below is the revised patch.  In the final form, it'll be split to a
> > few parts, but now it's all folded for ease of testing.
> > 
> 
> Hello Takashi,
> 
> sorry for delay, just made a test with the last patch and finally
> dmesg is clear.

Great, thanks for many tests!

I tried the autopm on my machine, but strangely I couldn't reproduce
this lockdep warning.  Could you give your kconfig?  I'd like to
check.


Takashi

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

[Alexander Kuleshov]

On 08-26-15, Takashi Iwai wrote:
> On Wed, 26 Aug 2015 15:16:43 +0200,
> Alexander Kuleshov wrote:
> > 
> > On 08-26-15, Takashi Iwai wrote:
> > > 
> > > Ah, sorry, this was the missing refcount increment at resume.
> > > Below is the revised patch.  In the final form, it'll be split to a
> > > few parts, but now it's all folded for ease of testing.
> > > 
> > 
> > Hello Takashi,
> > 
> > sorry for delay, just made a test with the last patch and finally
> > dmesg is clear.
> 
> Great, thanks for many tests!
> 
> I tried the autopm on my machine, but strangely I couldn't reproduce
> this lockdep warning.  Could you give your kconfig?  I'd like to
> check.

Yes, my .config: http://pastebin.com/N2e8TTtK