Quoting Tvrtko Ursulin (2019-03-14 16:18:17)
>
> On 13/03/2019 14:43, Chris Wilson wrote:
> > + if (local.flags & I915_CONTEXT_CLONE_VM) {
> > + struct i915_hw_ppgtt *ppgtt;
> > +
> > + do {
> > + ppgtt = READ_ONCE(src->ppgtt);
> > + if (!ppgtt)
> > + break;
> > +
> > + if (!kref_get_unless_zero(&ppgtt->ref))
> > + continue;
> > +
> > + if (ppgtt == READ_ONCE(src->ppgtt))
> > + break;
>
> This loop needs a comment. For instance I don't understand what is this
> line for. Firstly, we have a reference on ppgtt, so what do we care if
> the source context in the meantime changed it? Secondly, even though it
> matches now, why would it still match when we exit the loop?
Because it may have been reallocated between the read and the kref. Once
we have the kref, and passed through the strong memory barrier, can we
decide if this is the ppgtt that we wanted. It may indeed be released
from src->ppgtt after this point, but it can no longer be reused
elsewhere, so we can not inadvertently share the ppgtt from a third
party.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
