Re: [PATCH] drm/i915/gtt: Handle double alloc failures

[Matthew Auld <matthew.william.auld@xxxxxxxxx>]

On Thu, 4 Jul 2019 at 11:43, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Matthew Auld <matthew.william.auld@xxxxxxxxx>

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@xxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx