On Tue, 2021-11-30 at 16:02 +0100, Christian König wrote: > Am 30.11.21 um 15:35 schrieb Thomas Hellström: > > On Tue, 2021-11-30 at 14:26 +0100, Christian König wrote: > > > 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. > > Indeed, that would require some deeper surgery. > > > > > 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. > > Yes, this is actually the use-case. But what I can't easily > > guarantee > > is that that dma_fence_chain isn't fed into a dma_fence_array > > somewhere > > else. How do you typically avoid that? > > > > Meanwhile I guess I need to take a different approach in the driver > > to > > avoid this altogether. > > Jason and I came up with a deep dive iterator for his use case, but I > think we don't want to use that any more after my dma_resv rework. > > In other words when you need to create a new dma_fence_array you > flatten > out the existing construct which is at worst case > dma_fence_chain->dma_fence_array->dma_fence. Ok, Are there any cross-driver contract here, Like every driver using a dma_fence_array need to check for dma_fence_chain and flatten like above? /Thomas > > Regards, > Christian. > > > > > /Thomas > > > > > > > Regards, > > > Christian. > > > > > > > /Thomas > > > > > > > > > > > > > > > > > > > > > Regards, > > > > > Christian. > > > > > > > > > > > But I'll update the commit message with a typical splat. > > > > > > > > > > > > /Thomas > > >
