Re: [Intel-gfx] [PATCH v3 2/3] drm/i915: Fix userptr deadlock with aliased GTT mmappings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 09, 2015 at 02:56:16PM +0100, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 08/10/2015 09:51 AM, Chris Wilson wrote:
> > +out:
> >   	drm_free_large(pvec);
> >   	return ret;
> > +
> > +err:
> > +	/* No pages here, no need for the mmu-notifier to wake us */
> > +	__i915_gem_userptr_set_active(obj, false);
> > +err_active:
> > +	release_pages(pvec, pinned, 0);
> > +	goto out;
> >   }
> 
> I don't like the goto dance. Would something like the below be clearer?

We can condense it if we use a bool active and then feed everything
through the single exit path:

	active = false;
        if (pinned < 0)
                ret = pinned, pinned = 0;
        else if (pinned < num_pages)
                ret = __i915_gem_userptr_get_pages_queue(obj, &active);
        else
                ret = __i915_gem_userptr_set_pages(obj, pvec, num_pages);
        if (ret) {
                __i915_gem_userptr_set_active(obj, active);
                release_pages(pvec, pinned, 0);
        }
        drm_free_large(pvec);
        return ret;

Not happy with _queue. I guess i915_gem_userptr_get_pages_via_worker()
is better. Or i915_gem_userptr_get_pages_deferred().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]