Re: [PATCH 2/3] drm/i915/gtt: Free unused lower-level page tables

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ti, 2016-10-04 at 15:54 +0200, Michał Winiarski wrote:
> Since "Dynamic page table allocations" were introduced, our page tables
> can grow (being dynamically allocated) with address space range usage.
> Unfortunately, their lifetime is bound to vm. This is not a huge problem
> when we're not using softpin - drm_mm is creating an upper bound on used
> range by causing addresses for our VMAs to eventually be reused.
> 
> With softpin, long lived contexts can drain the system out of memory
> even with a single "small" object. For example:
> 
> bo = bo_alloc(size);
> while(true)
>     offset += size;
>     exec(bo, offset);
> 
> Will cause us to create new allocations until all memory in the system
> is used for tracking GPU pages (even though almost all PTEs in this vm
> are pointing to scratch).
> 
> Let's free unused page tables in clear_range to prevent this - if no
> entries are used, we can safely free it and return this information to
> the caller (so that higher-level entry is pointing to scratch).
> 

Sounds like this could and should have a I-G-T testcase, right?
 
> @@ -706,7 +710,7 @@ static int gen8_48b_mm_switch(struct i915_hw_ppgtt *ppgtt,
>  	return gen8_write_pdp(req, 0, px_dma(&ppgtt->pml4));
>  }
>  
> -static void gen8_ppgtt_clear_pt(struct i915_address_space *vm,

Add comment for non-obvious bool return value.

> +static bool gen8_ppgtt_clear_pt(struct i915_address_space *vm,
>  				struct i915_page_table *pt,
>  				uint64_t start,
>  				uint64_t length,
> @@ -724,50 +728,102 @@ static void gen8_ppgtt_clear_pt(struct i915_address_space *vm,
>  						 I915_CACHE_LLC, use_scratch);
>  
>  	if (WARN_ON(!px_page(pt)))
> -		return;
> +		return false;
>  
>  	bitmap_clear(pt->used_ptes, pte_start, num_entries);
>  
> +	if (bitmap_empty(pt->used_ptes, GEN8_PTES)) {
> +		free_pt(vm->dev, pt);

Maybe the caller should do the free_pt()? If kept here, should at least
be clearly documented.

Other than those, looks like good improvements to me.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx