Re: [PATCH 2/2] dma-buf/fence-array: hold fences reference when creating an array

[Gustavo Padovan <gustavo@padovan.org>]

2016-10-20 Christian König <deathsimple@vodafone.de>:

> Am 19.10.2016 um 20:35 schrieb Gustavo Padovan:
> > 2016-10-19 Christian König <deathsimple@vodafone.de>:
> > 
> > > Am 19.10.2016 um 19:48 schrieb Gustavo Padovan:
> > > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > > > 
> > > > When creating fence arrays we were not holding references to the fences
> > > > in the array, however when destroy the array we were putting away a
> > > > reference to these fences.
> > > > 
> > > > This patch hold the ref for all fences in the array when creating the
> > > > array.
> > > > 
> > > > It then removes the code that was holding the fences on both amdgpu_vm and
> > > > sync_file. For sync_file, specially, we worked on small referencing
> > > > refactor for sync_file_merge().
> > > > 
> > > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > > I would prefer it to keep it like it is, cause this is a bit inconsistent.
> > > 
> > > With this change the fence array takes the ownership of the array, but not
> > > of the fences inside it.
> > I was thinking more in to keep consistency between all fence users. Every
> > user should hold a ref to the fence assigned to it. That is what patch
> > 1 is doing for sync_file and think it is a good idea do the same here.
> 
> This might make the code easier to follow, but isn't necessary a good idea.
> 
> Usually with reference counted objects you increase the count every time the
> pointer to the object is assigned to a container. E.g. member of a larger
> structure or in this case an array of pointers.
> 
> > 
> > The array itself is not refcounted and the users calling
> > fence_array_create() doesn't store the allocated array anywhere. The
> > comment I errouneously removed already states that.
> 
> And exactly that's the point here. The array is the container for the
> pointers referencing the objects, since you give the ownership of this
> container to the fence_array object it is now responsible for releasing that
> reference before it releases the array.
> 
> This is good coding practice as far as I know.

Right, this makes sense. Let's keep this as is then.

Gustavo

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel