Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755668AbbHYRQE (ORCPT ); Tue, 25 Aug 2015 13:16:04 -0400 Received: from mail-lb0-f171.google.com ([209.85.217.171]:36048 "EHLO mail-lb0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752418AbbHYRQB (ORCPT ); Tue, 25 Aug 2015 13:16:01 -0400 Date: Tue, 25 Aug 2015 23:15:23 +0600 From: Alexnader Kuleshov To: Takashi Iwai Cc: Alexnader Kuleshov , Jaroslav Kysela , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org Subject: Re: ALSA/usb-audio: snd_usb_autoresume possible recursive locking detected Message-ID: <20150825171523.GA1413@localhost> References: <20150825133256.GA1804@localhost>

MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux X-Date: Tue Aug 25 23:11:37 ALMT 2015 User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7777 Lines: 170 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: [] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio] [ 13.422094] but task is already holding lock: [ 13.422094] (&chip->shutdown_rwsem){.+.+.+}, at: [] 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: [] __driver_attach+0x30/0x80 [ 13.422112] #1: (&dev->mutex){......}, at: [] __driver_attach+0x48/0x80 [ 13.422115] #2: (register_mutex#4){+.+.+.}, at: [] usb_audio_probe+0xa3/0x7b9 [snd_usb_audio] [ 13.422120] #3: (&chip->shutdown_rwsem){.+.+.+}, at: [] 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] [] dump_stack+0x4c/0x65 [ 13.422140] [] validate_chain.isra.9+0x75d/0xf59 [ 13.422142] [] ? mark_held_locks+0x5f/0x76 [ 13.422144] [] __lock_acquire+0x753/0xabf [ 13.422146] [] lock_acquire+0x7b/0x9c [ 13.422151] [] ? snd_usb_autoresume+0x1d/0x52 [snd_usb_audio] [ 13.422153] [] down_read+0x44/0x8d [ 13.422156] [] ? snd_usb_autoresume+0x1d/0x52 [snd_usb_audio] [ 13.422160] [] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio] [ 13.422162] [] ? down_read+0x84/0x8d [ 13.422167] [] snd_usb_mixer_set_ctl_value+0xe5/0x1ad [snd_usb_audio] [ 13.422171] [] get_min_max_with_quirks+0x155/0x5e4 [snd_usb_audio] [ 13.422176] [] build_feature_ctl+0x33a/0x419 [snd_usb_audio] [ 13.422181] [] parse_audio_unit+0x459/0xa03 [snd_usb_audio] [ 13.422186] [] snd_usb_mixer_controls+0xe8/0x169 [snd_usb_audio] [ 13.422190] [] snd_usb_create_mixer+0xb4/0x261 [snd_usb_audio] [ 13.422194] [] ? snd_usb_create_stream+0x151/0x164 [snd_usb_audio] [ 13.422197] [] usb_audio_probe+0x700/0x7b9 [snd_usb_audio] [ 13.422199] [] usb_probe_interface+0x139/0x1c3 [ 13.422201] [] driver_probe_device+0xd0/0x21a [ 13.422203] [] __driver_attach+0x5d/0x80 [ 13.422205] [] ? driver_probe_device+0x21a/0x21a [ 13.422207] [] bus_for_each_dev+0x77/0xa5 [ 13.422210] [] driver_attach+0x19/0x1b [ 13.422212] [] bus_add_driver+0xe8/0x1da [ 13.422213] [] driver_register+0x86/0xc3 [ 13.422215] [] usb_register_driver+0xa8/0x146 [ 13.422216] [] ? 0xffffffffa0345000 [ 13.422220] [] usb_audio_driver_init+0x1e/0x20 [snd_usb_audio] [ 13.422222] [] do_one_initcall+0x17f/0x194 [ 13.422225] [] ? __might_sleep+0x71/0x79 [ 13.422228] [] do_init_module+0x56/0x1d7 [ 13.422230] [] load_module+0x17f5/0x1c1e [ 13.422234] [] ? kernel_read+0x4b/0x6d [ 13.422236] [] SyS_finit_module+0x6f/0x90 [ 13.422239] [] 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 > 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: [] snd_usb_autoresume+0x1d/0x52 [snd_usb_audio] > but task is already holding lock: > (&chip->shutdown_rwsem){.+.+.+}, at: [] 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 > Cc: > Signed-off-by: Takashi Iwai > --- > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/