[Intel-gfx] [PATCH] drm/i915: Fix preallocated barrier list append

[Souza, Jose]

On Wed, 2020-01-29 at 23:59 +0000, Chris Wilson wrote:
> Quoting Souza, Jose (2020-01-29 23:53:21)
> > On Wed, 2020-01-29 at 23:33 +0000, Chris Wilson wrote:
> > > Quoting José Roberto de Souza (2020-01-29 23:23:45)
> > > > Only the first and the last nodes were being added to
> > > > ref->preallocated_barriers.
> > > > 
> > > > I'm not familiar with this part of the code but if that is the
> > > > expected behavior it is leaking the nodes in between.
> > > > 
> > > > Renaming variables to make it more easy to read.
> > > > 
> > > > Fixes: 841350223816 ("drm/i915/gt: Drop mutex serialisation
> > > > between
> > > > context pin/unpin")
> > > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_active.c | 19 ++++++++++---------
> > > >  1 file changed, 10 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_active.c
> > > > b/drivers/gpu/drm/i915/i915_active.c
> > > > index 9d6830885d2e..3d2e7cf55e52 100644
> > > > --- a/drivers/gpu/drm/i915/i915_active.c
> > > > +++ b/drivers/gpu/drm/i915/i915_active.c
> > > > @@ -607,7 +607,7 @@ int
> > > > i915_active_acquire_preallocate_barrier(struct i915_active
> > > > *ref,
> > > >                                             struct
> > > > intel_engine_cs
> > > > *engine)
> > > >  {
> > > >         intel_engine_mask_t tmp, mask = engine->mask;
> > > > -       struct llist_node *pos = NULL, *next;
> > > > +       struct llist_node *first = NULL, *last = NULL;
> > > 
> > > last cannot be NULL at the end.
> > 
> > last will be set in the first iteration and it will always interate
> > at
> > least once because the mask will at least match with the engine in
> > i915_active_acquire_preallocate_barrier() parameter.
> 
> I meant to say last cannot be unset at the end, so initialising it is
> silly.

It need to be initialized so it is set in the first iteration to point
to the last node.

> -Chris