Skip to content

Commit

Permalink
drm/ttm: fix two regressions since move_notify changes
Browse files Browse the repository at this point in the history
Both changes in dc97b34 cause serious
regressions in the nouveau driver.

move_notify() was originally able to presume that bo->mem is the old node,
and new_mem is the new node.  The above commit moves the call to
move_notify() to after move() has been done, which means that now, sometimes,
new_mem isn't the new node at all, bo->mem is, and new_mem points at a
stale, possibly-just-been-killed-by-move node.

This is clearly not a good situation.  This patch reverts this change, and
replaces it with a cleanup in the move() failure path instead.

The second issue is that the call to move_notify() from cleanup_memtype_use()
causes the TTM ghost objects to get passed into the driver.  This is clearly
bad as the driver knows nothing about these "fake" TTM BOs, and ends up
accessing uninitialised memory.

I worked around this in nouveau's move_notify() hook by ensuring the BO
destructor was nouveau's.  I don't particularly like this solution, and
would rather TTM never pass the driver these objects.  However, I don't
clearly understand the reason why we're calling move_notify() here anyway
and am happy to work around the problem in nouveau instead of breaking the
behaviour expected by other drivers.

Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Jerome Glisse <j.glisse@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
  • Loading branch information
Ben Skeggs authored and Dave Airlie committed Jan 25, 2012
1 parent 9fc04b5 commit 9f1feed
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 4 deletions.
4 changes: 4 additions & 0 deletions drivers/gpu/drm/nouveau/nouveau_bo.c
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,10 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, struct ttm_mem_reg *new_mem)
struct nouveau_bo *nvbo = nouveau_bo(bo);
struct nouveau_vma *vma;

/* ttm can now (stupidly) pass the driver bos it didn't create... */
if (bo->destroy != nouveau_bo_del_ttm)
return;

list_for_each_entry(vma, &nvbo->vma_list, head) {
if (new_mem && new_mem->mem_type == TTM_PL_VRAM) {
nouveau_vm_map(vma, new_mem->mm_node);
Expand Down
17 changes: 13 additions & 4 deletions drivers/gpu/drm/ttm/ttm_bo.c
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,9 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
}
}

if (bdev->driver->move_notify)
bdev->driver->move_notify(bo, mem);

if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
ret = ttm_bo_move_ttm(bo, evict, no_wait_reserve, no_wait_gpu, mem);
Expand All @@ -413,11 +416,17 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
else
ret = ttm_bo_move_memcpy(bo, evict, no_wait_reserve, no_wait_gpu, mem);

if (ret)
goto out_err;
if (ret) {
if (bdev->driver->move_notify) {
struct ttm_mem_reg tmp_mem = *mem;
*mem = bo->mem;
bo->mem = tmp_mem;
bdev->driver->move_notify(bo, mem);
bo->mem = *mem;
}

if (bdev->driver->move_notify)
bdev->driver->move_notify(bo, mem);
goto out_err;
}

moved:
if (bo->evicted) {
Expand Down

0 comments on commit 9f1feed

Please sign in to comment.