Re: [PATCH] drm/i915: Convert WARNs during userptr revoke to SIGBUS

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 09/23/2015 09:07 PM, Chris Wilson wrote:
> If the client revokes the virtual address it asked to be mapped into GPU
> space via userptr (by using anything along the lines of mmap, mprotect,
> madvise, munmap, ftruncate etc) the mmu notifier sends a range
> invalidate command to userptr. Upon receiving the invalidation signal
> for the revoked range, we try to release the struct pages we pinned into
> the GTT. However, this can fail if any of the GPU's VMA are pinned for
> use by the hardware (i.e. despite the user's intention we cannot
> relinquish the client's address range and keep uptodate with whatever is
> placed in there). Currently we emit a few WARN so that we would notice
> if this every occurred in the wild; it has. Sadly this means we need to
> replace those WARNs with the proper SIGBUS to the offending clients
> instead.

How does it happen? Frame buffer?

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem_userptr.c | 41 +++++++++++++++++++++++++++++----
>   1 file changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index f75d90118888..efb404b9fe69 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -81,11 +81,44 @@ static void __cancel_userptr__worker(struct work_struct *work)

This line is a reminder the previous series still hasn't landed. I think 
it was all r-b-ed, with only my request to not rely on release_pages (or 
something) handle negative and zero page count.

>   		was_interruptible = dev_priv->mm.interruptible;
>   		dev_priv->mm.interruptible = false;
>
> -		list_for_each_entry_safe(vma, tmp, &obj->vma_list, obj_link) {
> -			int ret = i915_vma_unbind(vma);
> -			WARN_ON(ret && ret != -EIO);
> +		list_for_each_entry_safe(vma, tmp, &obj->vma_list, obj_link)
> +			i915_vma_unbind(vma);
> +		if (i915_gem_object_put_pages(obj)) {
> +			struct task_struct *p;
> +
> +			DRM_ERROR("Unable to revoke ownership by userptr of"
> +				  " invalidated address range, sending SIGBUS"
> +				  " to attached clients.\n");
> +
> +			rcu_read_lock();
> +			for_each_process(p) {

I don't think this is safe this without holding the tasklist_lock.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx