Skip to content

Commit

Permalink
mse: core: Fix potential deadlock during audio transmission start
Browse files Browse the repository at this point in the history
refs #326355

mse_work_start_transmission_common() and mse_start_transmission() deal with,
among others, two spin locks: lock_buf_list and lock_state.
It's possible that these two locks will be acquired in opposite orders in two
parallel threads, potentially resulting in a deadlock. Do not try to acquire
lock_state lock while holding lock_buf_list to avoid this situation.

Example command to trigger the warning:
$ speaker-test -r 48000 -t sine -c 2 -F S16_BE -p 16000 -Dhw:ravbmse,1
Reproduction rate: 100%.

LOCKDEP warning:

[   34.776517] mse_packetq/2167 is trying to acquire lock:
[   34.783138]  (&instance->lock_state){....}, at: [<ffff000000b8bbd8>] mse_work_start_transmission_common+0xc8/0x428 [mse_core]
[   34.795864]
[   34.795864] but task is already holding lock:
[   34.801957]  (&(&instance->lock_buf_list)->rlock){....}, at: [<ffff000000b8bb70>] mse_work_start_transmission_common+0x60/0x428 [mse_core]
[   34.814572]
[   34.814572] which lock already depends on the new lock.
[   34.814572]
[   34.822833]
[   34.822833] the existing dependency chain (in reverse order) is:
[   34.830396]
[   34.830396] -> #1 (&(&instance->lock_buf_list)->rlock){....}:
[   34.837722]        _raw_spin_lock_irqsave+0x58/0x98
[   34.842732]        mse_start_transmission+0x178/0x2e8 [mse_core]
[   34.848826]        mse_adapter_alsa_trigger+0x100/0x1cc [mse_adapter_alsa]
[   34.855788]        snd_pcm_do_start+0x28/0x38
[   34.860211]        snd_pcm_action_single+0x44/0x7c
[   34.865071]        snd_pcm_action+0x24/0xf8
[   34.869324]        snd_pcm_start+0x20/0x28
[   34.873488]        __snd_pcm_lib_xfer+0x588/0x624
[   34.878261]        snd_pcm_common_ioctl+0x484/0x1024
[   34.883296]        snd_pcm_ioctl+0x28/0x3c
[   34.887465]        vfs_ioctl+0x24/0x40
[   34.891278]        do_vfs_ioctl+0xa4/0x890
[   34.895439]        SyS_ioctl+0x5c/0xd0
[   34.899257]        el0_svc_naked+0x34/0x38
[   34.903416]
[   34.903416] -> #0 (&instance->lock_state){....}:
[   34.909593]        lock_acquire+0x22c/0x25c
[   34.913842]        _raw_read_lock_irqsave+0x58/0x98
[   34.918871]        mse_work_start_transmission_common+0xc8/0x428 [mse_core]
[   34.925983]        mse_work_start_transmission+0x14c/0x150 [mse_core]
[   34.932508]        kthread_worker_fn+0x110/0x188
[   34.937194]        kthread+0x11c/0x12c
[   34.941010]        ret_from_fork+0x10/0x18
[   34.945170]
[   34.945170] other info that might help us debug this:
[   34.945170]
[   34.953256]  Possible unsafe locking scenario:
[   34.953256]
[   34.959249]        CPU0                    CPU1
[   34.963844]        ----                    ----
[   34.968438]   lock(&(&instance->lock_buf_list)->rlock);
[   34.973733]                                lock(&instance->lock_state);
[   34.980422]                                lock(&(&instance->lock_buf_list)->rlock);
[   34.988248]   lock(&instance->lock_state);
[   34.992408]
[   34.992408]  *** DEADLOCK ***

Signed-off-by: Yan Yankovskyi <[email protected]>
Signed-off-by: Masaru Nagai <[email protected]>
  • Loading branch information
yyankovskyi authored and mnagai committed Jul 26, 2021
1 parent 05127d4 commit e2dff1b
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion mse_core_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -4207,7 +4207,7 @@ static void mse_work_start_transmission_common(struct mse_instance *instance)

return;
} else if (IS_MSE_TYPE_AUDIO(instance->media->type)) {
if (mse_state_test(instance, MSE_STATE_STOPPING)) {
if (mse_state_test_nolock(instance, MSE_STATE_STOPPING)) {
spin_unlock_irqrestore(&instance->lock_buf_list,
flags);
queue_work(instance->wq_packet,
Expand Down

0 comments on commit e2dff1b

Please sign in to comment.