[RFC PATCH] drm/ttm: Do page counting after populate callback succeed

[Christian König]

Am 15.06.21 um 13:57 schrieb xinhui pan:
> Amdgpu set SG flag in populate callback. So TTM still count pages in SG
> BO.

It's probably better to fix this instead. E.g. why does amdgpu modify 
the SG flag during populate and not during initial creation? That 
doesn't seem to make sense.

Christian.

> One easy way to fix this is lets count pages after populate callback.
>
> We hit one issue that amdgpu alloc many SG BOs, but TTM try to do swap
> again and again even if swapout does not swap SG BOs at all.
>
> Signed-off-by: xinhui pan <xinhui.pan at amd.com>
> ---
>   drivers/gpu/drm/ttm/ttm_tt.c | 32 +++++++++++++-------------------
>   1 file changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index a1a25410ec74..4fa0a8cd71c0 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -317,13 +317,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
>   	if (ttm_tt_is_populated(ttm))
>   		return 0;
>   
> -	if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
> -		atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
> -		if (bdev->pool.use_dma32)
> -			atomic_long_add(ttm->num_pages,
> -					&ttm_dma32_pages_allocated);
> -	}
> -
>   	while (atomic_long_read(&ttm_pages_allocated) > ttm_pages_limit ||
>   	       atomic_long_read(&ttm_dma32_pages_allocated) >
>   	       ttm_dma32_pages_limit) {
> @@ -342,6 +335,13 @@ int ttm_tt_populate(struct ttm_device *bdev,
>   	if (ret)
>   		goto error;
>   
> +	if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
> +		atomic_long_add(ttm->num_pages, &ttm_pages_allocated);
> +		if (bdev->pool.use_dma32)
> +			atomic_long_add(ttm->num_pages,
> +					&ttm_dma32_pages_allocated);
> +	}
> +
>   	ttm_tt_add_mapping(bdev, ttm);
>   	ttm->page_flags |= TTM_PAGE_FLAG_PRIV_POPULATED;
>   	if (unlikely(ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)) {
> @@ -355,12 +355,6 @@ int ttm_tt_populate(struct ttm_device *bdev,
>   	return 0;
>   
>   error:
> -	if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
> -		atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
> -		if (bdev->pool.use_dma32)
> -			atomic_long_sub(ttm->num_pages,
> -					&ttm_dma32_pages_allocated);
> -	}
>   	return ret;
>   }
>   EXPORT_SYMBOL(ttm_tt_populate);
> @@ -384,12 +378,6 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct ttm_tt *ttm)
>   	if (!ttm_tt_is_populated(ttm))
>   		return;
>   
> -	ttm_tt_clear_mapping(ttm);
> -	if (bdev->funcs->ttm_tt_unpopulate)
> -		bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
> -	else
> -		ttm_pool_free(&bdev->pool, ttm);
> -
>   	if (!(ttm->page_flags & TTM_PAGE_FLAG_SG)) {
>   		atomic_long_sub(ttm->num_pages, &ttm_pages_allocated);
>   		if (bdev->pool.use_dma32)
> @@ -397,6 +385,12 @@ void ttm_tt_unpopulate(struct ttm_device *bdev, struct ttm_tt *ttm)
>   					&ttm_dma32_pages_allocated);
>   	}
>   
> +	ttm_tt_clear_mapping(ttm);
> +	if (bdev->funcs->ttm_tt_unpopulate)
> +		bdev->funcs->ttm_tt_unpopulate(bdev, ttm);
> +	else
> +		ttm_pool_free(&bdev->pool, ttm);
> +
>   	ttm->page_flags &= ~TTM_PAGE_FLAG_PRIV_POPULATED;
>   }
>