Re: [Linaro-mm-sig] Re: [PATCH] dma-fence: allow dma fence to have their own lock

[Christian König]

Am 01.06.22 um 16:52 schrieb Sergey Senozhatsky:
> On (22/06/01 16:38), Christian König wrote:
> > > > Well, you don't.
> > > >
> > > > If you have a dynamic context structure you need to reference count that as
> > > > well. In other words every time you create a fence in your context you need
> > > > to increment the reference count and every time a fence is release you
> > > > decrement it.
> > > OK then fence release should be able to point back to its "context"
> > > structure. Either a "private" data in dma fence or we need to "embed"
> > > fence into another object (refcounted) that owns the lock and provide
> > > dma fence ops->release callback, which can container_of() to the object
> > > that dma fence is embedded into.
> > >
> > > I think you are suggesting the latter. Thanks for clarifications.
> > Daniel might hurt me for this, but if you really only need a pointer to your
> > context then we could say that using a pointer value for the context field
> > is ok as well.
> >
> > That should be fine as well as long as you can guarantee that it will be
> > unique during the lifetime of all it's fences.
> I think we can guarantee that. Object that creates fence is kmalloc-ed and
> it sticks around until dma_fence_release() calls ops->release() and kfree-s
> it. We *probably* can even do something like it now, by re-purposing dma_fence
> context member:
>
>          dma_fence_init(obj->fence,
>                         &fence_ops,
>                         &obj->fence_lock,
>                         (u64)obj,                             <<   :/
>                         atomic64_inc_return(&obj->seqno));
>
> I'd certainly refrain from being creative here and doing things that
> are not documented/common. DMA fence embedding should work for us.

Yeah, exactly that's the idea. But if you are fine to create a subclass 
of the dma_fence than that would indeed be cleaner.

Christian.

> > > The limiting factor of this approach is that now our ops->release() is
> > > under the same "pressure" as dma_fence_put()->dma_fence_release() are.
> > > dma_fence_put() and dma_fence_release() can be called from any context,
> > > as far as I understand, e.g. IRQ, however our normal object ->release
> > > can schedule, we do things like synchronize_rcu() and so on. Nothing is
> > > impossible, just saying that even this approach is not 100% perfect and
> > > may need additional workarounds.
> > Well just use a work item for release.
> Yup, that's the plan.