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