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

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

 



On Fri, Oct 07, 2016 at 08:39:22PM +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).
> 
> v2: Document return value and free semantics (Joonas)
> v3: No newlines in vars block (Joonas)
> 
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Michel Thierry <michel.thierry@xxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx>

Aside from not liking the bitmaps at all (yay for the extra memory
pressure for no purpose),
Reviewed-by: Chris wilson <chris@xxxxxxxxxxxxxxxxxx>

(and for the earlier patches)

>  	gen8_for_each_pde(pt, pd, start, length, pde) {
>  		if (WARN_ON(!pd->page_table[pde]))
>  			break;
>  
> -		gen8_ppgtt_clear_pt(vm, pt, start, length);
> +		reduce = gen8_ppgtt_clear_pt(vm, pt, start, length);
> +
> +		if (reduce) {

I would not have put a newline here as the if() is coupled to the
clear_pte() (and I would have just used if (clear_pte())).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
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