Just sending that out once more to intel-gfx to let the CI systems take a look.
No functional change compared to the last version.
Christian.
Use the new interface instead.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/radeon/radeon_object.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 6c4a6802ca96..1d414ff4ab0c 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -133,9 +133,12 @@ int radeon_bo_create(struct radeon_device *rdev, struct dma_resv *resv, struct radeon_bo **bo_ptr) { - struct radeon_bo *bo; - enum ttm_bo_type type; unsigned long page_align = roundup(byte_align, PAGE_SIZE) >> PAGE_SHIFT; + + /* Kernel allocation are uninterruptible */ + struct ttm_operation_ctx ctx = { !kernel, false }; + enum ttm_bo_type type; + struct radeon_bo *bo; int r;
size = ALIGN(size, PAGE_SIZE); @@ -200,11 +203,13 @@ int radeon_bo_create(struct radeon_device *rdev, #endif
radeon_ttm_placement_from_domain(bo, domain); - /* Kernel allocation are uninterruptible */ down_read(&rdev->pm.mclk_lock); - r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type, - &bo->placement, page_align, !kernel, sg, resv, - &radeon_ttm_bo_destroy); + r = ttm_bo_init_reserved(&rdev->mman.bdev, &bo->tbo, size, type, + &bo->placement, page_align, &ctx, sg, resv, + &radeon_ttm_bo_destroy); + if (!r) + ttm_bo_unreserve(&bo->tbo); + up_read(&rdev->pm.mclk_lock); if (unlikely(r != 0)) { return r;
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Christian König Sent: Thursday, May 19, 2022 5:55 AM To: intel-gfx@lists.freedesktop.org Cc: matthew.william.auld@gmail.com; Christian König christian.koenig@amd.com; dri-devel@lists.freedesktop.org Subject: [PATCH 01/11] drm/radeon: switch over to ttm_bo_init_reserved
Use the new interface instead.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/radeon/radeon_object.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 6c4a6802ca96..1d414ff4ab0c 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -133,9 +133,12 @@ int radeon_bo_create(struct radeon_device *rdev, struct dma_resv *resv, struct radeon_bo **bo_ptr) {
- struct radeon_bo *bo;
- enum ttm_bo_type type; unsigned long page_align = roundup(byte_align, PAGE_SIZE) >>
PAGE_SHIFT;
/* Kernel allocation are uninterruptible */
struct ttm_operation_ctx ctx = { !kernel, false };
enum ttm_bo_type type;
struct radeon_bo *bo; int r;
size = ALIGN(size, PAGE_SIZE);
@@ -200,11 +203,13 @@ int radeon_bo_create(struct radeon_device *rdev, #endif
radeon_ttm_placement_from_domain(bo, domain);
- /* Kernel allocation are uninterruptible */ down_read(&rdev->pm.mclk_lock);
- r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
&bo->placement, page_align, !kernel, sg, resv,&radeon_ttm_bo_destroy);
- r = ttm_bo_init_reserved(&rdev->mman.bdev, &bo->tbo, size, type,
&bo->placement, page_align, &ctx, sg, resv,&radeon_ttm_bo_destroy);if (!r)ttm_bo_unreserve(&bo->tbo);
Hi Christian,
I am not understanding this unreserve.
The original code path does not have it. It looks like tt_bo_init will do this, but only if !resv.
Should this be: if (!resv) ttm_bo_unreserve(&bo->tbo);
?
M
up_read(&rdev->pm.mclk_lock); if (unlikely(r != 0)) { return r; -- 2.25.1
Am 19.05.22 um 14:54 schrieb Ruhl, Michael J:
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Christian König Sent: Thursday, May 19, 2022 5:55 AM To: intel-gfx@lists.freedesktop.org Cc: matthew.william.auld@gmail.com; Christian König christian.koenig@amd.com; dri-devel@lists.freedesktop.org Subject: [PATCH 01/11] drm/radeon: switch over to ttm_bo_init_reserved
Use the new interface instead.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/radeon/radeon_object.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 6c4a6802ca96..1d414ff4ab0c 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -133,9 +133,12 @@ int radeon_bo_create(struct radeon_device *rdev, struct dma_resv *resv, struct radeon_bo **bo_ptr) {
- struct radeon_bo *bo;
- enum ttm_bo_type type; unsigned long page_align = roundup(byte_align, PAGE_SIZE) >>
PAGE_SHIFT;
/* Kernel allocation are uninterruptible */
struct ttm_operation_ctx ctx = { !kernel, false };
enum ttm_bo_type type;
struct radeon_bo *bo; int r;
size = ALIGN(size, PAGE_SIZE);
@@ -200,11 +203,13 @@ int radeon_bo_create(struct radeon_device *rdev, #endif
radeon_ttm_placement_from_domain(bo, domain);
- /* Kernel allocation are uninterruptible */ down_read(&rdev->pm.mclk_lock);
- r = ttm_bo_init(&rdev->mman.bdev, &bo->tbo, size, type,
&bo->placement, page_align, !kernel, sg, resv,&radeon_ttm_bo_destroy);
- r = ttm_bo_init_reserved(&rdev->mman.bdev, &bo->tbo, size, type,
&bo->placement, page_align, &ctx, sg, resv,&radeon_ttm_bo_destroy);if (!r)ttm_bo_unreserve(&bo->tbo);Hi Christian,
I am not understanding this unreserve.
The original code path does not have it. It looks like tt_bo_init will do this, but only if !resv.
Should this be: if (!resv) ttm_bo_unreserve(&bo->tbo);
Ah, yes good point. That's a bug.
Thanks, Christian.
?
M
up_read(&rdev->pm.mclk_lock); if (unlikely(r != 0)) { return r; -- 2.25.1
Use the new interface instead.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/nouveau/nouveau_bo.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 05076e530e7d..858b9382036c 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -302,19 +302,23 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 domain, struct sg_table *sg, struct dma_resv *robj) { int type = sg ? ttm_bo_type_sg : ttm_bo_type_device; + struct ttm_operation_ctx ctx = { false, false }; int ret;
nouveau_bo_placement_set(nvbo, domain, 0); INIT_LIST_HEAD(&nvbo->io_reserve_lru);
- ret = ttm_bo_init(nvbo->bo.bdev, &nvbo->bo, size, type, - &nvbo->placement, align >> PAGE_SHIFT, false, sg, - robj, nouveau_bo_del_ttm); + ret = ttm_bo_init_reserved(nvbo->bo.bdev, &nvbo->bo, size, type, + &nvbo->placement, align >> PAGE_SHIFT, &ctx, + sg, robj, nouveau_bo_del_ttm); if (ret) { /* ttm will call nouveau_bo_del_ttm if it fails.. */ return ret; }
+ if (!robj) + ttm_bo_unreserve(&nvbo->bo); + return 0; }
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Christian König Sent: Thursday, May 19, 2022 5:55 AM To: intel-gfx@lists.freedesktop.org Cc: matthew.william.auld@gmail.com; Christian König christian.koenig@amd.com; dri-devel@lists.freedesktop.org Subject: [PATCH 02/11] drm/nouveau: switch over to ttm_bo_init_reserved
Use the new interface instead.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/nouveau/nouveau_bo.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 05076e530e7d..858b9382036c 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -302,19 +302,23 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 domain, struct sg_table *sg, struct dma_resv *robj) { int type = sg ? ttm_bo_type_sg : ttm_bo_type_device;
struct ttm_operation_ctx ctx = { false, false }; int ret;
nouveau_bo_placement_set(nvbo, domain, 0); INIT_LIST_HEAD(&nvbo->io_reserve_lru);
- ret = ttm_bo_init(nvbo->bo.bdev, &nvbo->bo, size, type,
&nvbo->placement, align >> PAGE_SHIFT, false, sg,robj, nouveau_bo_del_ttm);
- ret = ttm_bo_init_reserved(nvbo->bo.bdev, &nvbo->bo, size, type,
&nvbo->placement, align >> PAGE_SHIFT,&ctx,
sg, robj, nouveau_bo_del_ttm);if (ret) { /* ttm will call nouveau_bo_del_ttm if it fails.. */ return ret; }
if (!robj)
ttm_bo_unreserve(&nvbo->bo);
Ok, this implies that patch 1 does have an issue.
I see this usage in patch 1, 2, and 3. Would it make sense to move this _unreserve to ttm_bo_init_reserved?
Mike
return 0; }
-- 2.25.1
Am 19.05.22 um 15:19 schrieb Ruhl, Michael J:
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Christian König Sent: Thursday, May 19, 2022 5:55 AM To: intel-gfx@lists.freedesktop.org Cc: matthew.william.auld@gmail.com; Christian König christian.koenig@amd.com; dri-devel@lists.freedesktop.org Subject: [PATCH 02/11] drm/nouveau: switch over to ttm_bo_init_reserved
Use the new interface instead.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/nouveau/nouveau_bo.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 05076e530e7d..858b9382036c 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -302,19 +302,23 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 domain, struct sg_table *sg, struct dma_resv *robj) { int type = sg ? ttm_bo_type_sg : ttm_bo_type_device;
struct ttm_operation_ctx ctx = { false, false }; int ret;
nouveau_bo_placement_set(nvbo, domain, 0); INIT_LIST_HEAD(&nvbo->io_reserve_lru);
- ret = ttm_bo_init(nvbo->bo.bdev, &nvbo->bo, size, type,
&nvbo->placement, align >> PAGE_SHIFT, false, sg,robj, nouveau_bo_del_ttm);
- ret = ttm_bo_init_reserved(nvbo->bo.bdev, &nvbo->bo, size, type,
&nvbo->placement, align >> PAGE_SHIFT,&ctx,
sg, robj, nouveau_bo_del_ttm);if (ret) { /* ttm will call nouveau_bo_del_ttm if it fails.. */ return ret; }
if (!robj)
ttm_bo_unreserve(&nvbo->bo);Ok, this implies that patch 1 does have an issue.
I see this usage in patch 1, 2, and 3. Would it make sense to move this _unreserve to ttm_bo_init_reserved?
Well the whole point of ttm_bo_init_reserved is that you need to do the un-reserve manually.
But yeah, you are right. It would just make much more sense to rename ttm_bo_init() instead of adjusting all of it's users.
Thanks, Christian.
Mike
return 0; }
-- 2.25.1
Use the new interface instead.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/drm_gem_vram_helper.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 123045b58fec..7449cbc2f925 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -186,6 +186,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, size_t size, unsigned long pg_align) { + struct ttm_operation_ctx ctx = { false, false }; struct drm_gem_vram_object *gbo; struct drm_gem_object *gem; struct drm_vram_mm *vmm = dev->vram_mm; @@ -225,12 +226,13 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, * A failing ttm_bo_init will call ttm_buffer_object_destroy * to release gbo->bo.base and kfree gbo. */ - ret = ttm_bo_init(bdev, &gbo->bo, size, ttm_bo_type_device, - &gbo->placement, pg_align, false, NULL, NULL, - ttm_buffer_object_destroy); - if (ret) + ret = ttm_bo_init_reserved(bdev, &gbo->bo, size, ttm_bo_type_device, + &gbo->placement, pg_align, &ctx, NULL, NULL, + ttm_buffer_object_destroy); + if (ret) return ERR_PTR(ret);
+ ttm_bo_unreserve(&gbo->bo); return gbo; } EXPORT_SYMBOL(drm_gem_vram_create);
It's the only driver using this.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 85a66014c2b6..c4f376d5e1d0 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -462,6 +462,9 @@ int vmw_bo_create(struct vmw_private *vmw, return -ENOMEM; }
+ if (!bo_free) + bo_free = vmw_bo_default_destroy; + ret = vmw_bo_init(vmw, *p_bo, size, placement, interruptible, pin, bo_free);
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Christian König Sent: Thursday, May 19, 2022 5:55 AM To: intel-gfx@lists.freedesktop.org Cc: matthew.william.auld@gmail.com; Christian König christian.koenig@amd.com; dri-devel@lists.freedesktop.org Subject: [PATCH 04/11] drm/ttm: move default BO destructor into VMWGFX
It's the only driver using this.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 85a66014c2b6..c4f376d5e1d0 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -462,6 +462,9 @@ int vmw_bo_create(struct vmw_private *vmw, return -ENOMEM; }
- if (!bo_free)
bo_free = vmw_bo_default_destroy;
vmw_bo_init has a WARN_ON if this is NULL.
Also, all of the callers use vmw_bo_bo_free() or vmw_gem_destroy().
Both of those unmap, release and then free the object.
It doesn't look like vmw_bo_default_destroy does this work.
Is this the right "default" path? Or should the WARN_ON be used to check for this?
M
ret = vmw_bo_init(vmw, *p_bo, size, placement, interruptible, pin, bo_free); -- 2.25.1
Am 19.05.22 um 15:36 schrieb Ruhl, Michael J:
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Christian König Sent: Thursday, May 19, 2022 5:55 AM To: intel-gfx@lists.freedesktop.org Cc: matthew.william.auld@gmail.com; Christian König christian.koenig@amd.com; dri-devel@lists.freedesktop.org Subject: [PATCH 04/11] drm/ttm: move default BO destructor into VMWGFX
It's the only driver using this.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 85a66014c2b6..c4f376d5e1d0 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -462,6 +462,9 @@ int vmw_bo_create(struct vmw_private *vmw, return -ENOMEM; }
- if (!bo_free)
bo_free = vmw_bo_default_destroy;vmw_bo_init has a WARN_ON if this is NULL.
Also, all of the callers use vmw_bo_bo_free() or vmw_gem_destroy().
Both of those unmap, release and then free the object.
It doesn't look like vmw_bo_default_destroy does this work.
Is this the right "default" path? Or should the WARN_ON be used to check for this?
This patch here was just a rebase fallout I've overlooked.
Zak already reviewed it and I pushed a modified version of it upstream (that's where the WARN_ON comes from).
Thanks, Christian.
M
ret = vmw_bo_init(vmw, *p_bo, size, placement, interruptible, pin, bo_free); -- 2.25.1
Not used any more.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo.c | 26 --------------------- include/drm/ttm/ttm_bo_api.h | 44 ------------------------------------ 2 files changed, 70 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 296af2b89951..e652055b5175 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -985,32 +985,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, } EXPORT_SYMBOL(ttm_bo_init_reserved);
-int ttm_bo_init(struct ttm_device *bdev, - struct ttm_buffer_object *bo, - size_t size, - enum ttm_bo_type type, - struct ttm_placement *placement, - uint32_t page_alignment, - bool interruptible, - struct sg_table *sg, - struct dma_resv *resv, - void (*destroy) (struct ttm_buffer_object *)) -{ - struct ttm_operation_ctx ctx = { interruptible, false }; - int ret; - - ret = ttm_bo_init_reserved(bdev, bo, size, type, placement, - page_alignment, &ctx, sg, resv, destroy); - if (ret) - return ret; - - if (!resv) - ttm_bo_unreserve(bo); - - return 0; -} -EXPORT_SYMBOL(ttm_bo_init); - /* * buffer object vm functions. */ diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 2d524f8b0802..29384e2cb704 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -361,50 +361,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *));
-/** - * ttm_bo_init - * - * @bdev: Pointer to a ttm_device struct. - * @bo: Pointer to a ttm_buffer_object to be initialized. - * @size: Requested size of buffer object. - * @type: Requested type of buffer object. - * @placement: Initial placement for buffer object. - * @page_alignment: Data alignment in pages. - * @interruptible: If needing to sleep to wait for GPU resources, - * sleep interruptible. - * pinned in physical memory. If this behaviour is not desired, this member - * holds a pointer to a persistent shmem object. Typically, this would - * point to the shmem object backing a GEM object if TTM is used to back a - * GEM user interface. - * @sg: Scatter-gather table. - * @resv: Pointer to a dma_resv, or NULL to let ttm allocate one. - * @destroy: Destroy function. Use NULL for kfree(). - * - * This function initializes a pre-allocated struct ttm_buffer_object. - * As this object may be part of a larger structure, this function, - * together with the @destroy function, - * enables driver-specific objects derived from a ttm_buffer_object. - * - * On successful return, the caller owns an object kref to @bo. The kref and - * list_kref are usually set to 1, but note that in some situations, other - * tasks may already be holding references to @bo as well. - * - * If a failure occurs, the function will call the @destroy function, or - * kfree() if @destroy is NULL. Thus, after a failure, dereferencing @bo is - * illegal and will likely cause memory corruption. - * - * Returns - * -ENOMEM: Out of memory. - * -EINVAL: Invalid placement flags. - * -ERESTARTSYS: Interrupted by signal while sleeping waiting for resources. - */ -int ttm_bo_init(struct ttm_device *bdev, struct ttm_buffer_object *bo, - size_t size, enum ttm_bo_type type, - struct ttm_placement *placement, - uint32_t page_alignment, bool interrubtible, - struct sg_table *sg, struct dma_resv *resv, - void (*destroy) (struct ttm_buffer_object *)); - /** * ttm_kmap_obj_virtual *
Rename ttm_bo_init_reserved to ttm_bo_init_validate since that better matches what the function is actually doing.
Remove the unused size parameter, move the function's kerneldoc to the implementation and cleanup the whole error handling.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/drm_gem_vram_helper.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 5 +- drivers/gpu/drm/nouveau/nouveau_bo.c | 2 +- drivers/gpu/drm/qxl/qxl_object.c | 2 +- drivers/gpu/drm/radeon/radeon_object.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 92 ++++++++++++++-------- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 12 ++- include/drm/ttm/ttm_bo_api.h | 44 +---------- 9 files changed, 75 insertions(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 5444515c1476..116c8d31e646 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -590,7 +590,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev, if (!bp->destroy) bp->destroy = &amdgpu_bo_destroy;
- r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type, + r = ttm_bo_init_validate(&adev->mman.bdev, &bo->tbo, bp->type, &bo->placement, page_align, &ctx, NULL, bp->resv, bp->destroy); if (unlikely(r != 0)) diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 7449cbc2f925..73e91baccea0 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -226,7 +226,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, * A failing ttm_bo_init will call ttm_buffer_object_destroy * to release gbo->bo.base and kfree gbo. */ - ret = ttm_bo_init_reserved(bdev, &gbo->bo, size, ttm_bo_type_device, + ret = ttm_bo_init_validate(bdev, &gbo->bo, ttm_bo_type_device, &gbo->placement, pg_align, &ctx, NULL, NULL, ttm_buffer_object_destroy); if (ret) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 4c25d9b2f138..253188da91eb 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1229,9 +1229,8 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem, * Similarly, in delayed_destroy, we can't call ttm_bo_put() * until successful initialization. */ - ret = ttm_bo_init_reserved(&i915->bdev, i915_gem_to_ttm(obj), size, - bo_type, &i915_sys_placement, - page_size >> PAGE_SHIFT, + ret = ttm_bo_init_validate(&i915->bdev, i915_gem_to_ttm(obj), bo_type, + &i915_sys_placement, page_size >> PAGE_SHIFT, &ctx, NULL, NULL, i915_ttm_bo_destroy); if (ret) return i915_ttm_err_to_gem(ret); diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 858b9382036c..666941804297 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -308,7 +308,7 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 domain, nouveau_bo_placement_set(nvbo, domain, 0); INIT_LIST_HEAD(&nvbo->io_reserve_lru);
- ret = ttm_bo_init_reserved(nvbo->bo.bdev, &nvbo->bo, size, type, + ret = ttm_bo_init_validate(nvbo->bo.bdev, &nvbo->bo, type, &nvbo->placement, align >> PAGE_SHIFT, &ctx, sg, robj, nouveau_bo_del_ttm); if (ret) { diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c index b42a657e4c2f..da3f76f508ea 100644 --- a/drivers/gpu/drm/qxl/qxl_object.c +++ b/drivers/gpu/drm/qxl/qxl_object.c @@ -141,7 +141,7 @@ int qxl_bo_create(struct qxl_device *qdev, unsigned long size, qxl_ttm_placement_from_domain(bo, domain);
bo->tbo.priority = priority; - r = ttm_bo_init_reserved(&qdev->mman.bdev, &bo->tbo, size, type, + r = ttm_bo_init_validate(&qdev->mman.bdev, &bo->tbo, type, &bo->placement, 0, &ctx, NULL, NULL, &qxl_ttm_bo_destroy); if (unlikely(r != 0)) { diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 1d414ff4ab0c..550ca056b3ac 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -204,7 +204,7 @@ int radeon_bo_create(struct radeon_device *rdev,
radeon_ttm_placement_from_domain(bo, domain); down_read(&rdev->pm.mclk_lock); - r = ttm_bo_init_reserved(&rdev->mman.bdev, &bo->tbo, size, type, + r = ttm_bo_init_validate(&rdev->mman.bdev, &bo->tbo, type, &bo->placement, page_align, &ctx, sg, resv, &radeon_ttm_bo_destroy); if (!r) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index e652055b5175..2b01cb30694a 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -915,36 +915,61 @@ int ttm_bo_validate(struct ttm_buffer_object *bo, } EXPORT_SYMBOL(ttm_bo_validate);
-int ttm_bo_init_reserved(struct ttm_device *bdev, - struct ttm_buffer_object *bo, - size_t size, - enum ttm_bo_type type, - struct ttm_placement *placement, - uint32_t page_alignment, - struct ttm_operation_ctx *ctx, - struct sg_table *sg, - struct dma_resv *resv, +/** + * ttm_bo_init_validate + * + * @bdev: Pointer to a ttm_device struct. + * @bo: Pointer to a ttm_buffer_object to be initialized. + * @type: Requested type of buffer object. + * @placement: Initial placement for buffer object. + * @alignment: Data alignment in pages. + * @ctx: TTM operation context for memory allocation. + * @sg: Scatter-gather table. + * @resv: Pointer to a dma_resv, or NULL to let ttm allocate one. + * @destroy: Destroy function. Use NULL for kfree(). + * + * This function initializes a pre-allocated struct ttm_buffer_object. + * As this object may be part of a larger structure, this function, + * together with the @destroy function, enables driver-specific objects + * derived from a ttm_buffer_object. + * + * On successful return, the caller owns an object kref to @bo. The kref and + * list_kref are usually set to 1, but note that in some situations, other + * tasks may already be holding references to @bo as well. + * Furthermore, if resv == NULL, the buffer's reservation lock will be held, + * and it is the caller's responsibility to call ttm_bo_unreserve. + * + * If a failure occurs, the function will call the @destroy function. Thus, + * after a failure, dereferencing @bo is illegal and will likely cause memory + * corruption. + * + * Returns + * -ENOMEM: Out of memory. + * -EINVAL: Invalid placement flags. + * -ERESTARTSYS: Interrupted by signal while sleeping waiting for resources. + */ +int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo, + enum ttm_bo_type type, struct ttm_placement *placement, + uint32_t alignment, struct ttm_operation_ctx *ctx, + struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *)) { static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; - bool locked; int ret;
- bo->destroy = destroy; kref_init(&bo->kref); INIT_LIST_HEAD(&bo->ddestroy); bo->bdev = bdev; bo->type = type; - bo->page_alignment = page_alignment; + bo->page_alignment = alignment; + bo->destroy = destroy; bo->pin_count = 0; bo->sg = sg; bo->bulk_move = NULL; - if (resv) { + if (resv) bo->base.resv = resv; - dma_resv_assert_held(bo->base.resv); - } else { + else bo->base.resv = &bo->base._resv; - } atomic_inc(&ttm_glob.bo_count);
ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource); @@ -957,33 +982,36 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, * For ttm_bo_type_device buffers, allocate * address space from the device. */ - if (bo->type == ttm_bo_type_device || - bo->type == ttm_bo_type_sg) + if (bo->type == ttm_bo_type_device || bo->type == ttm_bo_type_sg) { ret = drm_vma_offset_add(bdev->vma_manager, &bo->base.vma_node, - bo->resource->num_pages); + PFN_UP(bo->base.size)); + if (ret) + goto err_put; + }
/* passed reservation objects should already be locked, * since otherwise lockdep will be angered in radeon. */ - if (!resv) { - locked = dma_resv_trylock(bo->base.resv); - WARN_ON(!locked); - } + if (!resv) + WARN_ON(!dma_resv_trylock(bo->base.resv)); + else + dma_resv_assert_held(resv);
- if (likely(!ret)) - ret = ttm_bo_validate(bo, placement, ctx); + ret = ttm_bo_validate(bo, placement, ctx); + if (unlikely(ret)) + goto err_unlock;
- if (unlikely(ret)) { - if (!resv) - ttm_bo_unreserve(bo); + return 0;
- ttm_bo_put(bo); - return ret; - } +err_unlock: + if (!resv) + dma_resv_unlock(bo->base.resv);
+err_put: + ttm_bo_put(bo); return ret; } -EXPORT_SYMBOL(ttm_bo_init_reserved); +EXPORT_SYMBOL(ttm_bo_init_validate);
/* * buffer object vm functions. diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index c4f376d5e1d0..2bda298e51f0 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -429,9 +429,9 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size,
drm_gem_private_object_init(vdev, &bo->base, size);
- ret = ttm_bo_init_reserved(&dev_priv->bdev, bo, size, - ttm_bo_type_kernel, placement, 0, - &ctx, NULL, NULL, vmw_bo_default_destroy); + ret = ttm_bo_init_validate(&dev_priv->bdev, bo, ttm_bo_type_kernel, + placement, 0, &ctx, NULL, NULL, + vmw_bo_default_destroy); if (unlikely(ret)) goto error_free;
@@ -515,10 +515,8 @@ int vmw_bo_init(struct vmw_private *dev_priv, size = ALIGN(size, PAGE_SIZE); drm_gem_private_object_init(vdev, &vmw_bo->base.base, size);
- ret = ttm_bo_init_reserved(bdev, &vmw_bo->base, size, - ttm_bo_type_device, - placement, - 0, &ctx, NULL, NULL, bo_free); + ret = ttm_bo_init_validate(bdev, &vmw_bo->base, ttm_bo_type_device, + placement, 0, &ctx, NULL, NULL, bo_free); if (unlikely(ret)) { return ret; } diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 29384e2cb704..d3821a130d74 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -317,47 +317,9 @@ void ttm_bo_unlock_delayed_workqueue(struct ttm_device *bdev, int resched); bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo, const struct ttm_place *place);
-/** - * ttm_bo_init_reserved - * - * @bdev: Pointer to a ttm_device struct. - * @bo: Pointer to a ttm_buffer_object to be initialized. - * @size: Requested size of buffer object. - * @type: Requested type of buffer object. - * @placement: Initial placement for buffer object. - * @page_alignment: Data alignment in pages. - * @ctx: TTM operation context for memory allocation. - * @sg: Scatter-gather table. - * @resv: Pointer to a dma_resv, or NULL to let ttm allocate one. - * @destroy: Destroy function. Use NULL for kfree(). - * - * This function initializes a pre-allocated struct ttm_buffer_object. - * As this object may be part of a larger structure, this function, - * together with the @destroy function, - * enables driver-specific objects derived from a ttm_buffer_object. - * - * On successful return, the caller owns an object kref to @bo. The kref and - * list_kref are usually set to 1, but note that in some situations, other - * tasks may already be holding references to @bo as well. - * Furthermore, if resv == NULL, the buffer's reservation lock will be held, - * and it is the caller's responsibility to call ttm_bo_unreserve. - * - * If a failure occurs, the function will call the @destroy function, or - * kfree() if @destroy is NULL. Thus, after a failure, dereferencing @bo is - * illegal and will likely cause memory corruption. - * - * Returns - * -ENOMEM: Out of memory. - * -EINVAL: Invalid placement flags. - * -ERESTARTSYS: Interrupted by signal while sleeping waiting for resources. - */ - -int ttm_bo_init_reserved(struct ttm_device *bdev, - struct ttm_buffer_object *bo, - size_t size, enum ttm_bo_type type, - struct ttm_placement *placement, - uint32_t page_alignment, - struct ttm_operation_ctx *ctx, +int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo, + enum ttm_bo_type type, struct ttm_placement *placement, + uint32_t alignment, struct ttm_operation_ctx *ctx, struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *));
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Christian König Sent: Thursday, May 19, 2022 5:55 AM To: intel-gfx@lists.freedesktop.org Cc: matthew.william.auld@gmail.com; Christian König christian.koenig@amd.com; dri-devel@lists.freedesktop.org Subject: [PATCH 06/11] drm/ttm: rename and cleanup ttm_bo_init_reserved
Rename ttm_bo_init_reserved to ttm_bo_init_validate since that better matches what the function is actually doing.
Remove the unused size parameter, move the function's kerneldoc to the implementation and cleanup the whole error handling.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/drm_gem_vram_helper.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 5 +- drivers/gpu/drm/nouveau/nouveau_bo.c | 2 +- drivers/gpu/drm/qxl/qxl_object.c | 2 +- drivers/gpu/drm/radeon/radeon_object.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 92 ++++++++++++++-------- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 12 ++- include/drm/ttm/ttm_bo_api.h | 44 +---------- 9 files changed, 75 insertions(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 5444515c1476..116c8d31e646 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -590,7 +590,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev, if (!bp->destroy) bp->destroy = &amdgpu_bo_destroy;
- r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp-
type,
- r = ttm_bo_init_validate(&adev->mman.bdev, &bo->tbo, bp->type, &bo->placement, page_align, &ctx, NULL, bp->resv, bp->destroy); if (unlikely(r != 0))
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 7449cbc2f925..73e91baccea0 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -226,7 +226,7 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, * A failing ttm_bo_init will call ttm_buffer_object_destroy * to release gbo->bo.base and kfree gbo. */
- ret = ttm_bo_init_reserved(bdev, &gbo->bo, size,
ttm_bo_type_device,
- ret = ttm_bo_init_validate(bdev, &gbo->bo, ttm_bo_type_device, &gbo->placement, pg_align, &ctx, NULL,
NULL, ttm_buffer_object_destroy); if (ret) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index 4c25d9b2f138..253188da91eb 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -1229,9 +1229,8 @@ int __i915_gem_ttm_object_init(struct intel_memory_region *mem, * Similarly, in delayed_destroy, we can't call ttm_bo_put() * until successful initialization. */
- ret = ttm_bo_init_reserved(&i915->bdev, i915_gem_to_ttm(obj),
size,
bo_type, &i915_sys_placement,page_size >> PAGE_SHIFT,
- ret = ttm_bo_init_validate(&i915->bdev, i915_gem_to_ttm(obj),
bo_type,
&i915_sys_placement, page_size >>PAGE_SHIFT, &ctx, NULL, NULL, i915_ttm_bo_destroy); if (ret) return i915_ttm_err_to_gem(ret); diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 858b9382036c..666941804297 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -308,7 +308,7 @@ nouveau_bo_init(struct nouveau_bo *nvbo, u64 size, int align, u32 domain, nouveau_bo_placement_set(nvbo, domain, 0); INIT_LIST_HEAD(&nvbo->io_reserve_lru);
- ret = ttm_bo_init_reserved(nvbo->bo.bdev, &nvbo->bo, size, type,
- ret = ttm_bo_init_validate(nvbo->bo.bdev, &nvbo->bo, type, &nvbo->placement, align >> PAGE_SHIFT,
&ctx, sg, robj, nouveau_bo_del_ttm); if (ret) { diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c index b42a657e4c2f..da3f76f508ea 100644 --- a/drivers/gpu/drm/qxl/qxl_object.c +++ b/drivers/gpu/drm/qxl/qxl_object.c @@ -141,7 +141,7 @@ int qxl_bo_create(struct qxl_device *qdev, unsigned long size, qxl_ttm_placement_from_domain(bo, domain);
bo->tbo.priority = priority;
- r = ttm_bo_init_reserved(&qdev->mman.bdev, &bo->tbo, size, type,
- r = ttm_bo_init_validate(&qdev->mman.bdev, &bo->tbo, type, &bo->placement, 0, &ctx, NULL, NULL, &qxl_ttm_bo_destroy); if (unlikely(r != 0)) {
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 1d414ff4ab0c..550ca056b3ac 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -204,7 +204,7 @@ int radeon_bo_create(struct radeon_device *rdev,
radeon_ttm_placement_from_domain(bo, domain); down_read(&rdev->pm.mclk_lock);
- r = ttm_bo_init_reserved(&rdev->mman.bdev, &bo->tbo, size, type,
- r = ttm_bo_init_validate(&rdev->mman.bdev, &bo->tbo, type, &bo->placement, page_align, &ctx, sg, resv, &radeon_ttm_bo_destroy); if (!r)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index e652055b5175..2b01cb30694a 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -915,36 +915,61 @@ int ttm_bo_validate(struct ttm_buffer_object *bo, } EXPORT_SYMBOL(ttm_bo_validate);
-int ttm_bo_init_reserved(struct ttm_device *bdev,
struct ttm_buffer_object *bo,size_t size,enum ttm_bo_type type,struct ttm_placement *placement,uint32_t page_alignment,struct ttm_operation_ctx *ctx,struct sg_table *sg,struct dma_resv *resv,+/**
- ttm_bo_init_validate
- @bdev: Pointer to a ttm_device struct.
- @bo: Pointer to a ttm_buffer_object to be initialized.
- @type: Requested type of buffer object.
- @placement: Initial placement for buffer object.
- @alignment: Data alignment in pages.
- @ctx: TTM operation context for memory allocation.
- @sg: Scatter-gather table.
- @resv: Pointer to a dma_resv, or NULL to let ttm allocate one.
- @destroy: Destroy function. Use NULL for kfree().
- This function initializes a pre-allocated struct ttm_buffer_object.
- As this object may be part of a larger structure, this function,
- together with the @destroy function, enables driver-specific objects
- derived from a ttm_buffer_object.
- On successful return, the caller owns an object kref to @bo. The kref and
- list_kref are usually set to 1, but note that in some situations, other
- tasks may already be holding references to @bo as well.
- Furthermore, if resv == NULL, the buffer's reservation lock will be held,
- and it is the caller's responsibility to call ttm_bo_unreserve.
Ahh, ok, I see why the first couple of patches are doing this unreserve.
Umm still not sure why the caller needs to be responsible for this. Can you explain the reason?
- If a failure occurs, the function will call the @destroy function. Thus,
- after a failure, dereferencing @bo is illegal and will likely cause memory
- corruption.
- Returns
- -ENOMEM: Out of memory.
- -EINVAL: Invalid placement flags.
- -ERESTARTSYS: Interrupted by signal while sleeping waiting for resources.
- */
+int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo,
enum ttm_bo_type type, struct ttm_placement*placement,
uint32_t alignment, struct ttm_operation_ctx *ctx,struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *)){ static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM };
bool locked; int ret;
bo->destroy = destroy; kref_init(&bo->kref); INIT_LIST_HEAD(&bo->ddestroy); bo->bdev = bdev; bo->type = type;
bo->page_alignment = page_alignment;
- bo->page_alignment = alignment;
- bo->destroy = destroy; bo->pin_count = 0; bo->sg = sg; bo->bulk_move = NULL;
- if (resv) {
- if (resv) bo->base.resv = resv;
dma_resv_assert_held(bo->base.resv);- } else {
- else bo->base.resv = &bo->base._resv;
} atomic_inc(&ttm_glob.bo_count);
ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource);
@@ -957,33 +982,36 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, * For ttm_bo_type_device buffers, allocate * address space from the device. */
- if (bo->type == ttm_bo_type_device ||
bo->type == ttm_bo_type_sg)
- if (bo->type == ttm_bo_type_device || bo->type ==
ttm_bo_type_sg) { ret = drm_vma_offset_add(bdev->vma_manager, &bo-
base.vma_node,
bo->resource->num_pages);
PFN_UP(bo->base.size));if (ret)goto err_put;}
/* passed reservation objects should already be locked,
- since otherwise lockdep will be angered in radeon.
*/
- if (!resv) {
locked = dma_resv_trylock(bo->base.resv);WARN_ON(!locked);- }
- if (!resv)
WARN_ON(!dma_resv_trylock(bo->base.resv));- else
dma_resv_assert_held(resv);
- if (likely(!ret))
ret = ttm_bo_validate(bo, placement, ctx);
- ret = ttm_bo_validate(bo, placement, ctx);
- if (unlikely(ret))
goto err_unlock;
- if (unlikely(ret)) {
if (!resv)ttm_bo_unreserve(bo);
- return 0;
ttm_bo_put(bo);return ret;- }
+err_unlock:
- if (!resv)
dma_resv_unlock(bo->base.resv);
The original error path does a ttm_bo_unreserve, not an unlock.
Was that a bug in the original code?
Mike
+err_put:
- ttm_bo_put(bo); return ret;
} -EXPORT_SYMBOL(ttm_bo_init_reserved); +EXPORT_SYMBOL(ttm_bo_init_validate);
/*
- buffer object vm functions.
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index c4f376d5e1d0..2bda298e51f0 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -429,9 +429,9 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size,
drm_gem_private_object_init(vdev, &bo->base, size);
- ret = ttm_bo_init_reserved(&dev_priv->bdev, bo, size,
ttm_bo_type_kernel, placement, 0,&ctx, NULL, NULL,vmw_bo_default_destroy);
- ret = ttm_bo_init_validate(&dev_priv->bdev, bo,
ttm_bo_type_kernel,
placement, 0, &ctx, NULL, NULL,vmw_bo_default_destroy);@@ -515,10 +515,8 @@ int vmw_bo_init(struct vmw_private *dev_priv, size = ALIGN(size, PAGE_SIZE); drm_gem_private_object_init(vdev, &vmw_bo->base.base, size);
- ret = ttm_bo_init_reserved(bdev, &vmw_bo->base, size,
ttm_bo_type_device,placement,0, &ctx, NULL, NULL, bo_free);
- ret = ttm_bo_init_validate(bdev, &vmw_bo->base,
ttm_bo_type_device,
placement, 0, &ctx, NULL, NULL, bo_free);diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 29384e2cb704..d3821a130d74 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -317,47 +317,9 @@ void ttm_bo_unlock_delayed_workqueue(struct ttm_device *bdev, int resched); bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo, const struct ttm_place *place);
-/**
- ttm_bo_init_reserved
- @bdev: Pointer to a ttm_device struct.
- @bo: Pointer to a ttm_buffer_object to be initialized.
- @size: Requested size of buffer object.
- @type: Requested type of buffer object.
- @placement: Initial placement for buffer object.
- @page_alignment: Data alignment in pages.
- @ctx: TTM operation context for memory allocation.
- @sg: Scatter-gather table.
- @resv: Pointer to a dma_resv, or NULL to let ttm allocate one.
- @destroy: Destroy function. Use NULL for kfree().
- This function initializes a pre-allocated struct ttm_buffer_object.
- As this object may be part of a larger structure, this function,
- together with the @destroy function,
- enables driver-specific objects derived from a ttm_buffer_object.
- On successful return, the caller owns an object kref to @bo. The kref and
- list_kref are usually set to 1, but note that in some situations, other
- tasks may already be holding references to @bo as well.
- Furthermore, if resv == NULL, the buffer's reservation lock will be held,
- and it is the caller's responsibility to call ttm_bo_unreserve.
- If a failure occurs, the function will call the @destroy function, or
- kfree() if @destroy is NULL. Thus, after a failure, dereferencing @bo is
- illegal and will likely cause memory corruption.
- Returns
- -ENOMEM: Out of memory.
- -EINVAL: Invalid placement flags.
- -ERESTARTSYS: Interrupted by signal while sleeping waiting for resources.
- */
-int ttm_bo_init_reserved(struct ttm_device *bdev,
struct ttm_buffer_object *bo,size_t size, enum ttm_bo_type type,struct ttm_placement *placement,uint32_t page_alignment,struct ttm_operation_ctx *ctx,+int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo,
enum ttm_bo_type type, struct ttm_placement*placement,
uint32_t alignment, struct ttm_operation_ctx *ctx, struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *));-- 2.25.1
Make sure we can at least move and release BOs without backing store.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 116c8d31e646..5cf3a58bc925 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -1302,7 +1302,7 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo) if (bo->base.resv == &bo->base._resv) amdgpu_amdkfd_remove_fence_on_pt_pd_bos(abo);
- if (bo->resource->mem_type != TTM_PL_VRAM || + if (!bo->resource || bo->resource->mem_type != TTM_PL_VRAM || !(abo->flags & AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE) || adev->in_suspend || adev->shutdown) return; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index ec26edd4f4d8..b79c93812342 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -471,7 +471,8 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
adev = amdgpu_ttm_adev(bo->bdev);
- if (old_mem->mem_type == TTM_PL_SYSTEM && bo->ttm == NULL) { + if (!old_mem || (old_mem->mem_type == TTM_PL_SYSTEM && + bo->ttm == NULL)) { ttm_bo_move_null(bo, new_mem); goto out; }
Make sure we can at least move and release BOs without backing store.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/nouveau/nouveau_bo.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 666941804297..fb903c62d322 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -1010,7 +1010,8 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, }
/* Fake bo copy. */ - if (old_reg->mem_type == TTM_PL_SYSTEM && !bo->ttm) { + if (!old_reg || (old_reg->mem_type == TTM_PL_SYSTEM && + !bo->ttm)) { ttm_bo_move_null(bo, new_reg); goto out; }
Allow BOs to exist without backing store.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 2b01cb30694a..a55564c8b57c 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -117,12 +117,13 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, struct ttm_place *hop) { - struct ttm_resource_manager *old_man, *new_man; struct ttm_device *bdev = bo->bdev; + bool old_use_tt, new_use_tt; int ret;
- old_man = ttm_manager_type(bdev, bo->resource->mem_type); - new_man = ttm_manager_type(bdev, mem->mem_type); + old_use_tt = bo->resource && + ttm_manager_type(bdev, bo->resource->mem_type)->use_tt; + new_use_tt = ttm_manager_type(bdev, mem->mem_type)->use_tt;
ttm_bo_unmap_virtual(bo);
@@ -130,11 +131,11 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, * Create and bind a ttm if required. */
- if (new_man->use_tt) { + if (new_use_tt) { /* Zero init the new TTM structure if the old location should * have used one as well. */ - ret = ttm_tt_create(bo, old_man->use_tt); + ret = ttm_tt_create(bo, old_use_tt); if (ret) goto out_err;
@@ -160,8 +161,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, return 0;
out_err: - new_man = ttm_manager_type(bdev, bo->resource->mem_type); - if (!new_man->use_tt) + if (!old_use_tt) ttm_bo_tt_destroy(bo);
return ret; @@ -898,7 +898,7 @@ int ttm_bo_validate(struct ttm_buffer_object *bo, /* * Check whether we need to move buffer. */ - if (!ttm_resource_compat(bo->resource, placement)) { + if (!bo->resource || !ttm_resource_compat(bo->resource, placement)) { ret = ttm_bo_move_buffer(bo, placement, ctx); if (ret) return ret;
-----Original Message----- From: dri-devel dri-devel-bounces@lists.freedesktop.org On Behalf Of Christian König Sent: Thursday, May 19, 2022 5:55 AM To: intel-gfx@lists.freedesktop.org Cc: matthew.william.auld@gmail.com; Christian König christian.koenig@amd.com; dri-devel@lists.freedesktop.org Subject: [PATCH 09/11] drm/ttm: audit bo->resource usage
Allow BOs to exist without backing store.
Took me a while to figure out that only the last line is related to this commit message.
Could you add something like:
Refactor usage information.
Allow BOs to exist without backing store.
?
Would make this patch a little easier to decipher.
M
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/ttm/ttm_bo.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 2b01cb30694a..a55564c8b57c 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -117,12 +117,13 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx, struct ttm_place *hop) {
- struct ttm_resource_manager *old_man, *new_man; struct ttm_device *bdev = bo->bdev;
- bool old_use_tt, new_use_tt; int ret;
- old_man = ttm_manager_type(bdev, bo->resource->mem_type);
- new_man = ttm_manager_type(bdev, mem->mem_type);
- old_use_tt = bo->resource &&
ttm_manager_type(bdev, bo->resource->mem_type)-use_tt;
new_use_tt = ttm_manager_type(bdev, mem->mem_type)->use_tt;
ttm_bo_unmap_virtual(bo);
@@ -130,11 +131,11 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, * Create and bind a ttm if required. */
- if (new_man->use_tt) {
- if (new_use_tt) { /* Zero init the new TTM structure if the old location should
*/
- have used one as well.
ret = ttm_tt_create(bo, old_man->use_tt);
ret = ttm_tt_create(bo, old_use_tt);@@ -160,8 +161,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, return 0;
out_err:
- new_man = ttm_manager_type(bdev, bo->resource->mem_type);
- if (!new_man->use_tt)
if (!old_use_tt) ttm_bo_tt_destroy(bo);
return ret;
@@ -898,7 +898,7 @@ int ttm_bo_validate(struct ttm_buffer_object *bo, /* * Check whether we need to move buffer. */
- if (!ttm_resource_compat(bo->resource, placement)) {
- if (!bo->resource || !ttm_resource_compat(bo->resource,
placement)) { ret = ttm_bo_move_buffer(bo, placement, ctx); if (ret) return ret; -- 2.25.1
That should not be necessary any more when drivers should at least be able to handle the move without a resource.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index a55564c8b57c..31aa4b040d1e 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -954,7 +954,6 @@ int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo, struct sg_table *sg, struct dma_resv *resv, void (*destroy) (struct ttm_buffer_object *)) { - static const struct ttm_place sys_mem = { .mem_type = TTM_PL_SYSTEM }; int ret;
kref_init(&bo->kref); @@ -972,12 +971,6 @@ int ttm_bo_init_validate(struct ttm_device *bdev, struct ttm_buffer_object *bo, bo->base.resv = &bo->base._resv; atomic_inc(&ttm_glob.bo_count);
- ret = ttm_resource_alloc(bo, &sys_mem, &bo->resource); - if (unlikely(ret)) { - ttm_bo_put(bo); - return ret; - } - /* * For ttm_bo_type_device buffers, allocate * address space from the device.
Greeting,
FYI, we noticed the following commit (built with gcc-11):
commit: 47e8c5019b432cfa8f875fe068254b8d4e2dd3c6 ("[PATCH 10/11] drm/ttm: stop allocating dummy resources during BO creation") url: https://github.com/intel-lab-lkp/linux/commits/Christian-K-nig/drm-radeon-sw... base: git://anongit.freedesktop.org/drm/drm-tip drm-tip patch link: https://lore.kernel.org/intel-gfx/20220519095508.115203-11-christian.koenig@...
in testcase: boot
on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
+---------------------------------------------+------------+------------+ | | abd7815e9a | 47e8c5019b | +---------------------------------------------+------------+------------+ | boot_successes | 36 | 0 | | boot_failures | 0 | 36 | | BUG:kernel_NULL_pointer_dereference,address | 0 | 36 | | Oops:#[##] | 0 | 36 | | RIP:ttm_bo_move_memcpy[ttm] | 0 | 36 | | Kernel_panic-not_syncing:Fatal_exception | 0 | 36 | +---------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag Reported-by: kernel test robot oliver.sang@intel.com
[ 9.741361][ T197] BUG: kernel NULL pointer dereference, address: 0000000000000010 [ 9.742532][ T197] #PF: supervisor read access in kernel mode [ 9.743345][ T197] #PF: error_code(0x0000) - not-present page [ 9.744148][ T197] PGD 0 P4D 0 [ 9.744608][ T197] Oops: 0000 [#1] SMP PTI [ 9.745191][ T197] CPU: 0 PID: 197 Comm: systemd-udevd Not tainted 5.18.0-rc7-01546-g47e8c5019b43 #1 [ 9.746444][ T197] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-debian-1.16.0-4 04/01/2014 [ 9.747818][ T197] RIP: 0010:ttm_bo_move_memcpy (include/drm/ttm/ttm_device.h:285 drivers/gpu/drm/ttm/ttm_bo_util.c:141) ttm [ 9.748653][ T197] Code: 01 00 00 65 48 8b 04 25 28 00 00 00 48 89 44 24 50 31 c0 48 63 42 10 4d 8b bc c6 90 00 00 00 48 8b 87 58 01 00 00 48 89 04 24 <48> 63 40 10 49 8b 84 c6 90 00 00 00 48 89 44 24 08 4d 85 ed 74 16 All code ======== 0: 01 00 add %eax,(%rax) 2: 00 65 48 add %ah,0x48(%rbp) 5: 8b 04 25 28 00 00 00 mov 0x28,%eax c: 48 89 44 24 50 mov %rax,0x50(%rsp) 11: 31 c0 xor %eax,%eax 13: 48 63 42 10 movslq 0x10(%rdx),%rax 17: 4d 8b bc c6 90 00 00 mov 0x90(%r14,%rax,8),%r15 1e: 00 1f: 48 8b 87 58 01 00 00 mov 0x158(%rdi),%rax 26: 48 89 04 24 mov %rax,(%rsp) 2a:* 48 63 40 10 movslq 0x10(%rax),%rax <-- trapping instruction 2e: 49 8b 84 c6 90 00 00 mov 0x90(%r14,%rax,8),%rax 35: 00 36: 48 89 44 24 08 mov %rax,0x8(%rsp) 3b: 4d 85 ed test %r13,%r13 3e: 74 16 je 0x56
Code starting with the faulting instruction =========================================== 0: 48 63 40 10 movslq 0x10(%rax),%rax 4: 49 8b 84 c6 90 00 00 mov 0x90(%r14,%rax,8),%rax b: 00 c: 48 89 44 24 08 mov %rax,0x8(%rsp) 11: 4d 85 ed test %r13,%r13 14: 74 16 je 0x2c [ 9.751280][ T197] RSP: 0000:ffffa9e28060b880 EFLAGS: 00010246 [ 9.752060][ T197] RAX: 0000000000000000 RBX: ffff9781d1227010 RCX: ffff9781cb56aba0 [ 9.753116][ T197] RDX: ffff9781cb56aba0 RSI: ffffa9e28060b9e8 RDI: ffff978180874200 [ 9.754196][ T197] RBP: ffff978180874200 R08: ffffa9e28060b968 R09: ffff9781d2aa8000 [ 9.755274][ T197] R10: 00000000001003e8 R11: 0000000000000000 R12: ffff9781cb56aba0 [ 9.756329][ T197] R13: ffff9781d2e51780 R14: ffff9781d1227010 R15: ffff9781d1227028 [ 9.757385][ T197] FS: 00007f0da77c1d40(0000) GS:ffff9784afc00000(0000) knlGS:0000000000000000 [ 9.758553][ T197] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 9.759459][ T197] CR2: 0000000000000010 CR3: 000000042fd9a000 CR4: 00000000000406f0 [ 9.760463][ T197] Call Trace: [ 9.760909][ T197] <TASK> [ 9.761302][ T197] ? kvmalloc_node (mm/util.c:586) [ 9.761951][ T197] ttm_bo_handle_move_mem (drivers/gpu/drm/ttm/ttm_bo.c:154) ttm [ 9.762735][ T197] ttm_bo_validate (drivers/gpu/drm/ttm/ttm_bo.c:871 drivers/gpu/drm/ttm/ttm_bo.c:902) ttm [ 9.763462][ T197] ? drm_vma_offset_add (include/drm/drm_mm.h:439 include/drm/drm_mm.h:462 drivers/gpu/drm/drm_vma_manager.c:209) drm [ 9.764276][ T197] ttm_bo_init_validate (drivers/gpu/drm/ttm/ttm_bo.c:994) ttm [ 9.765052][ T197] drm_gem_vram_create (drivers/gpu/drm/drm_gem_vram_helper.c:232) drm_vram_helper [ 9.765932][ T197] ? bo_driver_evict_flags (drivers/gpu/drm/drm_gem_vram_helper.c:131) drm_vram_helper [ 9.766866][ T197] drm_gem_vram_fill_create_dumb (drivers/gpu/drm/drm_gem_vram_helper.c:526) drm_vram_helper [ 9.767857][ T197] drm_client_framebuffer_create (drivers/gpu/drm/drm_client.c:269 drivers/gpu/drm/drm_client.c:419) drm [ 9.768773][ T197] drm_fb_helper_generic_probe (drivers/gpu/drm/drm_fb_helper.c:2375 (discriminator 4)) drm_kms_helper [ 9.769762][ T197] drm_fb_helper_single_fb_probe (drivers/gpu/drm/drm_fb_helper.c:1706) drm_kms_helper [ 9.770775][ T197] __drm_fb_helper_initial_config_and_unlock (drivers/gpu/drm/drm_fb_helper.c:1870) drm_kms_helper [ 9.771940][ T197] drm_fbdev_client_hotplug (drivers/gpu/drm/drm_fb_helper.c:1964 drivers/gpu/drm/drm_fb_helper.c:1956 drivers/gpu/drm/drm_fb_helper.c:2478) drm_kms_helper [ 9.772899][ T197] drm_fbdev_generic_setup (drivers/gpu/drm/drm_fb_helper.c:2565) drm_kms_helper [ 9.773835][ T197] bochs_pci_probe (drivers/gpu/drm/tiny/bochs.c:667 drivers/gpu/drm/tiny/bochs.c:632) bochs [ 9.774572][ T197] local_pci_probe (drivers/pci/pci-driver.c:323) [ 9.777192][ T197] pci_call_probe (drivers/pci/pci-driver.c:391) [ 9.777890][ T197] ? kernfs_create_link (fs/kernfs/symlink.c:48) [ 9.778587][ T197] pci_device_probe (drivers/pci/pci-driver.c:460) [ 9.779279][ T197] really_probe (drivers/base/dd.c:542 drivers/base/dd.c:621) [ 9.779889][ T197] __driver_probe_device (drivers/base/dd.c:752) [ 9.780598][ T197] driver_probe_device (drivers/base/dd.c:782) [ 9.781283][ T197] __driver_attach (drivers/base/dd.c:1142) [ 9.781924][ T197] ? __device_attach_driver (drivers/base/dd.c:1094) [ 9.782704][ T197] ? __device_attach_driver (drivers/base/dd.c:1094) [ 9.783436][ T197] bus_for_each_dev (drivers/base/bus.c:301) [ 9.783998][ T197] bus_add_driver (drivers/base/bus.c:619) [ 9.784631][ T197] driver_register (drivers/base/driver.c:171) [ 9.785273][ T197] ? 0xffffffffc0532000 [ 9.785837][ T197] do_one_initcall (init/main.c:1298) [ 9.786481][ T197] ? kmem_cache_alloc_trace (mm/slub.c:3219 mm/slub.c:3225 mm/slub.c:3256) [ 9.787268][ T197] do_init_module (kernel/module.c:3731) [ 9.787931][ T197] __do_sys_finit_module (kernel/module.c:4222) [ 9.788650][ T197] do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80) [ 9.789258][ T197] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:115) [ 9.790062][ T197] RIP: 0033:0x7f0da7fabf59 [ 9.790654][ T197] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48 All code ======== 0: 00 c3 add %al,%bl 2: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) 9: 00 00 00 c: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) 11: 48 89 f8 mov %rdi,%rax 14: 48 89 f7 mov %rsi,%rdi 17: 48 89 d6 mov %rdx,%rsi 1a: 48 89 ca mov %rcx,%rdx 1d: 4d 89 c2 mov %r8,%r10 20: 4d 89 c8 mov %r9,%r8 23: 4c 8b 4c 24 08 mov 0x8(%rsp),%r9 28: 0f 05 syscall 2a:* 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax <-- trapping instruction 30: 73 01 jae 0x33 32: c3 retq 33: 48 8b 0d 07 6f 0c 00 mov 0xc6f07(%rip),%rcx # 0xc6f41 3a: f7 d8 neg %eax 3c: 64 89 01 mov %eax,%fs:(%rcx) 3f: 48 rex.W
Code starting with the faulting instruction =========================================== 0: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax 6: 73 01 jae 0x9 8: c3 retq 9: 48 8b 0d 07 6f 0c 00 mov 0xc6f07(%rip),%rcx # 0xc6f17 10: f7 d8 neg %eax 12: 64 89 01 mov %eax,%fs:(%rcx) 15: 48 rex.W
To reproduce:
# build kernel cd linux cp config-5.18.0-rc7-01546-g47e8c5019b43 .config make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage modules make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 INSTALL_MOD_PATH=<mod-install-dir> modules_install cd <mod-install-dir> find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz
git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email
# if come across any failure that blocks the test, # please remove ~/.lkp and /lkp dir to run from a clean state.
That should not be necessary any more when drivers should at least be able to handle a move without a resource.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/ttm/ttm_bo_util.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 1cbfb00c1d65..585fc529cc75 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -600,16 +600,10 @@ EXPORT_SYMBOL(ttm_bo_move_sync_cleanup); */ 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;
- ret = ttm_resource_alloc(bo, &sys_mem, &sys_res); - if (ret) - return ret; - /* If already idle, no need for ghost object dance. */ ret = ttm_bo_wait(bo, false, true); if (ret != -EBUSY) { @@ -617,14 +611,13 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) /* See comment below about clearing. */ ret = ttm_tt_create(bo, true); if (ret) - goto error_free_sys_mem; + return ret; } else { ttm_tt_unpopulate(bo->bdev, bo->ttm); if (bo->type == ttm_bo_type_device) ttm_tt_mark_for_clear(bo->ttm); } ttm_resource_free(bo, &bo->resource); - ttm_bo_assign_mem(bo, sys_res); return 0; }
@@ -641,7 +634,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) ret = ttm_tt_create(bo, true); swap(bo->ttm, ttm); if (ret) - goto error_free_sys_mem; + return ret;
ret = ttm_buffer_object_transfer(bo, &ghost); if (ret) @@ -655,13 +648,9 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo) dma_resv_unlock(&ghost->base._resv); ttm_bo_put(ghost); bo->ttm = ttm; - ttm_bo_assign_mem(bo, sys_res); return 0;
error_destroy_tt: ttm_tt_destroy(bo->bdev, ttm); - -error_free_sys_mem: - ttm_resource_free(bo, &sys_res); return ret; }
On Thu, 19 May 2022 at 10:55, Christian König ckoenig.leichtzumerken@gmail.com wrote:
Just sending that out once more to intel-gfx to let the CI systems take a look.
If all went well it should normally appear at [1][2], if CI was able to pick up the series.
Since it's not currently there, I assume it's temporarily stuck in the moderation queue, assuming you are not subscribed to intel-gfx ml? If so, perhaps consider subscribing at [3] and then disable receiving any mail from the ml, so you get full use of CI without getting spammed.
[1] https://intel-gfx-ci.01.org/queue/index.html [2] https://patchwork.freedesktop.org/project/intel-gfx/series/ [3] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
No functional change compared to the last version.
Christian.
Am 19.05.22 um 12:50 schrieb Matthew Auld:
On Thu, 19 May 2022 at 10:55, Christian König ckoenig.leichtzumerken@gmail.com wrote:
Just sending that out once more to intel-gfx to let the CI systems take a look.
If all went well it should normally appear at [1][2], if CI was able to pick up the series.
Since it's not currently there, I assume it's temporarily stuck in the moderation queue, assuming you are not subscribed to intel-gfx ml?
Ah! Well I am subscribed, just not with the e-Mail address I use to send out those patches.
Going to fix that ASAP!
Thanks, Christian.
If so, perhaps consider subscribing at [3] and then disable receiving any mail from the ml, so you get full use of CI without getting spammed.
[1] https://intel-gfx-ci.01.org/queue/index.html [2] https://patchwork.freedesktop.org/project/intel-gfx/series/ [3] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
No functional change compared to the last version.
Christian.
dri-devel@lists.freedesktop.org
-
Christian König -
Christian König -
kernel test robot -
Matthew Auld -
Ruhl, Michael J