Quoting Matthew Auld (2019-07-04 13:27:07)
> 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>
I'm sure Mika is dying to rewrite these anyway, we can see what
solution he comes up with. :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
