[PATCH 3/5] drm/ttm: add page order support in ttm_pages_put

[Christian König]

That completely negates the advantage of setting write back on multiple 
pages at once.

In other words this way we wouldn't need the array any more at all and 
could remove the whole complicated handling.

I'm pretty close to saying just go ahead with that and even clean up the 
whole stuff with the static array, but I'm not sure if that doesn't 
result in some performance problems.

A possible alternative is the (untested) patch attached, this way we 
move the __free_page()/_free_pages() call out of ttm_pages_put and just 
need to add the correct number of pages to the array in the loop.

Regards,
Christian.

Am 22.11.2017 um 10:17 schrieb Roger He:
> Change-Id: Ia55b206d95812c5afcfd6cec29f580758d1f50f0
> Signed-off-by: Roger He <Hongbo.He at amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_page_alloc.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> index 9b48b93..2dc83c0 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> @@ -299,13 +299,16 @@ static struct ttm_page_pool *ttm_get_pool(int flags, bool huge,
>   }
>   
>   /* set memory back to wb and free the pages. */
> -static void ttm_pages_put(struct page *pages[], unsigned npages)
> +static void ttm_pages_put(struct page *pages[], unsigned npages,
> +		unsigned int order)
>   {
> -	unsigned i;
> -	if (set_pages_array_wb(pages, npages))
> -		pr_err("Failed to set %d pages to wb!\n", npages);
> -	for (i = 0; i < npages; ++i)
> -		__free_page(pages[i]);
> +	unsigned int i, pages_nr = (1 << order);
> +
> +	for (i = 0; i < npages; ++i) {
> +		if (set_pages_wb(pages[i], pages_nr))
> +			pr_err("Failed to set %d pages to wb!\n", pages_nr);
> +		__free_pages(pages[i], order);
> +	}
>   }
>   
>   static void ttm_pool_update_free_locked(struct ttm_page_pool *pool,
> @@ -368,7 +371,7 @@ static int ttm_page_pool_free(struct ttm_page_pool *pool, unsigned nr_free,
>   			 */
>   			spin_unlock_irqrestore(&pool->lock, irq_flags);
>   
> -			ttm_pages_put(pages_to_free, freed_pages);
> +			ttm_pages_put(pages_to_free, freed_pages, pool->order);
>   			if (likely(nr_free != FREE_ALL_PAGES))
>   				nr_free -= freed_pages;
>   
> @@ -403,7 +406,7 @@ static int ttm_page_pool_free(struct ttm_page_pool *pool, unsigned nr_free,
>   	spin_unlock_irqrestore(&pool->lock, irq_flags);
>   
>   	if (freed_pages)
> -		ttm_pages_put(pages_to_free, freed_pages);
> +		ttm_pages_put(pages_to_free, freed_pages, pool->order);
>   out:
>   	if (pages_to_free != static_buf)
>   		kfree(pages_to_free);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-ttm-move-__free_page-out-of-ttm_pages_put.patch
Type: text/x-patch
Size: 2744 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20171122/bf4cd019/attachment-0001.bin>