On Mon, 15 Mar 2021 at 16:04, Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
>
> Instead evict round robin from each devices SYSTEM and TT domain.
>
> v2: reorder num_pages access reported by Dan's script
>
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 33 ++--------------
> drivers/gpu/drm/ttm/ttm_bo_util.c | 1 -
> drivers/gpu/drm/ttm/ttm_device.c | 60 +++++++++++++++++++++--------
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
> include/drm/ttm/ttm_bo_api.h | 1 -
> include/drm/ttm/ttm_bo_driver.h | 1 -
> include/drm/ttm/ttm_device.h | 7 +---
> 7 files changed, 52 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 56d2e38af273..a1be88be357b 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -73,7 +73,6 @@ static void ttm_bo_del_from_lru(struct ttm_buffer_object *bo)
> {
> struct ttm_device *bdev = bo->bdev;
>
> - list_del_init(&bo->swap);
> list_del_init(&bo->lru);
>
> if (bdev->funcs->del_from_lru_notify)
> @@ -104,16 +103,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
>
> man = ttm_manager_type(bdev, mem->mem_type);
> list_move_tail(&bo->lru, &man->lru[bo->priority]);
> - if (man->use_tt && bo->ttm &&
> - !(bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
> - TTM_PAGE_FLAG_SWAPPED))) {
> - struct list_head *swap;
> -
> - swap = &ttm_glob.swap_lru[bo->priority];
> - list_move_tail(&bo->swap, swap);
> - } else {
> - list_del_init(&bo->swap);
> - }
>
> if (bdev->funcs->del_from_lru_notify)
> bdev->funcs->del_from_lru_notify(bo);
> @@ -128,9 +117,6 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
> ttm_bo_bulk_move_set_pos(&bulk->vram[bo->priority], bo);
> break;
> }
> - if (bo->ttm && !(bo->ttm->page_flags &
> - (TTM_PAGE_FLAG_SG | TTM_PAGE_FLAG_SWAPPED)))
> - ttm_bo_bulk_move_set_pos(&bulk->swap[bo->priority], bo);
> }
> }
> EXPORT_SYMBOL(ttm_bo_move_to_lru_tail);
> @@ -168,20 +154,6 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk)
> list_bulk_move_tail(&man->lru[i], &pos->first->lru,
> &pos->last->lru);
> }
> -
> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
> - struct ttm_lru_bulk_move_pos *pos = &bulk->swap[i];
> - struct list_head *lru;
> -
> - if (!pos->first)
> - continue;
> -
> - dma_resv_assert_held(pos->first->base.resv);
> - dma_resv_assert_held(pos->last->base.resv);
> -
> - lru = &ttm_glob.swap_lru[i];
> - list_bulk_move_tail(lru, &pos->first->swap, &pos->last->swap);
> - }
> }
> EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
>
> @@ -1058,7 +1030,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev,
> kref_init(&bo->kref);
> INIT_LIST_HEAD(&bo->lru);
> INIT_LIST_HEAD(&bo->ddestroy);
> - INIT_LIST_HEAD(&bo->swap);
> bo->bdev = bdev;
> bo->type = type;
> bo->mem.mem_type = TTM_PL_SYSTEM;
> @@ -1193,6 +1164,10 @@ int ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
> bool locked;
> int ret;
>
> + if (!bo->ttm || bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
> + TTM_PAGE_FLAG_SWAPPED))
> + return false;
> +
return 0; ?
Seems inconsistent to return zero here and not drop the lru lock? Or
maybe turn this into a programmer error, since the current caller
already checks for the above?
> if (!ttm_bo_evict_swapout_allowable(bo, ctx, &locked, NULL))
> return -EBUSY;
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 031e5819fec4..a2a17c84ceb3 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -303,7 +303,6 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
> atomic_inc(&ttm_glob.bo_count);
> INIT_LIST_HEAD(&fbo->base.ddestroy);
> INIT_LIST_HEAD(&fbo->base.lru);
> - INIT_LIST_HEAD(&fbo->base.swap);
> fbo->base.moving = NULL;
> drm_vma_node_reset(&fbo->base.base.vma_node);
>
> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
> index dfc2a7e4e490..2c280fb1e992 100644
> --- a/drivers/gpu/drm/ttm/ttm_device.c
> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> @@ -67,7 +67,6 @@ static int ttm_global_init(void)
> unsigned long num_pages;
> struct sysinfo si;
> int ret = 0;
> - unsigned i;
>
> mutex_lock(&ttm_global_mutex);
> if (++ttm_glob_use_count > 1)
> @@ -90,8 +89,6 @@ static int ttm_global_init(void)
> goto out;
> }
>
> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i)
> - INIT_LIST_HEAD(&glob->swap_lru[i]);
> INIT_LIST_HEAD(&glob->device_list);
> atomic_set(&glob->bo_count, 0);
>
> @@ -109,27 +106,60 @@ static int ttm_global_init(void)
> long ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags)
> {
> struct ttm_global *glob = &ttm_glob;
> + struct ttm_device *bdev;
> + int ret = -EBUSY;
> +
> + mutex_lock(&ttm_global_mutex);
> + list_for_each_entry(bdev, &glob->device_list, device_list) {
> + ret = ttm_device_swapout(bdev, ctx, gfp_flags);
Mixing int and long for num_pages.
Does ttm enforce a maximum page count somewhere for object sizes?
Something like INT_MAX, since it doesn't look like ttm is consistently
using the same type(unsigned long?) when representing the number of
pages for an object?
> + if (ret > 0) {
> + list_move_tail(&bdev->device_list, &glob->device_list);
> + break;
> + }
> + }
> + mutex_unlock(&ttm_global_mutex);
> + return ret;
> +}
> +EXPORT_SYMBOL(ttm_global_swapout);
> +
> +long ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
> + gfp_t gfp_flags)
> +{
> + struct ttm_global *glob = &ttm_glob;
> + struct ttm_resource_manager *man;
> struct ttm_buffer_object *bo;
> - unsigned i;
> + unsigned i, j;
> int ret;
>
> spin_lock(&glob->lru_lock);
> - for (i = 0; i < TTM_MAX_BO_PRIORITY; ++i) {
> - list_for_each_entry(bo, &glob->swap_lru[i], swap) {
> - uint32_t num_pages = bo->ttm->num_pages;
> -
> - ret = ttm_bo_swapout(bo, ctx, gfp_flags);
> - /* ttm_bo_swapout has dropped the lru_lock */
> - if (!ret)
> - return num_pages;
> - if (ret != -EBUSY)
> - return ret;
> + for (i = TTM_PL_SYSTEM; i < TTM_NUM_MEM_TYPES; ++i) {
> + man = ttm_manager_type(bdev, i);
> + if (!man || !man->use_tt)
> + continue;
> +
> + for (j = 0; j < TTM_MAX_BO_PRIORITY; ++j) {
> + list_for_each_entry(bo, &man->lru[j], lru) {
> + long num_pages;
> +
> + if (!bo->ttm ||
> + bo->ttm->page_flags & TTM_PAGE_FLAG_SG ||
> + bo->ttm->page_flags & TTM_PAGE_FLAG_SWAPPED)
> + continue;
> +
> + num_pages = bo->ttm->num_pages;
> + ret = ttm_bo_swapout(bo, ctx, gfp_flags);
> + /* ttm_bo_swapout has dropped the lru_lock */
> + if (!ret)
> + return num_pages;
> + if (ret != -EBUSY)
> + return ret;
> + }
> }
> }
> spin_unlock(&glob->lru_lock);
> return 0;
> }
> -EXPORT_SYMBOL(ttm_global_swapout);
> +EXPORT_SYMBOL(ttm_device_swapout);
>
> static void ttm_init_sysman(struct ttm_device *bdev)
> {
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index b991422e156c..0e82b0662d9e 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1371,7 +1371,7 @@ static int vmw_pm_freeze(struct device *kdev)
> vmw_execbuf_release_pinned_bo(dev_priv);
> vmw_resource_evict_all(dev_priv);
> vmw_release_device_early(dev_priv);
> - while (ttm_global_swapout(&ctx, GFP_KERNEL) > 0);
> + while (ttm_device_swapout(&dev_priv->bdev, &ctx, GFP_KERNEL) == 0);
Is this the intended behaviour? ttm_device_swapout() still just
returns num_pages if it swapped something out. I assume this wants to
keep swapping stuff out, until it can't anymore. Or am I missing
something?
> if (dev_priv->enable_fb)
> vmw_fifo_resource_dec(dev_priv);
> if (atomic_read(&dev_priv->num_fifo_resources) != 0) {
> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> index 5044ac330858..3587f660e8f4 100644
> --- a/include/drm/ttm/ttm_bo_api.h
> +++ b/include/drm/ttm/ttm_bo_api.h
> @@ -144,7 +144,6 @@ struct ttm_buffer_object {
>
> struct list_head lru;
> struct list_head ddestroy;
> - struct list_head swap;
>
> /**
> * Members protected by a bo reservation.
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 8959c0075cfd..d007feef7676 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -69,7 +69,6 @@ struct ttm_lru_bulk_move_pos {
> struct ttm_lru_bulk_move {
> struct ttm_lru_bulk_move_pos tt[TTM_MAX_BO_PRIORITY];
> struct ttm_lru_bulk_move_pos vram[TTM_MAX_BO_PRIORITY];
> - struct ttm_lru_bulk_move_pos swap[TTM_MAX_BO_PRIORITY];
> };
>
> /*
> diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
> index 6a0b267d4fe6..cda6efb4c34b 100644
> --- a/include/drm/ttm/ttm_device.h
> +++ b/include/drm/ttm/ttm_device.h
> @@ -63,11 +63,6 @@ extern struct ttm_global {
> */
> struct list_head device_list;
>
> - /**
> - * Protected by the lru_lock.
> - */
> - struct list_head swap_lru[TTM_MAX_BO_PRIORITY];
> -
> /**
> * Internal protection.
> */
> @@ -298,6 +293,8 @@ struct ttm_device {
> };
>
> long ttm_global_swapout(struct ttm_operation_ctx *ctx, gfp_t gfp_flags);
> +long ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
> + gfp_t gfp_flags);
>
> static inline struct ttm_resource_manager *
> ttm_manager_type(struct ttm_device *bdev, int mem_type)
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
