drm/i915/gtt: Avoid calling kcalloc in a loop when allocating temp bitmaps

Commit Message

Michał Winiarski Aug. 31, 2015, 4:27 p.m. UTC

On each call to gen8_alloc_va_range_3lvl we're allocating temporary
bitmaps needed for error handling. Unfortunately, when we increase
address space size (48b ppgtt) we do additional (512 - 4) calls to
kcalloc, increasing latency between exec and actual start of execution
on the GPU. Let's just do a single kcalloc and setup proper offsets in
an array, we can also drop the size from free_gen8_temp_bitmaps since
it's no longer needed.

Cc: Mika Kuoppala <[e-mail]>
Cc: Michel Thierry <[e-mail]>
Signed-off-by: Micha? Winiarski <[e-mail]>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

Comments

Chris Wilson Aug. 31, 2015, 4:40 p.m. UTC | #1

On Mon, Aug 31, 2015 at 06:27:40PM +0200, Micha? Winiarski wrote:
> On each call to gen8_alloc_va_range_3lvl we're allocating temporary
> bitmaps needed for error handling. Unfortunately, when we increase
> address space size (48b ppgtt) we do additional (512 - 4) calls to
> kcalloc, increasing latency between exec and actual start of execution
> on the GPU. Let's just do a single kcalloc and setup proper offsets in
> an array, we can also drop the size from free_gen8_temp_bitmaps since
> it's no longer needed.
> 
> Cc: Mika Kuoppala <[e-mail]>
> Cc: Michel Thierry <[e-mail]>
> Signed-off-by: Micha? Winiarski <[e-mail]>

Whilst you are here, mind using GFP_TEMPORARY as well?
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 4a76807..fd5545a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1126,13 +1126,9 @@  unwind_out:
 }
 
 static void
-free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned long **new_pts,
-		       uint32_t pdpes)
+free_gen8_temp_bitmaps(unsigned long *new_pds, unsigned long **new_pts)
 {
-	int i;
-
-	for (i = 0; i < pdpes; i++)
-		kfree(new_pts[i]);
+	kfree(*new_pts);
 	kfree(new_pts);
 	kfree(new_pds);
 }
@@ -1154,17 +1150,16 @@  int __must_check alloc_gen8_temp_bitmaps(unsigned long **new_pds,
 		return -ENOMEM;
 
 	pts = kcalloc(pdpes, sizeof(unsigned long *), GFP_KERNEL);
-	if (!pts) {
-		kfree(pds);
-		return -ENOMEM;
-	}
+	if (!pts)
+		goto err_out;
 
-	for (i = 0; i < pdpes; i++) {
-		pts[i] = kcalloc(BITS_TO_LONGS(I915_PDES),
-				 sizeof(unsigned long), GFP_KERNEL);
-		if (!pts[i])
-			goto err_out;
-	}
+	*pts = kcalloc(pdpes * BITS_TO_LONGS(I915_PDES),
+			sizeof(unsigned long), GFP_KERNEL);
+	if (!*pts)
+		goto err_out;
+
+	for (i = 0; i < pdpes; i++)
+		pts[i] = *pts + i * BITS_TO_LONGS(I915_PDES);
 
 	*new_pds = pds;
 	*new_pts = pts;
@@ -1172,7 +1167,7 @@  int __must_check alloc_gen8_temp_bitmaps(unsigned long **new_pds,
 	return 0;
 
 err_out:
-	free_gen8_temp_bitmaps(pds, pts, pdpes);
+	free_gen8_temp_bitmaps(pds, pts);
 	return -ENOMEM;
 }
 
@@ -1220,7 +1215,7 @@  static int gen8_alloc_va_range_3lvl(struct i915_address_space *vm,
 	ret = gen8_ppgtt_alloc_page_directories(vm, pdp, start, length,
 						new_page_dirs);
 	if (ret) {
-		free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
+		free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
 		return ret;
 	}
 
@@ -1278,7 +1273,7 @@  static int gen8_alloc_va_range_3lvl(struct i915_address_space *vm,
 		gen8_setup_page_directory(ppgtt, pdp, pd, pdpe);
 	}
 
-	free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
+	free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
 	mark_tlbs_dirty(ppgtt);
 	return 0;
 
@@ -1291,7 +1286,7 @@  err_out:
 	for_each_set_bit(pdpe, new_page_dirs, pdpes)
 		free_pd(dev, pdp->page_directory[pdpe]);
 
-	free_gen8_temp_bitmaps(new_page_dirs, new_page_tables, pdpes);
+	free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
 	mark_tlbs_dirty(ppgtt);
 	return ret;
 }