On Thu, Apr 02, 2015 at 05:11:58PM +0100, Tvrtko Ursulin wrote: > >+static struct drm_i915_gem_object * > >+find_object_from_vma(struct drm_device *dev, > >+ struct drm_i915_gem_userptr *args) > >+{ > >+ struct drm_i915_gem_object *obj = NULL; > >+ struct vm_area_struct *vma; > >+ > >+ down_read(¤t->mm->mmap_sem); > >+ vma = find_vma(current->mm, args->user_ptr); > >+ if (vma == NULL) > >+ goto out; > >+ > >+ if (vma->vm_ops != dev->driver->gem_vm_ops) > >+ goto out; > >+ > >+ if (vma->vm_start != args->user_ptr || > >+ vma->vm_end != args->user_ptr + args->user_size) { > >+ obj = ERR_PTR(-EINVAL); > >+ goto out; > >+ } > >+ > >+ obj = to_intel_bo(vma->vm_private_data); > >+ drm_gem_object_reference(obj); > > Hm, can't this race with last unreference in general, and with > cleanup worker with userptr objects?
The vma holds a reference to the object and that reference is dropped whilst holding down_write(current->mm->mmap_sem), hence I think the down_read(current->mm->mmap_sem) is sufficient locking to acquire a reference for ourselves. > >+out: ret = drm_gem_handle_create(file, &obj->base, &handle); > > > > /* drop reference from allocate - handle holds it now */ > > drm_gem_object_unreference_unlocked(&obj->base); > > Thing I don't like is how the user of this has no idea what kind of > object it "imported". Maybe it doesn't matter, hm. Need to think > about it more. Indeed. But since the userptr is a strict subset of the general bo, if they follow the rules for userptr bo then they won't notice a difference. read/writes into the memory block are coherent (since the pointer is wc) so as far the caller is concerned I think it just ends up being slower cpu side, faster gpu side than a system memory snooped userptr bo. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx