From: "Christian König" <christian.koenig@amd.com>
To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc: linaro-mm-sig@lists.linaro.org, matthew.auld@intel.com
Subject: Re: [RFC PATCH 1/2] dma-fence: Avoid establishing a locking order between fence classes
Date: Tue, 30 Nov 2021 14:26:20 +0100 [thread overview]
Message-ID: <2551da4d-2e51-cc24-7d4a-84ae00a1547c@amd.com> (raw)
In-Reply-To: <57df8b0b-1d65-155f-a9a6-8073bbd4f28f@linux.intel.com>
Am 30.11.21 um 13:56 schrieb Thomas Hellström:
>
> On 11/30/21 13:42, Christian König wrote:
>> Am 30.11.21 um 13:31 schrieb Thomas Hellström:
>>> [SNIP]
>>>> Other than that, I didn't investigate the nesting fails enough to
>>>> say I can accurately review this. :)
>>>
>>> Basically the problem is that within enable_signaling() which is
>>> called with the dma_fence lock held, we take the dma_fence lock of
>>> another fence. If that other fence is a dma_fence_array, or a
>>> dma_fence_chain which in turn tries to lock a dma_fence_array we hit
>>> a splat.
>>
>> Yeah, I already thought that you constructed something like that.
>>
>> You get the splat because what you do here is illegal, you can't mix
>> dma_fence_array and dma_fence_chain like this or you can end up in a
>> stack corruption.
>
> Hmm. Ok, so what is the stack corruption, is it that the
> enable_signaling() will end up with endless recursion? If so, wouldn't
> it be more usable we break that recursion chain and allow a more
> general use?
The problem is that this is not easily possible for dma_fence_array
containers. Just imagine that you drop the last reference to the
containing fences during dma_fence_array destruction if any of the
contained fences is another container you can easily run into recursion
and with that stack corruption.
That's one of the major reasons I came up with the dma_fence_chain
container. This one you can chain any number of elements together
without running into any recursion.
> Also what are the mixing rules between these? Never use a
> dma-fence-chain as one of the array fences and never use a
> dma-fence-array as a dma-fence-chain fence?
You can't add any other container to a dma_fence_array, neither other
dma_fence_array instances nor dma_fence_chain instances.
IIRC at least technically a dma_fence_chain can contain a
dma_fence_array if you absolutely need that, but Daniel, Jason and I
already had the same discussion a while back and came to the conclusion
to avoid that as well if possible.
Regards,
Christian.
>
> /Thomas
>
>
>
>
>>
>> Regards,
>> Christian.
>>
>>>
>>> But I'll update the commit message with a typical splat.
>>>
>>> /Thomas
>>
next prev parent reply other threads:[~2021-11-30 13:26 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-30 12:19 [RFC PATCH 0/2] Attempt to avoid dma-fence-[chain|array] lockdep splats Thomas Hellström
2021-11-30 12:19 ` [RFC PATCH 1/2] dma-fence: Avoid establishing a locking order between fence classes Thomas Hellström
2021-11-30 12:25 ` Maarten Lankhorst
2021-11-30 12:31 ` Thomas Hellström
2021-11-30 12:42 ` Christian König
2021-11-30 12:56 ` Thomas Hellström
2021-11-30 13:26 ` Christian König [this message]
2021-11-30 14:35 ` Thomas Hellström
2021-11-30 15:02 ` Christian König
2021-11-30 18:12 ` Thomas Hellström
2021-11-30 19:27 ` Thomas Hellström
2021-12-01 7:05 ` Christian König
2021-12-01 8:23 ` [Linaro-mm-sig] " Thomas Hellström (Intel)
2021-12-01 8:36 ` Christian König
2021-12-01 10:15 ` Thomas Hellström (Intel)
2021-12-01 10:32 ` Christian König
2021-12-01 11:04 ` Thomas Hellström (Intel)
2021-12-01 11:25 ` Christian König
2021-12-01 12:16 ` Thomas Hellström (Intel)
2021-12-03 13:08 ` Christian König
2021-12-03 14:18 ` Thomas Hellström
2021-12-03 14:26 ` Christian König
2021-12-03 14:50 ` Thomas Hellström
2021-12-03 15:00 ` Christian König
2021-12-03 15:13 ` Thomas Hellström (Intel)
2021-12-07 18:08 ` [Intel-gfx] " Daniel Vetter
2021-12-07 20:46 ` Thomas Hellström
2021-12-20 9:37 ` Daniel Vetter
2021-11-30 12:32 ` Thomas Hellström
2021-11-30 12:19 ` [RFC PATCH 2/2] dma-fence: Avoid excessive recursive fence locking from enable_signaling() callbacks Thomas Hellström
2021-11-30 12:36 ` [RFC PATCH 0/2] Attempt to avoid dma-fence-[chain|array] lockdep splats Christian König
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=2551da4d-2e51-cc24-7d4a-84ae00a1547c@amd.com \
--to=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=linaro-mm-sig@lists.linaro.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=matthew.auld@intel.com \
--cc=thomas.hellstrom@linux.intel.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;
as well as URLs for NNTP newsgroup(s).