[Intel-gfx] [PATCH] drm/i915/gtt: Handle double alloc failures

[Chris Wilson]

Quoting Matthew Auld (2019-07-04 13:27:07)
> On Thu, 4 Jul 2019 at 11:43, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >
> > Matthew pointed out that we could face a double failure with concurrent
> > allocations/frees, and so the assumption that the local var alloc was
> > NULL was fraught with danger. Rather than complicate the error paths too
> > much to add a second local for a second free, just do the second free
> > earlier on the unwind path.
> >
> > Reported-by: Matthew Auld <matthew.william.auld at gmail.com>
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.william.auld at gmail.com>
> 
> I quite liked your previous pattern:
> 
> @@ -1442,6 +1442,7 @@ static int gen8_ppgtt_alloc_pdp(struct
> i915_address_space *vm,
>  {
>         struct i915_page_directory *pd, *alloc = NULL;
>         u64 from = start;
> +       bool free = false;
>         unsigned int pdpe;
>         int ret = 0;
> 
> @@ -1489,10 +1490,11 @@ static int gen8_ppgtt_alloc_pdp(struct
> i915_address_space *vm,
>                 gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
>                 GEM_BUG_ON(!atomic_read(&pdp->used));
>                 atomic_dec(&pdp->used);
> -               GEM_BUG_ON(alloc);
> -               alloc = pd; /* defer the free to after the lock */
> +               free = true;
>         }
>         spin_unlock(&pdp->lock);
> +       if (free)
> +               free_pd(vm, pd);
>  unwind:
>         gen8_ppgtt_clear_pdp(vm, pdp, from, start - from);
>  out:
> 
> Anyway,
> Reviewed-by: Matthew Auld <matthew.william.auld at gmail.com>

I'm sure Mika is dying to rewrite these anyway, we can see what
solution he comes up with. :)
-Chris