[Intel-gfx] [PATCH i-g-t] i915/gem_vm_create: Race vm-destroy against object free

[Chris Wilson]

Quoting Matthew Auld (2020-12-14 15:52:06)
> On 08/12/2020 17:02, Chris Wilson wrote:
> > Matthew postulated that we should be able to hit a race in
> > __i915_vm_close() between the RCU object free and vma unbind viz
> > 
> >    GEM_BUG_ON(!list_empty(&vm->bound_list));
> > 
> > due to the effect of leaving the vma on the list if we are unable to
> > obtain the kref to the object. Let's try and find that race.
> 
> Hmm, what's the idea behind the bound_list stuff in __i915_vm_close(), 
> from the looks of it vm->open is always > 0 anyway for the lifetime of 
> the vm(?), so not sure if it's even possible to hit that path, at least 
> for direct userspace interactions.

>From userspace, the intent was to track vm->open. i.e. we could not
close the whole vm as it was being used by execbuf. With the individual
vma holding a reference to the vm to prevent it being freed while still
active on the GPU.

> I guess I was half expecting the 
> vm_destroy ioctl or similar, to also call i915_vm_close() at some point, 
> to match vm_create, and not just drop the vm ref.

Right, each user vm_id is only a reference to a vm, and the user may
have multiple vm_id to the same vm. So there's an ambiguity that
prevents us from immediately closing the vm on destroy, and so just
manage references instead.

> So looks like 
> __i915_vm_close() is only potentially interesting for kernel internal users?

Hmm. There's a call to i915_vm_close from context_close (and on
changing the context vm). And there definitely can be vma still bound at
that point, and those vma still linked into the obj->vma_list. So I
think the pruning is still relevant for GEM contexts.
-Chris