* [PATCH 2/5] drm/amdgpu: unbind in amdgpu_ttm_tt_unpopulate
2021-07-28 13:05 [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate Christian König
@ 2021-07-28 13:05 ` Christian König
2021-07-28 13:05 ` [PATCH 3/5] drm/nouveau: unbind in nouveau_ttm_tt_unpopulate Christian König
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2021-07-28 13:05 UTC (permalink / raw)
To: daniel.vetter, airlied; +Cc: dri-devel
Doing this in amdgpu_ttm_backend_destroy() is to late.
It turned out that this is not a good idea at all because it leaves pointers
to freed up system memory pages in the GART tables of the drivers.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index acd95d3a4434..2a57076c5233 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1066,7 +1066,6 @@ static void amdgpu_ttm_backend_destroy(struct ttm_device *bdev,
{
struct amdgpu_ttm_tt *gtt = (void *)ttm;
- amdgpu_ttm_backend_unbind(bdev, ttm);
ttm_tt_destroy_common(bdev, ttm);
if (gtt->usertask)
put_task_struct(gtt->usertask);
@@ -1148,6 +1147,8 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_device *bdev,
struct amdgpu_ttm_tt *gtt = (void *)ttm;
struct amdgpu_device *adev;
+ amdgpu_ttm_backend_unbind(bdev, ttm);
+
if (gtt && gtt->userptr) {
amdgpu_ttm_tt_set_user_pages(ttm, NULL);
kfree(ttm->sg);
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/5] drm/nouveau: unbind in nouveau_ttm_tt_unpopulate
2021-07-28 13:05 [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate Christian König
2021-07-28 13:05 ` [PATCH 2/5] drm/amdgpu: unbind in amdgpu_ttm_tt_unpopulate Christian König
@ 2021-07-28 13:05 ` Christian König
2021-07-28 13:05 ` [PATCH 4/5] drm/radeon: unbind in radeon_ttm_tt_unpopulate() Christian König
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2021-07-28 13:05 UTC (permalink / raw)
To: daniel.vetter, airlied; +Cc: dri-devel
Doing this in nouveau_ttm_tt_destroy()/nouveau_sgdma_destroy() is to late.
It turned out that this is not a good idea at all because it leaves pointers
to freed up system memory pages in the GART tables of the drivers.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/nouveau/nouveau_bo.c | 3 ++-
drivers/gpu/drm/nouveau/nouveau_sgdma.c | 1 -
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 6d07e653f82d..c33a56c2f068 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1277,6 +1277,8 @@ nouveau_ttm_tt_unpopulate(struct ttm_device *bdev,
if (slave)
return;
+ nouveau_ttm_tt_unbind(bdev, ttm);
+
drm = nouveau_bdev(bdev);
dev = drm->dev->dev;
@@ -1290,7 +1292,6 @@ nouveau_ttm_tt_destroy(struct ttm_device *bdev,
#if IS_ENABLED(CONFIG_AGP)
struct nouveau_drm *drm = nouveau_bdev(bdev);
if (drm->agp.bridge) {
- ttm_agp_unbind(ttm);
ttm_tt_destroy_common(bdev, ttm);
ttm_agp_destroy(ttm);
return;
diff --git a/drivers/gpu/drm/nouveau/nouveau_sgdma.c b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
index 256ec5b35473..bde92a9dae7a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
@@ -21,7 +21,6 @@ nouveau_sgdma_destroy(struct ttm_device *bdev, struct ttm_tt *ttm)
struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)ttm;
if (ttm) {
- nouveau_sgdma_unbind(bdev, ttm);
ttm_tt_destroy_common(bdev, ttm);
ttm_tt_fini(&nvbe->ttm);
kfree(nvbe);
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/5] drm/radeon: unbind in radeon_ttm_tt_unpopulate()
2021-07-28 13:05 [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate Christian König
2021-07-28 13:05 ` [PATCH 2/5] drm/amdgpu: unbind in amdgpu_ttm_tt_unpopulate Christian König
2021-07-28 13:05 ` [PATCH 3/5] drm/nouveau: unbind in nouveau_ttm_tt_unpopulate Christian König
@ 2021-07-28 13:05 ` Christian König
2021-07-28 13:05 ` [PATCH 5/5] drm/ttm: remove ttm_tt_destroy_common v2 Christian König
2021-08-23 11:05 ` [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate Christian König
4 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2021-07-28 13:05 UTC (permalink / raw)
To: daniel.vetter, airlied; +Cc: dri-devel
Doing this in radeon_ttm_tt_destroy() is to late.
It turned out that this is not a good idea at all because it leaves pointers
to freed up system memory pages in the GART tables of the drivers.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/radeon/radeon_ttm.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index a06d4cc2fb1c..ee343b76db54 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -488,9 +488,7 @@ static void radeon_ttm_backend_destroy(struct ttm_device *bdev, struct ttm_tt *t
{
struct radeon_ttm_tt *gtt = (void *)ttm;
- radeon_ttm_backend_unbind(bdev, ttm);
ttm_tt_destroy_common(bdev, ttm);
-
ttm_tt_fini(>t->ttm);
kfree(gtt);
}
@@ -574,6 +572,8 @@ static void radeon_ttm_tt_unpopulate(struct ttm_device *bdev, struct ttm_tt *ttm
struct radeon_ttm_tt *gtt = radeon_ttm_tt_to_gtt(rdev, ttm);
bool slave = !!(ttm->page_flags & TTM_PAGE_FLAG_SG);
+ radeon_ttm_tt_unbind(bdev, ttm);
+
if (gtt && gtt->userptr) {
kfree(ttm->sg);
ttm->page_flags &= ~TTM_PAGE_FLAG_SG;
@@ -651,7 +651,6 @@ static void radeon_ttm_tt_destroy(struct ttm_device *bdev,
struct radeon_device *rdev = radeon_get_rdev(bdev);
if (rdev->flags & RADEON_IS_AGP) {
- ttm_agp_unbind(ttm);
ttm_tt_destroy_common(bdev, ttm);
ttm_agp_destroy(ttm);
return;
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/5] drm/ttm: remove ttm_tt_destroy_common v2
2021-07-28 13:05 [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate Christian König
` (2 preceding siblings ...)
2021-07-28 13:05 ` [PATCH 4/5] drm/radeon: unbind in radeon_ttm_tt_unpopulate() Christian König
@ 2021-07-28 13:05 ` Christian König
2021-08-23 11:05 ` [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate Christian König
4 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2021-07-28 13:05 UTC (permalink / raw)
To: daniel.vetter, airlied; +Cc: dri-devel
Move the functionality into ttm_tt_fini and ttm_bo_tt_destroy instead.
We don't need this any more since we removed the unbind from the destroy
code paths in the drivers.
Also add a warning to ttm_tt_fini() if we try to fini a still populated TT
object.
v2: instead of reverting the patch move the functionality to different
places.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 -
drivers/gpu/drm/drm_gem_vram_helper.c | 1 -
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 1 -
drivers/gpu/drm/nouveau/nouveau_bo.c | 1 -
drivers/gpu/drm/nouveau/nouveau_sgdma.c | 1 -
drivers/gpu/drm/qxl/qxl_ttm.c | 1 -
drivers/gpu/drm/radeon/radeon_ttm.c | 2 --
drivers/gpu/drm/ttm/ttm_bo.c | 1 +
drivers/gpu/drm/ttm/ttm_tt.c | 17 ++++++-----------
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 1 -
include/drm/ttm/ttm_tt.h | 7 -------
11 files changed, 7 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 2a57076c5233..3e89a26cb63d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1066,7 +1066,6 @@ static void amdgpu_ttm_backend_destroy(struct ttm_device *bdev,
{
struct amdgpu_ttm_tt *gtt = (void *)ttm;
- ttm_tt_destroy_common(bdev, ttm);
if (gtt->usertask)
put_task_struct(gtt->usertask);
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 1e9b82e51a07..cc81fbac1a13 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -846,7 +846,6 @@ static const struct drm_gem_object_funcs drm_gem_vram_object_funcs = {
static void bo_driver_ttm_tt_destroy(struct ttm_device *bdev, struct ttm_tt *tt)
{
- ttm_tt_destroy_common(bdev, tt);
ttm_tt_fini(tt);
kfree(tt);
}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index bf33724bed5c..e646aac9d7a4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -118,7 +118,6 @@ static void i915_ttm_tt_destroy(struct ttm_device *bdev, struct ttm_tt *ttm)
{
struct i915_ttm_tt *i915_tt = container_of(ttm, typeof(*i915_tt), ttm);
- ttm_tt_destroy_common(bdev, ttm);
kfree(i915_tt);
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index c33a56c2f068..33dca2565cca 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1292,7 +1292,6 @@ nouveau_ttm_tt_destroy(struct ttm_device *bdev,
#if IS_ENABLED(CONFIG_AGP)
struct nouveau_drm *drm = nouveau_bdev(bdev);
if (drm->agp.bridge) {
- ttm_tt_destroy_common(bdev, ttm);
ttm_agp_destroy(ttm);
return;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_sgdma.c b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
index bde92a9dae7a..85c03c83259b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
@@ -21,7 +21,6 @@ nouveau_sgdma_destroy(struct ttm_device *bdev, struct ttm_tt *ttm)
struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)ttm;
if (ttm) {
- ttm_tt_destroy_common(bdev, ttm);
ttm_tt_fini(&nvbe->ttm);
kfree(nvbe);
}
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 37a1b6a6ad6d..b2e33d5ba5d0 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -101,7 +101,6 @@ int qxl_ttm_io_mem_reserve(struct ttm_device *bdev,
*/
static void qxl_ttm_backend_destroy(struct ttm_device *bdev, struct ttm_tt *ttm)
{
- ttm_tt_destroy_common(bdev, ttm);
ttm_tt_fini(ttm);
kfree(ttm);
}
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index ee343b76db54..7793249bc549 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -488,7 +488,6 @@ static void radeon_ttm_backend_destroy(struct ttm_device *bdev, struct ttm_tt *t
{
struct radeon_ttm_tt *gtt = (void *)ttm;
- ttm_tt_destroy_common(bdev, ttm);
ttm_tt_fini(>t->ttm);
kfree(gtt);
}
@@ -651,7 +650,6 @@ static void radeon_ttm_tt_destroy(struct ttm_device *bdev,
struct radeon_device *rdev = radeon_get_rdev(bdev);
if (rdev->flags & RADEON_IS_AGP) {
- ttm_tt_destroy_common(bdev, ttm);
ttm_agp_destroy(ttm);
return;
}
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index ea4add2b9717..49f4bc97c35a 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1224,6 +1224,7 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
if (bo->ttm == NULL)
return;
+ ttm_tt_unpopulate(bo->bdev, bo->ttm);
ttm_tt_destroy(bo->bdev, bo->ttm);
bo->ttm = NULL;
}
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 24031a8acd2d..506b3c926a68 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -123,17 +123,6 @@ static int ttm_sg_tt_alloc_page_directory(struct ttm_tt *ttm)
return 0;
}
-void ttm_tt_destroy_common(struct ttm_device *bdev, struct ttm_tt *ttm)
-{
- ttm_tt_unpopulate(bdev, ttm);
-
- if (ttm->swap_storage)
- fput(ttm->swap_storage);
-
- ttm->swap_storage = NULL;
-}
-EXPORT_SYMBOL(ttm_tt_destroy_common);
-
void ttm_tt_destroy(struct ttm_device *bdev, struct ttm_tt *ttm)
{
bdev->funcs->ttm_tt_destroy(bdev, ttm);
@@ -168,6 +157,12 @@ EXPORT_SYMBOL(ttm_tt_init);
void ttm_tt_fini(struct ttm_tt *ttm)
{
+ WARN_ON(ttm->page_flags & TTM_PAGE_FLAG_PRIV_POPULATED);
+
+ if (ttm->swap_storage)
+ fput(ttm->swap_storage);
+ ttm->swap_storage = NULL;
+
if (ttm->pages)
kvfree(ttm->pages);
else
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 904031d03dbe..f35bdc1cb197 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -526,7 +526,6 @@ static void vmw_ttm_destroy(struct ttm_device *bdev, struct ttm_tt *ttm)
struct vmw_ttm_tt *vmw_be =
container_of(ttm, struct vmw_ttm_tt, dma_ttm);
- ttm_tt_destroy_common(bdev, ttm);
vmw_ttm_unmap_dma(vmw_be);
ttm_tt_fini(ttm);
if (vmw_be->mob)
diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
index 818680c6a8ed..e402dab1d0f6 100644
--- a/include/drm/ttm/ttm_tt.h
+++ b/include/drm/ttm/ttm_tt.h
@@ -134,13 +134,6 @@ void ttm_tt_fini(struct ttm_tt *ttm);
*/
void ttm_tt_destroy(struct ttm_device *bdev, struct ttm_tt *ttm);
-/**
- * ttm_tt_destroy_common:
- *
- * Called from driver to destroy common path.
- */
-void ttm_tt_destroy_common(struct ttm_device *bdev, struct ttm_tt *ttm);
-
/**
* ttm_tt_swapin:
*
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate
2021-07-28 13:05 [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate Christian König
` (3 preceding siblings ...)
2021-07-28 13:05 ` [PATCH 5/5] drm/ttm: remove ttm_tt_destroy_common v2 Christian König
@ 2021-08-23 11:05 ` Christian König
2021-08-23 11:15 ` Thomas Hellström
4 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2021-08-23 11:05 UTC (permalink / raw)
To: daniel.vetter, airlied; +Cc: dri-devel, Thomas Hellström
Adding Thomas on CC as well.
Just a gentle ping. I think the patch set makes sense now.
Regards,
Christian.
Am 28.07.21 um 15:05 schrieb Christian König:
> Doing this in vmw_ttm_destroy() is to late.
>
> It turned out that this is not a good idea at all because it leaves pointers
> to freed up system memory pages in the GART tables of the drivers.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index b0973c27e774..904031d03dbe 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -526,14 +526,9 @@ static void vmw_ttm_destroy(struct ttm_device *bdev, struct ttm_tt *ttm)
> struct vmw_ttm_tt *vmw_be =
> container_of(ttm, struct vmw_ttm_tt, dma_ttm);
>
> - vmw_ttm_unbind(bdev, ttm);
> ttm_tt_destroy_common(bdev, ttm);
> vmw_ttm_unmap_dma(vmw_be);
> - if (vmw_be->dev_priv->map_mode == vmw_dma_alloc_coherent)
> - ttm_tt_fini(&vmw_be->dma_ttm);
> - else
> - ttm_tt_fini(ttm);
> -
> + ttm_tt_fini(ttm);
> if (vmw_be->mob)
> vmw_mob_destroy(vmw_be->mob);
>
> @@ -578,6 +573,8 @@ static void vmw_ttm_unpopulate(struct ttm_device *bdev,
> dma_ttm);
> unsigned int i;
>
> + vmw_ttm_unbind(bdev, ttm);
> +
> if (vmw_tt->mob) {
> vmw_mob_destroy(vmw_tt->mob);
> vmw_tt->mob = NULL;
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate
2021-08-23 11:05 ` [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate Christian König
@ 2021-08-23 11:15 ` Thomas Hellström
2021-08-26 8:49 ` Daniel Vetter
0 siblings, 1 reply; 12+ messages in thread
From: Thomas Hellström @ 2021-08-23 11:15 UTC (permalink / raw)
To: Christian König, daniel.vetter, airlied; +Cc: dri-devel
On Mon, 2021-08-23 at 13:05 +0200, Christian König wrote:
> Adding Thomas on CC as well.
>
> Just a gentle ping. I think the patch set makes sense now.
>
> Regards,
> Christian.
>
> Am 28.07.21 um 15:05 schrieb Christian König:
> > Doing this in vmw_ttm_destroy() is to late.
> >
> > It turned out that this is not a good idea at all because it leaves
> > pointers
> > to freed up system memory pages in the GART tables of the drivers.
> >
> > Signed-off-by: Christian König <christian.koenig@amd.com>
> > ---
> > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 9 +++------
> > 1 file changed, 3 insertions(+), 6 deletions(-)
> >
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate
2021-08-23 11:15 ` Thomas Hellström
@ 2021-08-26 8:49 ` Daniel Vetter
2021-08-26 10:11 ` Christian König
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2021-08-26 8:49 UTC (permalink / raw)
To: Thomas Hellström
Cc: Christian König, daniel.vetter, airlied, dri-devel
On Mon, Aug 23, 2021 at 01:15:20PM +0200, Thomas Hellström wrote:
> On Mon, 2021-08-23 at 13:05 +0200, Christian König wrote:
> > Adding Thomas on CC as well.
> >
> > Just a gentle ping. I think the patch set makes sense now.
> >
> > Regards,
> > Christian.
> >
> > Am 28.07.21 um 15:05 schrieb Christian König:
> > > Doing this in vmw_ttm_destroy() is to late.
> > >
> > > It turned out that this is not a good idea at all because it leaves
> > > pointers
> > > to freed up system memory pages in the GART tables of the drivers.
> > >
> > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > ---
> > > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 9 +++------
> > > 1 file changed, 3 insertions(+), 6 deletions(-)
> > >
>
> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
For next time around I think recording a bit more of the discussions and
git history in these would be really good. At least I'd like to get more
people ramped up on ttm work, and for that to work out things need to be a
bit more accessible ... The above commit message is pretty much useless if
you ever hit it in a git blame, if you haven't been involved in any of
these discussions.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate
2021-08-26 8:49 ` Daniel Vetter
@ 2021-08-26 10:11 ` Christian König
2021-08-26 12:50 ` Daniel Vetter
0 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2021-08-26 10:11 UTC (permalink / raw)
To: Daniel Vetter, Thomas Hellström; +Cc: daniel.vetter, airlied, dri-devel
Am 26.08.21 um 10:49 schrieb Daniel Vetter:
> On Mon, Aug 23, 2021 at 01:15:20PM +0200, Thomas Hellström wrote:
>> On Mon, 2021-08-23 at 13:05 +0200, Christian König wrote:
>>> Adding Thomas on CC as well.
>>>
>>> Just a gentle ping. I think the patch set makes sense now.
>>>
>>> Regards,
>>> Christian.
>>>
>>> Am 28.07.21 um 15:05 schrieb Christian König:
>>>> Doing this in vmw_ttm_destroy() is to late.
>>>>
>>>> It turned out that this is not a good idea at all because it leaves
>>>> pointers
>>>> to freed up system memory pages in the GART tables of the drivers.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 9 +++------
>>>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>>>
>> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> For next time around I think recording a bit more of the discussions and
> git history in these would be really good. At least I'd like to get more
> people ramped up on ttm work, and for that to work out things need to be a
> bit more accessible ... The above commit message is pretty much useless if
> you ever hit it in a git blame, if you haven't been involved in any of
> these discussions.
I've pushed it with a link tag back to the patches in patchwork, but I
should probably include a link tag to the older versions as well for
completeness.
Christian.
> -Daniel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/5] drm/vmwgfx: unbind in vmw_ttm_unpopulate
2021-08-26 10:11 ` Christian König
@ 2021-08-26 12:50 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2021-08-26 12:50 UTC (permalink / raw)
To: Christian König
Cc: Daniel Vetter, Thomas Hellström, daniel.vetter, airlied, dri-devel
On Thu, Aug 26, 2021 at 12:11:04PM +0200, Christian König wrote:
>
>
> Am 26.08.21 um 10:49 schrieb Daniel Vetter:
> > On Mon, Aug 23, 2021 at 01:15:20PM +0200, Thomas Hellström wrote:
> > > On Mon, 2021-08-23 at 13:05 +0200, Christian König wrote:
> > > > Adding Thomas on CC as well.
> > > >
> > > > Just a gentle ping. I think the patch set makes sense now.
> > > >
> > > > Regards,
> > > > Christian.
> > > >
> > > > Am 28.07.21 um 15:05 schrieb Christian König:
> > > > > Doing this in vmw_ttm_destroy() is to late.
> > > > >
> > > > > It turned out that this is not a good idea at all because it leaves
> > > > > pointers
> > > > > to freed up system memory pages in the GART tables of the drivers.
> > > > >
> > > > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > > > ---
> > > > > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 9 +++------
> > > > > 1 file changed, 3 insertions(+), 6 deletions(-)
> > > > >
> > > Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > For next time around I think recording a bit more of the discussions and
> > git history in these would be really good. At least I'd like to get more
> > people ramped up on ttm work, and for that to work out things need to be a
> > bit more accessible ... The above commit message is pretty much useless if
> > you ever hit it in a git blame, if you haven't been involved in any of
> > these discussions.
>
> I've pushed it with a link tag back to the patches in patchwork, but I
> should probably include a link tag to the older versions as well for
> completeness.
Imo references to the commits that landed which broke stuff and full
quotes of the relevant discussions in the other thread. Just adding the
links still means you have to painstakingly stitch the story together
yourself, but at least it would be somewhat possible.
Really I think stuff like this should be documented in kerneldoc for these
callbacks, and we just got to start doing that. Waiting until magically
someone else fixes the ttm kerneldoc wont happen :-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 12+ messages in thread