Re: [RFC 03/10] drm/i915/vm_bind: Support private and shared BOs

[Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>]

On Fri, Jul 08, 2022 at 07:44:54AM -0700, Hellstrom, Thomas wrote:
>On Fri, 2022-07-08 at 15:43 +0200, Thomas Hellström wrote:
>> > The vm_bind/bound_list and the non_priv_vm_bind_list are there for
>> > very different reasons.
>> >
>> > The reason for having separate vm_bind_list and vm_bound_list is
>> > that
>> > during the execbuf path, we can rebind the unbound mappings by
>> > scooping
>> > all unbound vmas back from bound list into the bind list and
>> > binding
>> > them. In fact, this probably can be done with a single vm_bind_list
>> > and
>> > a 'eb.bind_list' (local to execbuf3 ioctl) for rebinding.
>> >
>> > The non_priv_vm_bind_list is just an optimization to loop only
>> > through
>> > non-priv objects while taking the locks in
>> > eb_lock_persistent_vmas()
>> > as only non-priv objects needs that (private objects are locked in
>> > a
>> > single shot with vm_priv_lock). A non-priv mapping will also be in
>> > the
>> > vm_bind/bound_list.
>> >
>> > I think, we need to add this as documentation to be more clear.
>>
>> OK, I understood it as private objects were either on the vm_bind
>> list
>> or vm_bound_list depending on whether they needed rebinding or not,
>> and
>> shared objects only on the non_priv_vm_bind list, and were always
>> locked, validated and fenced...
>>
>> Need to take a deeper look...
>>
>> /Thomas
>>
>>
>>
>> >
>> > Niranjana
>> >
>> >
>
>Hmm. Just a quick thought on this, Since the non-private vm-bind
>objects all need to be iterated through (locked and fenced and userptr
>valid) on each execbuf, and checking for validation (resident and
>bound) is a very quick check, then we'd never need to add them to the
>rebind list at all, right? If so the rebind list would be exclusive to
>vm-private objects.

Yah, non-private vm-bind objects all need to be iterated through, locked
and fenced (not required to be validated/userptr validated unless there
is an invalidation).

Yah, we can say that it is a quick check to see if BO needs rebinding
(validated), and hence rebind_list is mainly useful only for vm-private
objects.
But there has been some discussions to optimize the non-private BOs case
by not having to iterate. I am not sure how that will shape up, but
something to consider here.

>
>Also I don't think the vm_bind list can be execbuf-local, since binding
>may not have completed at vma_release time, at which point the objects
>need to remain on the vm_bind list until the next execbuf...

Yah, in execbuf3, after scooping all unbind BOs in vm_bind_list and
validating (binding) them, during eb_release_vmas, we only move those
BOs to vm_bound_list for which the binding is complete. If not, they
will remain in vm_bind_list. If we make this list, execbuf-local, then
we have to add all the BOs for which binding is not complete back into
rebind_list so that next execbuf will pick it up. Probably the current
vm_bind/bound_list is the better option.

Niranjana

>
>/Thomas
>
>