Quoting Tvrtko Ursulin (2019-04-18 10:50:30)
>
> On 18/04/2019 10:13, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-04-18 09:57:43)
> >>
> >> On 18/04/2019 07:57, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2019-04-18 07:47:51)
> >>>>
> >>>> On 17/04/2019 08:56, Chris Wilson wrote:
> >>>>> +static void
> >>>>> +virtual_bond_execute(struct i915_request *rq, struct dma_fence *signal)
> >>>>> +{
> >>>>> + struct virtual_engine *ve = to_virtual_engine(rq->engine);
> >>>>> + struct ve_bond *bond;
> >>>>> +
> >>>>> + bond = virtual_find_bond(ve, to_request(signal)->engine);
> >>>>> + if (bond) {
> >>>>> + intel_engine_mask_t old, new, cmp;
> >>>>> +
> >>>>> + cmp = READ_ONCE(rq->execution_mask);
> >>>>> + do {
> >>>>> + old = cmp;
> >>>>> + new = cmp & bond->sibling_mask;
> >>>>> + } while ((cmp = cmpxchg(&rq->execution_mask, old, new)) != old);
> >>>>
> >>>> Loop implies someone else might be modifying the rq->execution_mask in
> >>>> parallel?
> >>>
> >>> There's nothing that prevents there being multiple bonds being
> >>> executed simultaneously (other than practicality). There's also nothing
> >>> that says this should be the only way to modify rq->execution_mask in
> >>> the future.
> >>
> >> But request is one, how can it be submitted multiple times simultaneously?
> >
> > You mean "How can it be signaled multiple times simultaneously?"
>
> Okay yes, signaled. You could give same submit fence to multiple slaves,
> but you can't have same slave request receive notification from multiple
> masters.
>
> Or you can if you build a composite fence and pass that in? Is this the
> story about signal-on-any vs signal-on-all?
There's nothing inherent in the design to prevent virtual_bond_execute
being called multiple times given multiple fences along one or more
engines.
There's a practical limitation in the proposed uAPI to limit it to a
single submit-fence, but may indeed be a composite fence. There's also
the question of whether to squeeze in syncobj support.
> >>>>> +
> >>>>> + err = check_user_mbz(&ext->flags);
> >>>>> + if (err)
> >>>>> + return err;
> >>>>> +
> >>>>> + for (n = 0; n < ARRAY_SIZE(ext->mbz64); n++) {
> >>>>> + err = check_user_mbz(&ext->mbz64[n]);
> >>>>> + if (err)
> >>>>> + return err;
> >>>>> + }
> >>>>> +
> >>>>> + if (get_user(class, &ext->master_class))
> >>>>> + return -EFAULT;
> >>>>> +
> >>>>> + if (get_user(instance, &ext->master_instance))
> >>>>> + return -EFAULT;
> >>>>> +
> >>>>> + master = intel_engine_lookup_user(set->ctx->i915, class, instance);
> >>>>> + if (!master) {
> >>>>> + DRM_DEBUG("Unrecognised master engine: { class:%d, instance:%d }\n",
> >>>>> + class, instance);
> >>>>> + return -EINVAL;
> >>>>> + }
> >>>>> +
> >>>>> + if (get_user(num_bonds, &ext->num_bonds))
> >>>>> + return -EFAULT;
> >>>>
> >>>> Should num_bonds > virtual->num_siblings be an error?
> >>>
> >>> They could specify the same bond multiple times for whatever reason (and
> >>> probably should allow skipping NONE?), if the target doesn't exist that's
> >>> definitely an error.
> >>
> >> So which bond we pick if they specify multiple ones? Just the first one
> >> found. Hm actually I was thinking about making sure each master is only
> >> specified once, not siblings. For siblings we indeed do not care.
> >
> > No, it's a mask of if parent executes on master, use this set of
> > children.
> >
> > I was reasonably happy to use a cumulative mask if master is specified
> > by more than one bond ext; but maybe it should be an intersection. Hmm.
>
> Do you see a realistic and making sense use case for specifying the same
> master in multiple bonds? If not I'd just disallow it and then we don't
> have a question of union vs intersection policy.
Rather the opposite, I don't see that it breaks anything nor need it be
ill-defined, hence no reason to reject as a means to protect oneself.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
