On 09/09/2015 04:03 PM, Chris Wilson wrote:
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().
Looks much better on a glance. If release_pages with pinned = 0 is OK.
For the queueue/via_worker/deferred maybe _schedule_get_pages_worker?
Tvrtko
--
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