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