* [PATCH 1/3] drm/ttm/dma: Only call set_pages_array_wb when the page is not in WB pool.
2012-01-04 21:35 [PATCH] fixes to drm-next - ttm code (v2) found when using nouveau Konrad Rzeszutek Wilk
@ 2012-01-04 21:35 ` Konrad Rzeszutek Wilk
2012-01-05 15:55 ` Jerome Glisse
2012-01-04 21:35 ` [PATCH 2/3] drm/ttm/dma: Fix accounting error when calling ttm_mem_global_free_page and don't try to free freed pages Konrad Rzeszutek Wilk
2012-01-04 21:35 ` [PATCH 3/3] Revert "drm/ttm: callback move_notify any time bo placement change v4" Konrad Rzeszutek Wilk
2 siblings, 1 reply; 7+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-01-04 21:35 UTC (permalink / raw)
To: linux-kernel, dri-devel, j.glisse, bskeggs, thellstrom, airlied
Cc: Konrad Rzeszutek Wilk
Otherwise we are doing redundant work. Especially since the 'unbind'
and 'unpopulate' have been merged and nouveau driver ends up calling
it quite excessivly. On a GeForce 8600 GT with Gnome Shell (GNOME 3)
we end up spending about 54% CPU time in __change_page_attr_set_clr
checking the page flags.
The callgraph (annotated) looks as so before this patch:
53.29% gnome-shell [kernel.kallsyms] [k] static_protections
|
--- static_protections
|
|--91.80%-- __change_page_attr_set_clr
| change_page_attr_set_clr
| set_pages_array_wb
| |
| |--96.55%-- ttm_dma_unpopulate
| | nouveau_ttm_tt_unpopulate
| | ttm_tt_destroy
| | ttm_bo_cleanup_memtype_use
| | ttm_bo_release
| | kref_put
| | ttm_bo_unref
| | nouveau_gem_object_del
| | drm_gem_object_free
| | kref_put
| | drm_gem_object_unreference_unlocked
| | drm_gem_object_handle_unreference_unlocked.part.1
| | drm_gem_handle_delete
| | drm_gem_close_ioctl
| | drm_ioctl
| | do_vfs_ioctl
| | sys_ioctl
| | system_call_fastpath
| | __GI___ioctl
| |
| --3.45%-- ttm_dma_pages_put
| ttm_dma_page_pool_free
| ttm_dma_unpopulate
| nouveau_ttm_tt_unpopulate
| ttm_tt_destroy
| ttm_bo_cleanup_memtype_use
| ttm_bo_release
| kref_put
| ttm_bo_unref
| nouveau_gem_object_del
| drm_gem_object_free
| kref_put
| drm_gem_object_unreference_unlocked
| drm_gem_object_handle_unreference_unlocked.part.1
| drm_gem_handle_delete
| drm_gem_close_ioctl
| drm_ioctl
| do_vfs_ioctl
| sys_ioctl
| system_call_fastpath
| __GI___ioctl
|
--8.20%-- change_page_attr_set_clr
set_pages_array_wb
|
|--93.76%-- ttm_dma_unpopulate
| nouveau_ttm_tt_unpopulate
| ttm_tt_destroy
| ttm_bo_cleanup_memtype_use
| ttm_bo_release
| kref_put
| ttm_bo_unref
| nouveau_gem_object_del
| drm_gem_object_free
| kref_put
| drm_gem_object_unreference_unlocked
| drm_gem_object_handle_unreference_unlocked.part.1
| drm_gem_handle_delete
| drm_gem_close_ioctl
| drm_ioctl
| do_vfs_ioctl
| sys_ioctl
| system_call_fastpath
| __GI___ioctl
|
--6.24%-- ttm_dma_pages_put
ttm_dma_page_pool_free
ttm_dma_unpopulate
nouveau_ttm_tt_unpopulate
ttm_tt_destroy
ttm_bo_cleanup_memtype_use
ttm_bo_release
kref_put
ttm_bo_unref
nouveau_gem_object_del
drm_gem_object_free
kref_put
drm_gem_object_unreference_unlocked
drm_gem_object_handle_unreference_unlocked.part.1
drm_gem_handle_delete
drm_gem_close_ioctl
drm_ioctl
do_vfs_ioctl
sys_ioctl
system_call_fastpath
__GI___ioctl
and after this patch all of that disappears.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index 6678abc..6c06d0b 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -384,7 +384,9 @@ static void ttm_dma_pages_put(struct dma_pool *pool, struct list_head *d_pages,
{
struct dma_page *d_page, *tmp;
- if (npages && set_pages_array_wb(pages, npages))
+ /* Don't set WB on WB page pool. */
+ if (npages && !(pool->type & IS_CACHED) &&
+ set_pages_array_wb(pages, npages))
pr_err(TTM_PFX "%s: Failed to set %d pages to wb!\n",
pool->dev_name, npages);
@@ -396,7 +398,8 @@ static void ttm_dma_pages_put(struct dma_pool *pool, struct list_head *d_pages,
static void ttm_dma_page_put(struct dma_pool *pool, struct dma_page *d_page)
{
- if (set_pages_array_wb(&d_page->p, 1))
+ /* Don't set WB on WB page pool. */
+ if (!(pool->type & IS_CACHED) && set_pages_array_wb(&d_page->p, 1))
pr_err(TTM_PFX "%s: Failed to set %d pages to wb!\n",
pool->dev_name, 1);
--
1.7.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] drm/ttm/dma: Only call set_pages_array_wb when the page is not in WB pool.
2012-01-04 21:35 ` [PATCH 1/3] drm/ttm/dma: Only call set_pages_array_wb when the page is not in WB pool Konrad Rzeszutek Wilk
@ 2012-01-05 15:55 ` Jerome Glisse
0 siblings, 0 replies; 7+ messages in thread
From: Jerome Glisse @ 2012-01-05 15:55 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-kernel, dri-devel, bskeggs, thellstrom, airlied
On Wed, Jan 04, 2012 at 04:35:14PM -0500, Konrad Rzeszutek Wilk wrote:
> Otherwise we are doing redundant work. Especially since the 'unbind'
> and 'unpopulate' have been merged and nouveau driver ends up calling
> it quite excessivly. On a GeForce 8600 GT with Gnome Shell (GNOME 3)
> we end up spending about 54% CPU time in __change_page_attr_set_clr
> checking the page flags.
>
> The callgraph (annotated) looks as so before this patch:
>
> 53.29% gnome-shell [kernel.kallsyms] [k] static_protections
> |
> --- static_protections
> |
> |--91.80%-- __change_page_attr_set_clr
> | change_page_attr_set_clr
> | set_pages_array_wb
> | |
> | |--96.55%-- ttm_dma_unpopulate
> | | nouveau_ttm_tt_unpopulate
> | | ttm_tt_destroy
> | | ttm_bo_cleanup_memtype_use
> | | ttm_bo_release
> | | kref_put
> | | ttm_bo_unref
> | | nouveau_gem_object_del
> | | drm_gem_object_free
> | | kref_put
> | | drm_gem_object_unreference_unlocked
> | | drm_gem_object_handle_unreference_unlocked.part.1
> | | drm_gem_handle_delete
> | | drm_gem_close_ioctl
> | | drm_ioctl
> | | do_vfs_ioctl
> | | sys_ioctl
> | | system_call_fastpath
> | | __GI___ioctl
> | |
> | --3.45%-- ttm_dma_pages_put
> | ttm_dma_page_pool_free
> | ttm_dma_unpopulate
> | nouveau_ttm_tt_unpopulate
> | ttm_tt_destroy
> | ttm_bo_cleanup_memtype_use
> | ttm_bo_release
> | kref_put
> | ttm_bo_unref
> | nouveau_gem_object_del
> | drm_gem_object_free
> | kref_put
> | drm_gem_object_unreference_unlocked
> | drm_gem_object_handle_unreference_unlocked.part.1
> | drm_gem_handle_delete
> | drm_gem_close_ioctl
> | drm_ioctl
> | do_vfs_ioctl
> | sys_ioctl
> | system_call_fastpath
> | __GI___ioctl
> |
> --8.20%-- change_page_attr_set_clr
> set_pages_array_wb
> |
> |--93.76%-- ttm_dma_unpopulate
> | nouveau_ttm_tt_unpopulate
> | ttm_tt_destroy
> | ttm_bo_cleanup_memtype_use
> | ttm_bo_release
> | kref_put
> | ttm_bo_unref
> | nouveau_gem_object_del
> | drm_gem_object_free
> | kref_put
> | drm_gem_object_unreference_unlocked
> | drm_gem_object_handle_unreference_unlocked.part.1
> | drm_gem_handle_delete
> | drm_gem_close_ioctl
> | drm_ioctl
> | do_vfs_ioctl
> | sys_ioctl
> | system_call_fastpath
> | __GI___ioctl
> |
> --6.24%-- ttm_dma_pages_put
> ttm_dma_page_pool_free
> ttm_dma_unpopulate
> nouveau_ttm_tt_unpopulate
> ttm_tt_destroy
> ttm_bo_cleanup_memtype_use
> ttm_bo_release
> kref_put
> ttm_bo_unref
> nouveau_gem_object_del
> drm_gem_object_free
> kref_put
> drm_gem_object_unreference_unlocked
> drm_gem_object_handle_unreference_unlocked.part.1
> drm_gem_handle_delete
> drm_gem_close_ioctl
> drm_ioctl
> do_vfs_ioctl
> sys_ioctl
> system_call_fastpath
> __GI___ioctl
>
> and after this patch all of that disappears.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
> ---
> drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 7 +++++--
> 1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> index 6678abc..6c06d0b 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> @@ -384,7 +384,9 @@ static void ttm_dma_pages_put(struct dma_pool *pool, struct list_head *d_pages,
> {
> struct dma_page *d_page, *tmp;
>
> - if (npages && set_pages_array_wb(pages, npages))
> + /* Don't set WB on WB page pool. */
> + if (npages && !(pool->type & IS_CACHED) &&
> + set_pages_array_wb(pages, npages))
> pr_err(TTM_PFX "%s: Failed to set %d pages to wb!\n",
> pool->dev_name, npages);
>
> @@ -396,7 +398,8 @@ static void ttm_dma_pages_put(struct dma_pool *pool, struct list_head *d_pages,
>
> static void ttm_dma_page_put(struct dma_pool *pool, struct dma_page *d_page)
> {
> - if (set_pages_array_wb(&d_page->p, 1))
> + /* Don't set WB on WB page pool. */
> + if (!(pool->type & IS_CACHED) && set_pages_array_wb(&d_page->p, 1))
> pr_err(TTM_PFX "%s: Failed to set %d pages to wb!\n",
> pool->dev_name, 1);
>
> --
> 1.7.7.4
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] drm/ttm/dma: Fix accounting error when calling ttm_mem_global_free_page and don't try to free freed pages.
2012-01-04 21:35 [PATCH] fixes to drm-next - ttm code (v2) found when using nouveau Konrad Rzeszutek Wilk
2012-01-04 21:35 ` [PATCH 1/3] drm/ttm/dma: Only call set_pages_array_wb when the page is not in WB pool Konrad Rzeszutek Wilk
@ 2012-01-04 21:35 ` Konrad Rzeszutek Wilk
2012-01-04 21:35 ` [PATCH 3/3] Revert "drm/ttm: callback move_notify any time bo placement change v4" Konrad Rzeszutek Wilk
2 siblings, 0 replies; 7+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-01-04 21:35 UTC (permalink / raw)
To: linux-kernel, dri-devel, j.glisse, bskeggs, thellstrom, airlied
Cc: Konrad Rzeszutek Wilk
The code to figure out how many pages to shrink the pool
ends up capping the 'count' at _manager->options.max_size - which is OK.
Except that the 'count' is also used when accounting for how many pages
are recycled - which we end up with the invalid values. This fixes
it by using a different value for the amount of pages to shrink.
On top of that we would free the cached page pool - which is nonsense
as they are deleted from the pool - so there are no free pages in that
pool..
Also we also missed the opportunity to batch the amount of pages
to free (similar to how ttm_page_alloc.c does it). This reintroduces
the code that was lost during rebasing.
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index 6c06d0b..156ddcd 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -949,7 +949,7 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev)
struct dma_page *d_page, *next;
enum pool_type type;
bool is_cached = false;
- unsigned count = 0, i;
+ unsigned count = 0, i, npages = 0;
unsigned long irq_flags;
type = ttm_to_type(ttm->page_flags, ttm->caching_state);
@@ -974,8 +974,13 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev)
} else {
pool->npages_free += count;
list_splice(&ttm_dma->pages_list, &pool->free_list);
+ npages = count;
if (pool->npages_free > _manager->options.max_size) {
- count = pool->npages_free - _manager->options.max_size;
+ npages = pool->npages_free - _manager->options.max_size;
+ /* free at least NUM_PAGES_TO_ALLOC number of pages
+ * to reduce calls to set_memory_wb */
+ if (npages < NUM_PAGES_TO_ALLOC)
+ npages = NUM_PAGES_TO_ALLOC;
}
}
spin_unlock_irqrestore(&pool->lock, irq_flags);
@@ -999,9 +1004,9 @@ void ttm_dma_unpopulate(struct ttm_dma_tt *ttm_dma, struct device *dev)
ttm_dma->dma_address[i] = 0;
}
- /* shrink pool if necessary */
- if (count)
- ttm_dma_page_pool_free(pool, count);
+ /* shrink pool if necessary (only on !is_cached pools)*/
+ if (npages)
+ ttm_dma_page_pool_free(pool, npages);
ttm->state = tt_unpopulated;
}
EXPORT_SYMBOL_GPL(ttm_dma_unpopulate);
--
1.7.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 3/3] Revert "drm/ttm: callback move_notify any time bo placement change v4"
2012-01-04 21:35 [PATCH] fixes to drm-next - ttm code (v2) found when using nouveau Konrad Rzeszutek Wilk
2012-01-04 21:35 ` [PATCH 1/3] drm/ttm/dma: Only call set_pages_array_wb when the page is not in WB pool Konrad Rzeszutek Wilk
2012-01-04 21:35 ` [PATCH 2/3] drm/ttm/dma: Fix accounting error when calling ttm_mem_global_free_page and don't try to free freed pages Konrad Rzeszutek Wilk
@ 2012-01-04 21:35 ` Konrad Rzeszutek Wilk
2012-01-05 18:29 ` Jerome Glisse
2 siblings, 1 reply; 7+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-01-04 21:35 UTC (permalink / raw)
To: linux-kernel, dri-devel, j.glisse, bskeggs, thellstrom, airlied
Cc: Konrad Rzeszutek Wilk
This reverts commit 5c2680ddbe14b24771d24841dd66881f90d3d740 otherwise
when we unbind the device we get this:
sh-4.1# echo "0000:00:0d.0" > unbind
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<ffffffffa011ab1
CPU 1
Modules linked in: iscsi_boot_sysfs iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi libcrc32c crc32c radeon sg sd_mod nouveau fbcon tileblit ttm font sata_nv bitblit softcursor drm_kms_helper mxm_wmi ata_generic wmi libata video e1000 scsi_mod skge
Pid: 3070, comm: sh Not tainted 3.2.0-rc7-00108-gf8eb6d6 #1 BIOSTAR Group N61PB-M2S/N61PB-M2S
RIP: 0010:[<ffffffffa011ab1d>] [<ffffffffa011ab1d>] nouveau_bo_move_ntfy+0x1d/0xc0 [nouveau]
RSP: 0018:ffff88012deb7be8 EFLAGS: 00010296
RAX: ffffffffa011ab00 RBX: ffff8801292ec210 RCX: ffff880132480498
RDX: ffff880132480498 RSI: 0000000000000000 RDI: ffff8801292ec000
RBP: ffff88012deb7c18 R08: 0000000000000000 R09: ffff88012e654df8
R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000001
R13: ffff880132480728 R14: ffff880132480320 R15: ffff88012e654df8
FS: 00007f191ca6a700(0000) GS:ffff88013fc80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000012de89000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process sh (pid: 3070, threadinfo ffff88012deb6000, task ffff8801295f9e30)
Stack:
ffff88012deb7c08 ffff8801292ec000 0000000000000001 ffff880132480728
ffff880132480320 ffff88012e654df8 ffff88012deb7c38 ffffffffa00eabe6
ffff88012deb7c38 ffff8801292ec000 ffff88012deb7ca8 ffffffffa00edcd1
Call Trace:
[<ffffffffa00eabe6>] ttm_bo_cleanup_memtype_use+0x26/0x80 [ttm]
[<ffffffffa00edcd1>] ttm_bo_release+0x251/0x270 [ttm]
[<ffffffff8108ede1>] ? get_parent_ip+0x11/0x50
[<ffffffffa00eda80>] ? ttm_bo_create+0x110/0x110 [ttm]
[<ffffffff812f0997>] kref_put+0x37/0x70
[<ffffffffa00eb572>] ttm_bo_unref+0x42/0x60 [ttm]
[<ffffffff813d5cb0>] ? drm_gem_object_handle_free+0x90/0x90
[<ffffffffa011e9bf>] nouveau_gem_object_del+0x3f/0x70 [nouveau]
[<ffffffff813d5cda>] drm_gem_object_free+0x2a/0x40
[<ffffffff812f0997>] kref_put+0x37/0x70
[<ffffffffa013bd16>] nouveau_fbcon_fini+0xb6/0x130 [nouveau]
[<ffffffffa0111f3d>] nouveau_unload+0x1bd/0x230 [nouveau]
[<ffffffff813d4132>] ? drm_lastclose+0x2c2/0x310
[<ffffffff813da403>] drm_put_dev+0x73/0x240
[<ffffffffa011101d>] nouveau_pci_remove+0x1d/0x20 [nouveau]
[<ffffffff813199a7>] pci_device_remove+0x37/0x70
[<ffffffff813f3a76>] __device_release_driver+0x66/0xc0
[<ffffffff813f3bcd>] device_release_driver+0x2d/0x40
[<ffffffff813f3011>] driver_unbind+0xa1/0xc0
[<ffffffff813f244c>] drv_attr_store+0x2c/0x30
[<ffffffff81213189>] sysfs_write_file+0xf9/0x180
[<ffffffff8119f778>] vfs_write+0xc8/0x190
[<ffffffff8119f941>] sys_write+0x51/0x90
[<ffffffff81644c82>] system_call_fastpath+0x16/0x1b
Code: 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 08 66 66 66 66 90 48 8b 9f 10 02 00 00 <4c> 8b 3e 4c 8d af 10 02 00 00 49 89 fe 49 89 f4 4c 39 eb 75 16
RIP [<ffffffffa011ab1d>] nouveau_bo_move_ntfy+0x1d/0xc0 [nouveau]
RSP <ffff88012deb7be8>
CR2: 0000000000000000
---[ end trace b57ad98adec9841d ]---
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/gpu/drm/nouveau/nouveau_bo.c | 4 ++--
drivers/gpu/drm/ttm/ttm_bo.c | 9 +++------
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index aa91f7b..47d0b05 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -815,10 +815,10 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, struct ttm_mem_reg *new_mem)
struct nouveau_vma *vma;
list_for_each_entry(vma, &nvbo->vma_list, head) {
- if (new_mem && new_mem->mem_type == TTM_PL_VRAM) {
+ if (new_mem->mem_type == TTM_PL_VRAM) {
nouveau_vm_map(vma, new_mem->mm_node);
} else
- if (new_mem && new_mem->mem_type == TTM_PL_TT &&
+ if (new_mem->mem_type == TTM_PL_TT &&
nvbo->page_shift == vma->vm->spg_shift) {
nouveau_vm_map_sg(vma, 0, new_mem->
num_pages << PAGE_SHIFT,
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 2f0eab6..483cded 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -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);
@@ -416,9 +419,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
if (ret)
goto out_err;
- if (bdev->driver->move_notify)
- bdev->driver->move_notify(bo, mem);
-
moved:
if (bo->evicted) {
ret = bdev->driver->invalidate_caches(bdev, bo->mem.placement);
@@ -457,9 +457,6 @@ out_err:
static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
{
- if (bo->bdev->driver->move_notify)
- bo->bdev->driver->move_notify(bo, NULL);
-
if (bo->ttm) {
ttm_tt_unbind(bo->ttm);
ttm_tt_destroy(bo->ttm);
--
1.7.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] Revert "drm/ttm: callback move_notify any time bo placement change v4"
2012-01-04 21:35 ` [PATCH 3/3] Revert "drm/ttm: callback move_notify any time bo placement change v4" Konrad Rzeszutek Wilk
@ 2012-01-05 18:29 ` Jerome Glisse
0 siblings, 0 replies; 7+ messages in thread
From: Jerome Glisse @ 2012-01-05 18:29 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-kernel, dri-devel, bskeggs, thellstrom, airlied
On Wed, Jan 04, 2012 at 04:35:16PM -0500, Konrad Rzeszutek Wilk wrote:
> This reverts commit 5c2680ddbe14b24771d24841dd66881f90d3d740 otherwise
> when we unbind the device we get this:
> sh-4.1# echo "0000:00:0d.0" > unbind
> BUG: unable to handle kernel NULL pointer dereference at (null)
> IP: [<ffffffffa011ab1
> CPU 1
> Modules linked in: iscsi_boot_sysfs iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi libcrc32c crc32c radeon sg sd_mod nouveau fbcon tileblit ttm font sata_nv bitblit softcursor drm_kms_helper mxm_wmi ata_generic wmi libata video e1000 scsi_mod skge
>
> Pid: 3070, comm: sh Not tainted 3.2.0-rc7-00108-gf8eb6d6 #1 BIOSTAR Group N61PB-M2S/N61PB-M2S
> RIP: 0010:[<ffffffffa011ab1d>] [<ffffffffa011ab1d>] nouveau_bo_move_ntfy+0x1d/0xc0 [nouveau]
> RSP: 0018:ffff88012deb7be8 EFLAGS: 00010296
> RAX: ffffffffa011ab00 RBX: ffff8801292ec210 RCX: ffff880132480498
> RDX: ffff880132480498 RSI: 0000000000000000 RDI: ffff8801292ec000
> RBP: ffff88012deb7c18 R08: 0000000000000000 R09: ffff88012e654df8
> R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000001
> R13: ffff880132480728 R14: ffff880132480320 R15: ffff88012e654df8
> FS: 00007f191ca6a700(0000) GS:ffff88013fc80000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000000 CR3: 000000012de89000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process sh (pid: 3070, threadinfo ffff88012deb6000, task ffff8801295f9e30)
> Stack:
> ffff88012deb7c08 ffff8801292ec000 0000000000000001 ffff880132480728
> ffff880132480320 ffff88012e654df8 ffff88012deb7c38 ffffffffa00eabe6
> ffff88012deb7c38 ffff8801292ec000 ffff88012deb7ca8 ffffffffa00edcd1
> Call Trace:
> [<ffffffffa00eabe6>] ttm_bo_cleanup_memtype_use+0x26/0x80 [ttm]
> [<ffffffffa00edcd1>] ttm_bo_release+0x251/0x270 [ttm]
> [<ffffffff8108ede1>] ? get_parent_ip+0x11/0x50
> [<ffffffffa00eda80>] ? ttm_bo_create+0x110/0x110 [ttm]
> [<ffffffff812f0997>] kref_put+0x37/0x70
> [<ffffffffa00eb572>] ttm_bo_unref+0x42/0x60 [ttm]
> [<ffffffff813d5cb0>] ? drm_gem_object_handle_free+0x90/0x90
> [<ffffffffa011e9bf>] nouveau_gem_object_del+0x3f/0x70 [nouveau]
> [<ffffffff813d5cda>] drm_gem_object_free+0x2a/0x40
> [<ffffffff812f0997>] kref_put+0x37/0x70
> [<ffffffffa013bd16>] nouveau_fbcon_fini+0xb6/0x130 [nouveau]
> [<ffffffffa0111f3d>] nouveau_unload+0x1bd/0x230 [nouveau]
> [<ffffffff813d4132>] ? drm_lastclose+0x2c2/0x310
> [<ffffffff813da403>] drm_put_dev+0x73/0x240
> [<ffffffffa011101d>] nouveau_pci_remove+0x1d/0x20 [nouveau]
> [<ffffffff813199a7>] pci_device_remove+0x37/0x70
> [<ffffffff813f3a76>] __device_release_driver+0x66/0xc0
> [<ffffffff813f3bcd>] device_release_driver+0x2d/0x40
> [<ffffffff813f3011>] driver_unbind+0xa1/0xc0
> [<ffffffff813f244c>] drv_attr_store+0x2c/0x30
> [<ffffffff81213189>] sysfs_write_file+0xf9/0x180
> [<ffffffff8119f778>] vfs_write+0xc8/0x190
> [<ffffffff8119f941>] sys_write+0x51/0x90
> [<ffffffff81644c82>] system_call_fastpath+0x16/0x1b
> Code: 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 08 66 66 66 66 90 48 8b 9f 10 02 00 00 <4c> 8b 3e 4c 8d af 10 02 00 00 49 89 fe 49 89 f4 4c 39 eb 75 16
> RIP [<ffffffffa011ab1d>] nouveau_bo_move_ntfy+0x1d/0xc0 [nouveau]
> RSP <ffff88012deb7be8>
> CR2: 0000000000000000
> ---[ end trace b57ad98adec9841d ]---
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
NAK proper fix on its way.
Cheers,
Jerome
> ---
> drivers/gpu/drm/nouveau/nouveau_bo.c | 4 ++--
> drivers/gpu/drm/ttm/ttm_bo.c | 9 +++------
> 2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index aa91f7b..47d0b05 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -815,10 +815,10 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, struct ttm_mem_reg *new_mem)
> struct nouveau_vma *vma;
>
> list_for_each_entry(vma, &nvbo->vma_list, head) {
> - if (new_mem && new_mem->mem_type == TTM_PL_VRAM) {
> + if (new_mem->mem_type == TTM_PL_VRAM) {
> nouveau_vm_map(vma, new_mem->mm_node);
> } else
> - if (new_mem && new_mem->mem_type == TTM_PL_TT &&
> + if (new_mem->mem_type == TTM_PL_TT &&
> nvbo->page_shift == vma->vm->spg_shift) {
> nouveau_vm_map_sg(vma, 0, new_mem->
> num_pages << PAGE_SHIFT,
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 2f0eab6..483cded 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -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);
> @@ -416,9 +419,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
> if (ret)
> goto out_err;
>
> - if (bdev->driver->move_notify)
> - bdev->driver->move_notify(bo, mem);
> -
> moved:
> if (bo->evicted) {
> ret = bdev->driver->invalidate_caches(bdev, bo->mem.placement);
> @@ -457,9 +457,6 @@ out_err:
>
> static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
> {
> - if (bo->bdev->driver->move_notify)
> - bo->bdev->driver->move_notify(bo, NULL);
> -
> if (bo->ttm) {
> ttm_tt_unbind(bo->ttm);
> ttm_tt_destroy(bo->ttm);
> --
> 1.7.7.4
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] drm/ttm/dma: Only call set_pages_array_wb when the page is not in WB pool.
2011-12-12 20:09 [PATCH] fixes to drm-next - TTM DMA code (v1) Konrad Rzeszutek Wilk
@ 2011-12-12 20:09 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 7+ messages in thread
From: Konrad Rzeszutek Wilk @ 2011-12-12 20:09 UTC (permalink / raw)
To: linux-kernel, j.glisse, dri-devel
Cc: bskeggs, thellstrom, Konrad Rzeszutek Wilk
Otherwise we are doing redundant work. Especially since the 'unbind'
and 'unpopulate' have been merged and nouveau driver ends up calling
it quite excessivly. On a GeForce 8600 GT with Gnome Shell (GNOME 3)
we end up spending about 54% CPU time in __change_page_attr_set_clr
checking the page flags when I move the mouse cursor all over the screen.
The callgraph (annotated) looks as so before this patch:
53.29% gnome-shell [kernel.kallsyms] [k] static_protections
|
--- static_protections
|
|--91.80%-- __change_page_attr_set_clr
| change_page_attr_set_clr
| set_pages_array_wb
| |
| |--96.55%-- ttm_dma_unpopulate
| | nouveau_ttm_tt_unpopulate
| | ttm_tt_destroy
| | ttm_bo_cleanup_memtype_use
| | ttm_bo_release
| | kref_put
| | ttm_bo_unref
| | nouveau_gem_object_del
| | drm_gem_object_free
| | kref_put
| | drm_gem_object_unreference_unlocked
| | drm_gem_object_handle_unreference_unlocked.part.1
| | drm_gem_handle_delete
| | drm_gem_close_ioctl
| | drm_ioctl
| | do_vfs_ioctl
| | sys_ioctl
| | system_call_fastpath
| | __GI___ioctl
| |
| --3.45%-- ttm_dma_pages_put
| ttm_dma_page_pool_free
| ttm_dma_unpopulate
| nouveau_ttm_tt_unpopulate
| ttm_tt_destroy
| ttm_bo_cleanup_memtype_use
| ttm_bo_release
| kref_put
| ttm_bo_unref
| nouveau_gem_object_del
| drm_gem_object_free
| kref_put
| drm_gem_object_unreference_unlocked
| drm_gem_object_handle_unreference_unlocked.part.1
| drm_gem_handle_delete
| drm_gem_close_ioctl
| drm_ioctl
| do_vfs_ioctl
| sys_ioctl
| system_call_fastpath
| __GI___ioctl
|
--8.20%-- change_page_attr_set_clr
set_pages_array_wb
|
|--93.76%-- ttm_dma_unpopulate
| nouveau_ttm_tt_unpopulate
| ttm_tt_destroy
| ttm_bo_cleanup_memtype_use
| ttm_bo_release
| kref_put
| ttm_bo_unref
| nouveau_gem_object_del
| drm_gem_object_free
| kref_put
| drm_gem_object_unreference_unlocked
| drm_gem_object_handle_unreference_unlocked.part.1
| drm_gem_handle_delete
| drm_gem_close_ioctl
| drm_ioctl
| do_vfs_ioctl
| sys_ioctl
| system_call_fastpath
| __GI___ioctl
|
--6.24%-- ttm_dma_pages_put
ttm_dma_page_pool_free
ttm_dma_unpopulate
nouveau_ttm_tt_unpopulate
ttm_tt_destroy
ttm_bo_cleanup_memtype_use
ttm_bo_release
kref_put
ttm_bo_unref
nouveau_gem_object_del
drm_gem_object_free
kref_put
drm_gem_object_unreference_unlocked
drm_gem_object_handle_unreference_unlocked.part.1
drm_gem_handle_delete
drm_gem_close_ioctl
drm_ioctl
do_vfs_ioctl
sys_ioctl
system_call_fastpath
__GI___ioctl
and after this patch all of that disappears.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index 6678abc..6c06d0b 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -384,7 +384,9 @@ static void ttm_dma_pages_put(struct dma_pool *pool, struct list_head *d_pages,
{
struct dma_page *d_page, *tmp;
- if (npages && set_pages_array_wb(pages, npages))
+ /* Don't set WB on WB page pool. */
+ if (npages && !(pool->type & IS_CACHED) &&
+ set_pages_array_wb(pages, npages))
pr_err(TTM_PFX "%s: Failed to set %d pages to wb!\n",
pool->dev_name, npages);
@@ -396,7 +398,8 @@ static void ttm_dma_pages_put(struct dma_pool *pool, struct list_head *d_pages,
static void ttm_dma_page_put(struct dma_pool *pool, struct dma_page *d_page)
{
- if (set_pages_array_wb(&d_page->p, 1))
+ /* Don't set WB on WB page pool. */
+ if (!(pool->type & IS_CACHED) && set_pages_array_wb(&d_page->p, 1))
pr_err(TTM_PFX "%s: Failed to set %d pages to wb!\n",
pool->dev_name, 1);
--
1.7.7.3
^ permalink raw reply [flat|nested] 7+ messages in thread