* [PATCH] fixes to drm-next - ttm code (v2) found when using nouveau.
@ 2012-01-04 21:35 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
` (2 more replies)
0 siblings, 3 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
When I was debugging some DMA API accounting error (which were found to be
with the DMA API debug code), I encountered a couple of issues:
1). Doing "unbind" on the PCI device would crash the kernel _only_ with the
nouveau driver. The readon worked fine. This was due to:
"drm/ttm: callback move_notify any time bo.." and reverting that fixed it.
2). Extreme slowness when moving the cursor madly around. This was due to the
nouveau driver calling the unpopulate (now that unbind + unpopulate are squashed)
quite a lot (this is using Gnome Shell - I think GNOME2 did not have those issues
but I can't recall). Radeon did not have these issues.
Anyhow these patches fix the 50% perf regression I saw on nouveau and also some minor bugs
that I noticed. Please apply.
Thomas asked me to also do some perf results on AGP hardware (both nouveau and radeon)
and I am going to do that this week - any pointers of what is a good game/OpenGL
application to run a time-based benchmark?
^ 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.
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
* [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 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
* 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
end of thread, other threads:[~2012-01-05 18:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-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
2012-01-05 18:29 ` Jerome Glisse
-- strict thread matches above, loose matches on Subject: below --
2011-12-12 20:09 [PATCH] fixes to drm-next - TTM DMA code (v1) Konrad Rzeszutek Wilk
2011-12-12 20:09 ` [PATCH 1/3] drm/ttm/dma: Only call set_pages_array_wb when the page is not in WB pool Konrad Rzeszutek Wilk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®