Re: [PATCH 6/6] drm/i915: Avoid allocating a vmap arena for a single page

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

 




On 06/04/16 11:05, Chris Wilson wrote:
On Wed, Apr 06, 2016 at 10:49:36AM +0100, Tvrtko Ursulin wrote:
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 985f067c1f0e..dc8e1b76c896 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2233,7 +2233,10 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
  	list_del(&obj->global_list);

  	if (obj->vmapping) {
-		vunmap(obj->vmapping);
+		if (obj->base.size == PAGE_SIZE)
+			kunmap(kmap_to_page(obj->vmapping));
+		else
+			vunmap(obj->vmapping);

Can't think of a reason why it would be better but there is also
is_vmalloc_addr(addr) as used by kvfree.

For consistency with the shrinker (see below).

  		obj->vmapping = NULL;
  	}

@@ -2416,15 +2419,22 @@ void *i915_gem_object_pin_vmap(struct drm_i915_gem_object *obj)
  	i915_gem_object_pin_pages(obj);

  	if (obj->vmapping == NULL) {
-		struct sg_page_iter sg_iter;
  		struct page **pages;
-		int n;

-		n = obj->base.size >> PAGE_SHIFT;
-		pages = drm_malloc_gfp(n, sizeof(*pages), GFP_TEMPORARY);
+		pages = NULL;
+		if (obj->base.size == PAGE_SIZE)
+			obj->vmapping = kmap(sg_page(obj->pages->sgl));
+		else
+			pages = drm_malloc_gfp(obj->base.size >> PAGE_SHIFT,
+					       sizeof(*pages),
+					       GFP_TEMPORARY);
  		if (pages != NULL) {
+			struct sg_page_iter sg_iter;
+			int n;
+
  			n = 0;
-			for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0)
+			for_each_sg_page(obj->pages->sgl, &sg_iter,
+					 obj->pages->nents, 0)
  				pages[n++] = sg_page_iter_page(&sg_iter);

  			obj->vmapping = vmap(pages, n, 0, PAGE_KERNEL);


Two problems I can spot are:

1. Callers of pin_vmap now don't know which kind of address they are
getting. Maybe call it pin_kvmap or something? Just mention in
kerneldoc could be enough.

I think just mention, and we can rename this to i915_gem_object_pin_map().
Hmm. I liked the pin in the name since it ties to to pin_pages (later
I plan to change that to get_pages and get_vmap/get_map as the pin
becomes implicit).

2. Shrinker will try to kick out kmapped objects because they have
obj->vmapping set.

Not caring that much since the vmap_purge is very heavy weight, but we
can apply is_vmalloc_addr() to the shrinker.

Ok, happy to call this obj->mapping and i915_gem_object_pin_map() ?

Sounds good. (Including the is_vmalloc_addr() smartening in the shrinker.)

Regards,

Tvrtko


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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux