From: Gustavo Padovan <gustavo@padovan.org>
To: Chris Wilson <chris@chris-wilson.co.uk>,
Rob Clark <robdclark@gmail.com>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Subject: Re: [PATCH] dma-buf/fence-array: fix deadlock in fence-array
Date: Mon, 17 Oct 2016 17:44:48 -0200 [thread overview]
Message-ID: <20161017194448.GD11538@joana> (raw)
In-Reply-To: <20161017193938.GB29072@nuc-i3427.alporthouse.com>
2016-10-17 Chris Wilson <chris@chris-wilson.co.uk>:
> On Mon, Oct 17, 2016 at 02:59:52PM -0400, Rob Clark wrote:
> > On Mon, Oct 17, 2016 at 2:52 PM, Gustavo Padovan <gustavo@padovan.org> wrote:
> > > 2016-10-17 Rob Clark <robdclark@gmail.com>:
> > >
> > >> Currently with fence-array, we have a potential deadlock situation. If we
> > >> fence_add_callback() on an array-fence, the array-fence's lock is acquired
> > >> first, and in it's ->enable_signaling() callback, it will install cb's on
> > >> it's array-member fences, so the array-member's lock is acquired second.
> > >>
> > >> But in the signal path, the array-member's lock is acquired first, and the
> > >> array-fence's lock acquired second.
> > >>
> > >> To solve that, always enabling signaling up-front (in the fence_array
> > >> constructor) without the fence_array's lock held.
> > >
> > > Do we always want to enable signaling for arrays? One of the things we
> > > removed from the Sync Framework was the need to enable signalling at
> > > creation time.
> > >
> > > Just merging fencing doesn't mean you want signaling, that is supposed
> > > to happen only when poll() is called on the sync file.
> >
> > It was something Maarten suggested, as an alternative to introducing a
> > wq into the mix or worse hacks..
> > https://lists.freedesktop.org/archives/dri-devel/2016-October/120868.html
> >
> > I think I agree with him that it is an optimization that is unlikely
> > to be useful in the case of fence-arrays. If you need to wait on
> > multiple fences from different timelines, you probably aren't doing
> > that in hw.
>
> For 2 i915 fences, I definitely do not want signaling enabled at
> creation time.
Should we add arg flags for fence_array_create()? We already have
signal_on_any flag there. We can convert that arg to a bitfield.
Gustavo
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2016-10-17 19:44 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-17 18:40 [PATCH] dma-buf/fence-array: fix deadlock in fence-array Rob Clark
2016-10-17 18:52 ` Gustavo Padovan
2016-10-17 18:59 ` Rob Clark
2016-10-17 19:26 ` Gustavo Padovan
2016-10-17 19:39 ` Chris Wilson
2016-10-17 19:44 ` Gustavo Padovan [this message]
2016-10-18 7:41 ` Daniel Vetter
2016-10-18 11:49 ` Rob Clark
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161017194448.GD11538@joana \
--to=gustavo@padovan.org \
--cc=chris@chris-wilson.co.uk \
--cc=dri-devel@lists.freedesktop.org \
--cc=gustavo.padovan@collabora.co.uk \
--cc=robdclark@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox