[PATCH] intel: Leak the userptr test bo

[PATCH] intel: Leak the userptr test bo

[Chris Wilson]

In order to use userptr, the kernel tracks the owner's mm with a
mmu_notifier. Setting that is very expensive - it involves taking all
mm_locks and a stop_machine(). This tracking lives only for as long as
the client is using userptr objects - so if the client allocates then
frees a userptr in a loop, we will be executing that heavyweight setup
everytime. To ammoritize this cost, just leak the test bo and the single
backing page we use for detecting userptr.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 intel/intel_bufmgr_gem.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index a938441..51f8afe 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -976,15 +976,12 @@ retry:
 		return false;
 	}
 
-	memclear(close_bo);
-	close_bo.handle = userptr.handle;
-	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_GEM_CLOSE, &close_bo);
-	free(ptr);
-	if (ret) {
-		fprintf(stderr, "Failed to release test userptr object! (%d) "
-				"i915 kernel driver may not be sane!\n", errno);
-		return false;
-	}
+	/* We don't release the userptr bo here as we want to keep the
+	 * kernel mm tracking alive for our lifetime. The first time we
+	 * create a userptr object the kernel has to install a mmu_notifer
+	 * which is a heavyweight operation (e.g. it requires taking all
+	 * mm_locks and stop_machine()).
+	 */
 
 	return true;
 }
-- 
2.1.4

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

Re: [PATCH] intel: Leak the userptr test bo

[Tvrtko Ursulin]

On 04/14/2015 05:31 PM, Chris Wilson wrote:
> In order to use userptr, the kernel tracks the owner's mm with a
> mmu_notifier. Setting that is very expensive - it involves taking all
> mm_locks and a stop_machine(). This tracking lives only for as long as
> the client is using userptr objects - so if the client allocates then
> frees a userptr in a loop, we will be executing that heavyweight setup
> everytime. To ammoritize this cost, just leak the test bo and the single

Spellcheck on this line.

Also, if drm_intel_bufmgr_destroy is what I think it is, I think for 
correctness we would need to release that stuff there. What do you 
think? I could respin it with that if you are too busy?

Regards,

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

Re: [PATCH] intel: Leak the userptr test bo

[Chris Wilson]

On Wed, Apr 15, 2015 at 03:08:56PM +0100, Tvrtko Ursulin wrote:
> 
> On 04/14/2015 05:31 PM, Chris Wilson wrote:
> >In order to use userptr, the kernel tracks the owner's mm with a
> >mmu_notifier. Setting that is very expensive - it involves taking all
> >mm_locks and a stop_machine(). This tracking lives only for as long as
> >the client is using userptr objects - so if the client allocates then
> >frees a userptr in a loop, we will be executing that heavyweight setup
> >everytime. To ammoritize this cost, just leak the test bo and the single
> 
> Spellcheck on this line.
> 
> Also, if drm_intel_bufmgr_destroy is what I think it is, I think for
> correctness we would need to release that stuff there. What do you
> think? I could respin it with that if you are too busy?

I contemplated it, then decided I was too lazy to store a couple of
pointers. If you want to respin with that and push, please do.
-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