Re: sound: deadlock involving snd_hrtimer_callback

From	Dmitry Vyukov <dvyukov@google.com>
Newsgroups	linux.kernel
Subject	Re: sound: deadlock involving snd_hrtimer_callback
Date	2016-04-24 19:20 +0200
Message-ID	<rrvRn-8ar-5@gated-at.bofh.it> (permalink)
References	<rr66B-3Ns-9@gated-at.bofh.it> <rrcYq-1lD-51@gated-at.bofh.it> <rrtZf-6Hu-5@gated-at.bofh.it> <rruVl-7re-27@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Sun, Apr 24, 2016 at 6:16 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Sun, 24 Apr 2016 17:16:32 +0200,
> Dmitry Vyukov wrote:
>>
>> On Sat, Apr 23, 2016 at 11:02 PM, Takashi Iwai <tiwai@suse.de> wrote:
>> > On Sat, 23 Apr 2016 15:40:21 +0200,
>> > Dmitry Vyukov wrote:
>> >>
>> >> Hi Takashi,
>> >>
>> >> I've incorporated your hrtimer fixes (but also updated to
>> >> ddce192106e4f984123884f8e878f66ace94b573) and now I am seeing lots of
>> >> the following deadlock messages:
>> >>
>> >>
>> >> [ INFO: possible circular locking dependency detected ]
>> >> 4.6.0-rc4+ #351 Not tainted
>> >> -------------------------------------------------------
>> >> swapper/0/0 is trying to acquire lock:
>> >>  (&(&timer->lock)->rlock){-.-...}, at: [<ffffffff8537a749>]
>> >> snd_timer_interrupt+0xa9/0xd30 sound/core/timer.c:701
>> >>
>> >> but task is already holding lock:
>> >>  (&(&stime->lock)->rlock){-.....}, at: [<ffffffff85383d3f>]
>> >> snd_hrtimer_callback+0x4f/0x2b0 sound/core/hrtimer.c:54
>> >>
>> >> which lock already depends on the new lock.
>> >
>> > Oh crap, my second patch is buggy, it leads to ABBA lock, indeed.
>> > The first patch is still OK, as it just adds a new behavior mode.
>> >
>> > Could you replace the second patch with the below one?
>>
>>
>> I've replaced the second path with this one. The deadlocks has gone,
>> but I've hit these two hangs that look related:
>>
>> https://gist.githubusercontent.com/dvyukov/805718ea249c49d17ae759d1b0160684/raw/20891f7e87fe9af3967565559d465d296469244b/gistfile1.txt
>> https://gist.githubusercontent.com/dvyukov/7f397ea4aeb9e35596e0c8053cf35a11/raw/3fc22f24f7bab5941e47bab604f96487b5f1944d/gistfile1.txt
>
> Hmm, so it wasn't a good idea to call hrtimer_cancel() in the
> spinlock, in anyway.  Scratch the previous one.
>
> OK, below is the yet revised two patches.  One is the simplified
> version of the patch, and another is to call hrtimer_cancel() in a new
> timer op without spinlock.  Apply these after the first patch
> "ALSA: timer: Allow backend disabling start/stop from handler".

Done. I will let you know if I see any failures.

Thread

sound: deadlock involving snd_hrtimer_callback Dmitry Vyukov <dvyukov@google.com> - 2016-04-23 15:50 +0200
  Re: sound: deadlock involving snd_hrtimer_callback Takashi Iwai <tiwai@suse.de> - 2016-04-23 23:10 +0200
    Re: sound: deadlock involving snd_hrtimer_callback Dmitry Vyukov <dvyukov@google.com> - 2016-04-24 17:20 +0200
      Re: sound: deadlock involving snd_hrtimer_callback Dmitry Vyukov <dvyukov@google.com> - 2016-04-24 17:30 +0200
      Re: sound: deadlock involving snd_hrtimer_callback Takashi Iwai <tiwai@suse.de> - 2016-04-24 18:20 +0200
        Re: sound: deadlock involving snd_hrtimer_callback Dmitry Vyukov <dvyukov@google.com> - 2016-04-24 19:20 +0200
          Re: sound: deadlock involving snd_hrtimer_callback Takashi Iwai <tiwai@suse.de> - 2016-04-24 23:40 +0200
            Re: sound: deadlock involving snd_hrtimer_callback Dmitry Vyukov <dvyukov@google.com> - 2016-04-25 10:10 +0200
              Re: sound: deadlock involving snd_hrtimer_callback Takashi Iwai <tiwai@suse.de> - 2016-04-25 10:40 +0200

csiph-web