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

[Chris Wilson]

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.

>         struct intel_gt *gt = engine->gt;
>         int err;
>  
> @@ -626,6 +626,7 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>         GEM_BUG_ON(!mask);
>         for_each_engine_masked(engine, gt, mask, tmp) {
>                 u64 idx = engine->kernel_context->timeline->fence_context;
> +               struct llist_node *prev = first;
>                 struct active_node *node;
>  
>                 node = reuse_idle_barrier(ref, idx);
> @@ -659,23 +660,23 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
>                 GEM_BUG_ON(rcu_access_pointer(node->base.fence) != ERR_PTR(-EAGAIN));
>  
>                 GEM_BUG_ON(barrier_to_engine(node) != engine);
> -               next = barrier_to_ll(node);
> -               next->next = pos;
> -               if (!pos)
> -                       pos = next;

Yeah, that was a bit of nonsense.

Minus the spurious NULL,
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris