[alsa-devel] ALSA/usb-audio: snd_usb_autoresume possible recursive locking detected
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+.
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
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; }
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
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 */
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
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 */
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.
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:
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 */
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.
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
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
participants (3)
-
Alexander Kuleshov -
Alexnader Kuleshov
-
Takashi Iwai