[Intel-gfx] [PATCH v2] drm/i915/gtt: Avoid calling kcalloc in a loop when allocating temp bitmaps

[Chris Wilson]

On Mon, Aug 31, 2015 at 06:59: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.
> 
> v2: Use GFP_TEMPORARY to make the allocations reclaimable.
> 
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> Cc: Michel Thierry <michel.thierry at intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>

Is there any reason why it remains a 2D array though? Looks like it can
be reduced to a single block. And why do we allocate a bitmap for the
whole address space rather than just the range being allocated?

While you may just answer the questions posed, this looks clumsy:

> +	*pts = kcalloc(pdpes * BITS_TO_LONGS(I915_PDES),
> +			sizeof(unsigned long), GFP_TEMPORARY);
> +	if (!*pts)
> +		goto err_out;
> +
> +	for (i = 0; i < pdpes; i++)
> +		pts[i] = *pts + i * BITS_TO_LONGS(I915_PDES);

i..e

for (i = 1; i < pdpes; i++)
	pts[i] = pts[0] + i* BITS_TO_LONGS(I915_PDES);

raises fewer questions.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre