[Intel-gfx] [PATCH 01/12] drm/i915: Remove kref from i915_sw_fence

[Chris Wilson]

On Fri, May 12, 2017 at 10:43:31AM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > My original intention was for i915_sw_fence to be the base class and
> > provide the reference count for the container. This was from starting
> > with a design to handle async_work. In practice, for i915 we embed
> > fences into structs which have their own independent reference counting,
> > making the i915_sw_fence.kref duplicitous. If we remove the kref, we
> > remove the i915_sw_fence's ability to free itself and its independence,
> > it can only exist within a container and must be supplied with a
> > callback to handle its release.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_sw_fence.c | 55 ++++++++----------------------------
> >  drivers/gpu/drm/i915/i915_sw_fence.h |  1 -
> >  2 files changed, 11 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c b/drivers/gpu/drm/i915/i915_sw_fence.c
> > index a277f8eb7beb..a0a690d6627e 100644
> > --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> > +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> > @@ -120,34 +120,6 @@ void i915_sw_fence_fini(struct i915_sw_fence *fence)
> >  }
> >  #endif
> >  
> > -static void i915_sw_fence_release(struct kref *kref)
> > -{
> > -	struct i915_sw_fence *fence = container_of(kref, typeof(*fence), kref);
> > -
> > -	WARN_ON(atomic_read(&fence->pending) > 0);
> > -	debug_fence_destroy(fence);
> > -
> > -	if (fence->flags & I915_SW_FENCE_MASK) {
> > -		__i915_sw_fence_notify(fence, FENCE_FREE);
> 
> Is the current design so that the callback equals embeddding?

To embed, in 99.999% of cases you really do have to employ the callback
to resolve lifetime. Yes, it is possible by explicitly calling wait
that you do know its state; and that is useful for on-stack fences.
However, the current usage of debugobjects negects that (on-stack
objects are banned), so we do a need constructor for
i915_sw_fence_init_onstack(), so mandating a callback simplifies
everyone's lives.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre