We need to make sure to allocate the sys_mem resource before the point of no return.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo_util.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 9be6a10a5873..eccf2ad8f335 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -582,6 +582,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) { static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; struct ttm_buffer_object *ghost; + struct ttm_resource *sys_res; struct ttm_tt *ttm; int ret;
@@ -602,6 +603,9 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) return ttm_resource_alloc(bo, &sys_mem, &bo->resource); }
+ + ret = ttm_resource_alloc(bo, &sys_mem, &sys_res); + /* * We need an unpopulated ttm_tt after giving our current one, * if any, to the ghost object. And we can't afford to fail @@ -615,13 +619,11 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) ret = ttm_tt_create(bo, true); swap(bo->ttm, ttm); if (ret) - return ret; + goto error_free_sys_mem;
ret = ttm_buffer_object_transfer(bo, &ghost); - if (ret) { - ttm_tt_destroy(bo->bdev, ttm); - return ret; - } + if (ret) + goto error_destroy_tt;
ret = dma_resv_copy_fences(&ghost->base._resv, bo->base.resv); /* Last resort, wait for the BO to be idle when we are OOM */ @@ -631,6 +633,14 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) dma_resv_unlock(&ghost->base._resv); ttm_bo_put(ghost); bo->ttm = ttm; + bo->resource = NULL; + ttm_bo_assign_mem(bo, sys_mem); + return 0;
- return ttm_resource_alloc(bo, &sys_mem, &bo->resource); +error_destroy_tt: + ttm_tt_destroy(bo->bdev, ttm); + +error_free_sys_mem: + ttm_resource_free(bo, &sys_mem); + return ret; }
On 6/8/21 9:50 AM, Christian König wrote:
We need to make sure to allocate the sys_mem resource before the point of no return.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/ttm/ttm_bo_util.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 9be6a10a5873..eccf2ad8f335 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -582,6 +582,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) { static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; struct ttm_buffer_object *ghost;
- struct ttm_resource *sys_res; struct ttm_tt *ttm; int ret;
@@ -602,6 +603,9 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) return ttm_resource_alloc(bo, &sys_mem, &bo->resource); }
- ret = ttm_resource_alloc(bo, &sys_mem, &sys_res);
This needs to be moved higher up to also cover the idle optimization path. Also the return value doesn't seem to be checked?
/Thomas
Hi "Christian,
I love your patch! Yet something to improve:
[auto build test ERROR on drm-tip/drm-tip] [cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master drm/drm-next v5.13-rc5 next-20210607] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Christian-K-nig/drm-ttm-fix-pipelin... base: git://anongit.freedesktop.org/drm/drm-tip drm-tip config: x86_64-randconfig-s021-20210607 (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.3-341-g8af24329-dirty # https://github.com/0day-ci/linux/commit/f7422b929beac0ad1d98db9ede99fa91a73f... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Christian-K-nig/drm-ttm-fix-pipelined-gutting/20210608-155139 git checkout f7422b929beac0ad1d98db9ede99fa91a73f0360 # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
drivers/gpu/drm/ttm/ttm_bo_util.c: In function 'ttm_bo_pipeline_gutting':
drivers/gpu/drm/ttm/ttm_bo_util.c:637:24: error: incompatible type for argument 2 of 'ttm_bo_assign_mem'
637 | ttm_bo_assign_mem(bo, sys_mem); | ^~~~~~~ | | | const struct ttm_place In file included from drivers/gpu/drm/ttm/ttm_bo_util.c:32: include/drm/ttm/ttm_bo_driver.h:190:31: note: expected 'struct ttm_resource *' but argument is of type 'const struct ttm_place' 190 | struct ttm_resource *new_mem) | ~~~~~~~~~~~~~~~~~~~~~^~~~~~~
drivers/gpu/drm/ttm/ttm_bo_util.c:644:24: error: passing argument 2 of 'ttm_resource_free' from incompatible pointer type [-Werror=incompatible-pointer-types]
644 | ttm_resource_free(bo, &sys_mem); | ^~~~~~~~ | | | const struct ttm_place * In file included from include/drm/ttm/ttm_device.h:30, from include/drm/ttm/ttm_bo_driver.h:40, from drivers/gpu/drm/ttm/ttm_bo_util.c:32: include/drm/ttm/ttm_resource.h:268:76: note: expected 'struct ttm_resource **' but argument is of type 'const struct ttm_place *' 268 | void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res); | ~~~~~~~~~~~~~~~~~~~~~~^~~ cc1: some warnings being treated as errors
vim +/ttm_bo_assign_mem +637 drivers/gpu/drm/ttm/ttm_bo_util.c
569 570 /** 571 * ttm_bo_pipeline_gutting - purge the contents of a bo 572 * @bo: The buffer object 573 * 574 * Purge the contents of a bo, async if the bo is not idle. 575 * After a successful call, the bo is left unpopulated in 576 * system placement. The function may wait uninterruptible 577 * for idle on OOM. 578 * 579 * Return: 0 if successful, negative error code on failure. 580 */ 581 int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) 582 { 583 static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; 584 struct ttm_buffer_object *ghost; 585 struct ttm_resource *sys_res; 586 struct ttm_tt *ttm; 587 int ret; 588 589 /* If already idle, no need for ghost object dance. */ 590 ret = ttm_bo_wait(bo, false, true); 591 if (ret != -EBUSY) { 592 if (!bo->ttm) { 593 /* See comment below about clearing. */ 594 ret = ttm_tt_create(bo, true); 595 if (ret) 596 return ret; 597 } else { 598 ttm_tt_unpopulate(bo->bdev, bo->ttm); 599 if (bo->type == ttm_bo_type_device) 600 ttm_tt_mark_for_clear(bo->ttm); 601 } 602 ttm_resource_free(bo, &bo->resource); 603 return ttm_resource_alloc(bo, &sys_mem, &bo->resource); 604 } 605 606 607 ret = ttm_resource_alloc(bo, &sys_mem, &sys_res); 608 609 /* 610 * We need an unpopulated ttm_tt after giving our current one, 611 * if any, to the ghost object. And we can't afford to fail 612 * creating one *after* the operation. If the bo subsequently gets 613 * resurrected, make sure it's cleared (if ttm_bo_type_device) 614 * to avoid leaking sensitive information to user-space. 615 */ 616 617 ttm = bo->ttm; 618 bo->ttm = NULL; 619 ret = ttm_tt_create(bo, true); 620 swap(bo->ttm, ttm); 621 if (ret) 622 goto error_free_sys_mem; 623 624 ret = ttm_buffer_object_transfer(bo, &ghost); 625 if (ret) 626 goto error_destroy_tt; 627 628 ret = dma_resv_copy_fences(&ghost->base._resv, bo->base.resv); 629 /* Last resort, wait for the BO to be idle when we are OOM */ 630 if (ret) 631 ttm_bo_wait(bo, false, false); 632 633 dma_resv_unlock(&ghost->base._resv); 634 ttm_bo_put(ghost); 635 bo->ttm = ttm; 636 bo->resource = NULL;
637 ttm_bo_assign_mem(bo, sys_mem);
638 return 0; 639 640 error_destroy_tt: 641 ttm_tt_destroy(bo->bdev, ttm); 642 643 error_free_sys_mem:
644 ttm_resource_free(bo, &sys_mem);
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi "Christian,
I love your patch! Yet something to improve:
[auto build test ERROR on drm-tip/drm-tip] [also build test ERROR on next-20210608] [cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master drm/drm-next v5.13-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Christian-K-nig/drm-ttm-fix-pipelin... base: git://anongit.freedesktop.org/drm/drm-tip drm-tip config: x86_64-randconfig-a005-20210607 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project d32cc150feb72f315a5bbd34f92e7beca21a50da) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/f7422b929beac0ad1d98db9ede99fa91a73f... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Christian-K-nig/drm-ttm-fix-pipelined-gutting/20210608-155139 git checkout f7422b929beac0ad1d98db9ede99fa91a73f0360 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
drivers/gpu/drm/ttm/ttm_bo_util.c:637:24: error: passing 'const struct ttm_place' to parameter of incompatible type 'struct ttm_resource *'
ttm_bo_assign_mem(bo, sys_mem); ^~~~~~~ include/drm/ttm/ttm_bo_driver.h:190:31: note: passing argument to parameter 'new_mem' here struct ttm_resource *new_mem) ^
drivers/gpu/drm/ttm/ttm_bo_util.c:644:24: error: incompatible pointer types passing 'const struct ttm_place *' to parameter of type 'struct ttm_resource **' [-Werror,-Wincompatible-pointer-types]
ttm_resource_free(bo, &sys_mem); ^~~~~~~~ include/drm/ttm/ttm_resource.h:268:76: note: passing argument to parameter 'res' here void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res); ^ 2 errors generated.
vim +637 drivers/gpu/drm/ttm/ttm_bo_util.c
569 570 /** 571 * ttm_bo_pipeline_gutting - purge the contents of a bo 572 * @bo: The buffer object 573 * 574 * Purge the contents of a bo, async if the bo is not idle. 575 * After a successful call, the bo is left unpopulated in 576 * system placement. The function may wait uninterruptible 577 * for idle on OOM. 578 * 579 * Return: 0 if successful, negative error code on failure. 580 */ 581 int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) 582 { 583 static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; 584 struct ttm_buffer_object *ghost; 585 struct ttm_resource *sys_res; 586 struct ttm_tt *ttm; 587 int ret; 588 589 /* If already idle, no need for ghost object dance. */ 590 ret = ttm_bo_wait(bo, false, true); 591 if (ret != -EBUSY) { 592 if (!bo->ttm) { 593 /* See comment below about clearing. */ 594 ret = ttm_tt_create(bo, true); 595 if (ret) 596 return ret; 597 } else { 598 ttm_tt_unpopulate(bo->bdev, bo->ttm); 599 if (bo->type == ttm_bo_type_device) 600 ttm_tt_mark_for_clear(bo->ttm); 601 } 602 ttm_resource_free(bo, &bo->resource); 603 return ttm_resource_alloc(bo, &sys_mem, &bo->resource); 604 } 605 606 607 ret = ttm_resource_alloc(bo, &sys_mem, &sys_res); 608 609 /* 610 * We need an unpopulated ttm_tt after giving our current one, 611 * if any, to the ghost object. And we can't afford to fail 612 * creating one *after* the operation. If the bo subsequently gets 613 * resurrected, make sure it's cleared (if ttm_bo_type_device) 614 * to avoid leaking sensitive information to user-space. 615 */ 616 617 ttm = bo->ttm; 618 bo->ttm = NULL; 619 ret = ttm_tt_create(bo, true); 620 swap(bo->ttm, ttm); 621 if (ret) 622 goto error_free_sys_mem; 623 624 ret = ttm_buffer_object_transfer(bo, &ghost); 625 if (ret) 626 goto error_destroy_tt; 627 628 ret = dma_resv_copy_fences(&ghost->base._resv, bo->base.resv); 629 /* Last resort, wait for the BO to be idle when we are OOM */ 630 if (ret) 631 ttm_bo_wait(bo, false, false); 632 633 dma_resv_unlock(&ghost->base._resv); 634 ttm_bo_put(ghost); 635 bo->ttm = ttm; 636 bo->resource = NULL;
637 ttm_bo_assign_mem(bo, sys_mem);
638 return 0; 639 640 error_destroy_tt: 641 ttm_tt_destroy(bo->bdev, ttm); 642 643 error_free_sys_mem:
644 ttm_resource_free(bo, &sys_mem);
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
dri-devel@lists.freedesktop.org
-
Christian König -
kernel test robot -
Thomas Hellström