On Tue, Jan 04, 2022 at 04:08:20PM +0100, Christian König wrote:
> Am 22.12.21 um 22:43 schrieb Daniel Vetter:
> > On Tue, Dec 07, 2021 at 01:34:00PM +0100, Christian König wrote:
> > > So far we had the approach of using a directed acyclic
> > > graph with the dma_resv obj.
> > >
> > > This turned out to have many downsides, especially it means
> > > that every single driver and user of this interface needs
> > > to be aware of this restriction when adding fences. If the
> > > rules for the DAG are not followed then we end up with
> > > potential hard to debug memory corruption, information
> > > leaks or even elephant big security holes because we allow
> > > userspace to access freed up memory.
> > >
> > > Since we already took a step back from that by always
> > > looking at all fences we now go a step further and stop
> > > dropping the shared fences when a new exclusive one is
> > > added.
> > >
> > > Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> > > ---
> > > drivers/dma-buf/dma-resv.c | 13 -------------
> > > 1 file changed, 13 deletions(-)
> > >
> > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> > > index 9acceabc9399..ecb2ff606bac 100644
> > > --- a/drivers/dma-buf/dma-resv.c
> > > +++ b/drivers/dma-buf/dma-resv.c
> > No doc update at all!
>
> Scratching my head I'm pretty sure I've updated at least the kerneldoc for
> dma_resv_add_excl_fence(). Must have gone lost in some rebase.
>
> >
> > I checked, we're not that shitty with docs,
>
> Well I wouldn't say shitty, but they are not perfect either.
This was sarcasm, I meant to say that despite the struggles the docs
in-tree are pretty good nowadays. Email just sucks sometimes for
communication.
> > Minimally the DOC: section
> > header and also the struct dma_resv kerneldoc. Also there's maybe more
> > references and stuff I've missed on a quick look, please check for them
> > (e.g. dma_buf.resv kerneldoc is rather important to keep correct too).
> >
> > Code itself does what it says in the commit message, but we really need
> > the most accurate docs we can get for this stuff, or the confusion will
> > persist :-/
>
> Yeah completely agree, going to fix that.
Awesome!
Cheers, Daniel
>
> Thanks,
> Christian.
>
> >
> > Cheers, Daniel
> >
> > > @@ -383,29 +383,16 @@ EXPORT_SYMBOL(dma_resv_replace_fences);
> > > void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence)
> > > {
> > > struct dma_fence *old_fence = dma_resv_excl_fence(obj);
> > > - struct dma_resv_list *old;
> > > - u32 i = 0;
> > > dma_resv_assert_held(obj);
> > > - old = dma_resv_shared_list(obj);
> > > - if (old)
> > > - i = old->shared_count;
> > > -
> > > dma_fence_get(fence);
> > > write_seqcount_begin(&obj->seq);
> > > /* write_seqcount_begin provides the necessary memory barrier */
> > > RCU_INIT_POINTER(obj->fence_excl, fence);
> > > - if (old)
> > > - old->shared_count = 0;
> > > write_seqcount_end(&obj->seq);
> > > - /* inplace update, no shared fences */
> > > - while (i--)
> > > - dma_fence_put(rcu_dereference_protected(old->shared[i],
> > > - dma_resv_held(obj)));
> > > -
> > > dma_fence_put(old_fence);
> > > }
> > > EXPORT_SYMBOL(dma_resv_add_excl_fence);
> > > --
> > > 2.25.1
> > >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
