Am 15.03.21 um 19:54 schrieb Matthew Auld:
On Mon, 15 Mar 2021 at 16:04, Christian König ckoenig.leichtzumerken@gmail.com wrote:
[SNIP] @@ -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?
Thanks, that is just an artifact from rebasing and should be removed.
[SNIP]
@@ -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?
We should use 32 bit values for the number of pages in TTM, even signed values allow for 8TB large BOs.
And I really hope that we can get rid of the BO approach in general before we ever come close to that limit.
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?
I should probably add a check for that in the tt code, yes.
[SNIP] 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?
Indeed that's a mix up. Thanks for pointing that out.
Christian.