I found this when I was doing a bit of testing of my TTM patches. The issue is that during unload/unbind:
echo 0000:01:05.0 > /sys/bus/pci/drivers/radeon/unbind
the radeon driver would unload, but if compiled with the CONFIG_DMA_API_DEBUG=y it would throw out this warning:
[ 22.113420] drm: unregistered panic notifier^M [ 22.126500] [drm] radeon: finishing device.^M [ 22.130868] [drm] radeon: cp finalized^M [ 22.135384] [TTM] Finalizing pool allocator.^M [ 22.139669] [TTM] Zone kernel: Used memory at exit: 0 kiB.^M [ 22.145229] [drm] radeon: ttm finalized^M [ 22.149055] vga_switcheroo: disabled^M [ 22.153393] ------------[ cut here ]------------^M [ 22.158021] WARNING: at /home/konrad/ssd/linux/lib/dma-debug.c:689 dma_debug_device_change+0x12e/0x161()^M [ 22.167475] Hardware name: PowerEdge T105 ^M [ 22.171562] pci 0000:01:08.0: DMA-API: device driver has pending DMA allocations while released from device [count=1]^M [ 22.182139] Modules linked in: nouveau video sr_mod sg sd_mod cdrom ata_generic qla2xxx radeon sata_nv scsi_transport_fc scsi_tgt libata fbcon tileblit font bitblit softcursor ttm e1000e drm_kms_helper scsi_mod^M [ 22.201101] Pid: 2550, comm: primary_fb Not tainted 2.6.38-rc6-linux-next #1^M [ 22.208129] Call Trace:^M [ 22.210575] [<ffffffff81049580>] ? warn_slowpath_common+0x80/0x98^M [ 22.216737] [<ffffffff8104962c>] ? warn_slowpath_fmt+0x41/0x43^M [ 22.222643] [<ffffffff81208fe8>] ? dma_debug_device_change+0x12e/0x161^M [ 22.229240] [<ffffffff8144e467>] ? notifier_call_chain+0x32/0x5e^M [ 22.235319] [<ffffffff81067d5f>] ? __blocking_notifier_call_chain+0x4b/0x62^M [ 22.242350] [<ffffffff81067d85>] ? blocking_notifier_call_chain+0xf/0x11^M [ 22.249121] [<ffffffff812b54fd>] ? __device_release_driver+0xab/0xb0^M [ 22.255543] [<ffffffff812b55d0>] ? device_release_driver+0x1e/0x2b^M [ 22.261794] [<ffffffff812b4c86>] ? driver_unbind+0x57/0x8c^M [ 22.267352] [<ffffffff812b440d>] ? drv_attr_store+0x27/0x29^M [ 22.272998] [<ffffffff8114a64d>] ? sysfs_write_file+0xff/0x13b^M [ 22.278903] [<ffffffff810f5b88>] ? vfs_write+0xa9/0x105^M [ 22.284202] [<ffffffff810f5c9d>] ? sys_write+0x45/0x6c^M [ 22.289415] [<ffffffff8100a992>] ? system_call_fastpath+0x16/0x1b^M [ 22.295578] ---[ end trace f6e27fa61e0cc3ec ]---^M ERROR: Module radeon is in use^M
The radeon_gart_fini was missing an pci_unmap for the dummy page. Interestingly enough the r600.c has a call to radeon_dummy_page_fini() in its unwind path so there are definitly some chipsets on which this is done without any hinderance.
This above test is easily manifested with a stock 2.6.38-rc6 on a machine with a ATI ES1000 chipset (Dell T105, IBM x366, SuperMicro X7DB8).
git commit 82568565683b4991964a5fc89a9ca0c7122818e8 adds a dummy page so that if something goes wrong it will at least fetch/write data _only_ to this location.
The radeon_gart_init sets it up.. but there is no corresponding free-ing of this dummy page. There is one invocation to free-ing the dummy page in r600.c but nothing in the radeon_gart_fini path.
This patch adds in the unloading/unbind path the free-ing of said dummy page.
Without this patch and if we have CONFIG_DMA_API_DEBUG=y set we get this:
[ 22.113420] drm: unregistered panic notifier [ 22.126500] [drm] radeon: finishing device. [ 22.130868] [drm] radeon: cp finalized [ 22.135384] [TTM] Finalizing pool allocator. [ 22.139669] [TTM] Zone kernel: Used memory at exit: 0 kiB. [ 22.145229] [drm] radeon: ttm finalized [ 22.149055] vga_switcheroo: disabled [ 22.153393] ------------[ cut here ]------------ [ 22.158021] WARNING: at /home/konrad/ssd/linux/lib/dma-debug.c:689 dma_debug_device_change+0x12e/0x161() [ 22.167475] Hardware name: PowerEdge T105 [ 22.171562] pci 0000:01:08.0: DMA-API: device driver has pending DMA allocations while released from device [count=1]
With this patch we don't see this anymore.
Signed-off-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com --- drivers/gpu/drm/radeon/radeon_gart.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 6501611..dc04c7b 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -269,4 +269,6 @@ void radeon_gart_fini(struct radeon_device *rdev) kfree(rdev->gart.pages_addr); rdev->gart.pages = NULL; rdev->gart.pages_addr = NULL; + + radeon_dummy_page_fini(rdev); }
On Wed, Feb 23, 2011 at 3:48 PM, Konrad Rzeszutek Wilk konrad.wilk@oracle.com wrote:
git commit 82568565683b4991964a5fc89a9ca0c7122818e8 adds a dummy page so that if something goes wrong it will at least fetch/write data _only_ to this location.
The radeon_gart_init sets it up.. but there is no corresponding free-ing of this dummy page. There is one invocation to free-ing the dummy page in r600.c but nothing in the radeon_gart_fini path.
This patch adds in the unloading/unbind path the free-ing of said dummy page.
This patch looks good, but we might as go one step further and just remove the dummy page stuff from r600.c, rv770.c and evergreen.c and just consolidate it in gart_init() and gart_fini(). See the attached patch.
Alex
Without this patch and if we have CONFIG_DMA_API_DEBUG=y set we get this:
[ 22.113420] drm: unregistered panic notifier [ 22.126500] [drm] radeon: finishing device. [ 22.130868] [drm] radeon: cp finalized [ 22.135384] [TTM] Finalizing pool allocator. [ 22.139669] [TTM] Zone kernel: Used memory at exit: 0 kiB. [ 22.145229] [drm] radeon: ttm finalized [ 22.149055] vga_switcheroo: disabled [ 22.153393] ------------[ cut here ]------------ [ 22.158021] WARNING: at /home/konrad/ssd/linux/lib/dma-debug.c:689 dma_debug_device_change+0x12e/0x161() [ 22.167475] Hardware name: PowerEdge T105 [ 22.171562] pci 0000:01:08.0: DMA-API: device driver has pending DMA allocations while released from device [count=1]
With this patch we don't see this anymore.
Signed-off-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com
drivers/gpu/drm/radeon/radeon_gart.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 6501611..dc04c7b 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -269,4 +269,6 @@ void radeon_gart_fini(struct radeon_device *rdev) kfree(rdev->gart.pages_addr); rdev->gart.pages = NULL; rdev->gart.pages_addr = NULL;
- radeon_dummy_page_fini(rdev);
}
1.7.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Feb 24, 2011 at 03:25:43AM -0500, Alex Deucher wrote:
On Wed, Feb 23, 2011 at 3:48 PM, Konrad Rzeszutek Wilk konrad.wilk@oracle.com wrote:
git commit 82568565683b4991964a5fc89a9ca0c7122818e8 adds a dummy page so that if something goes wrong it will at least fetch/write data _only_ to this location.
The radeon_gart_init sets it up.. but there is no corresponding free-ing of this dummy page. There is one invocation to free-ing the dummy page in r600.c but nothing in the radeon_gart_fini path.
This patch adds in the unloading/unbind path the free-ing of said dummy page.
This patch looks good, but we might as go one step further and just remove the dummy page stuff from r600.c, rv770.c and evergreen.c and just consolidate it in gart_init() and gart_fini(). See the attached patch.
Excellent. Thank you!
Alex
Without this patch and if we have CONFIG_DMA_API_DEBUG=y set we get this:
[ 22.113420] drm: unregistered panic notifier [ 22.126500] [drm] radeon: finishing device. [ 22.130868] [drm] radeon: cp finalized [ 22.135384] [TTM] Finalizing pool allocator. [ 22.139669] [TTM] Zone kernel: Used memory at exit: 0 kiB. [ 22.145229] [drm] radeon: ttm finalized [ 22.149055] vga_switcheroo: disabled [ 22.153393] ------------[ cut here ]------------ [ 22.158021] WARNING: at /home/konrad/ssd/linux/lib/dma-debug.c:689 dma_debug_device_change+0x12e/0x161() [ 22.167475] Hardware name: PowerEdge T105 [ 22.171562] pci 0000:01:08.0: DMA-API: device driver has pending DMA allocations while released from device [count=1]
With this patch we don't see this anymore.
Signed-off-by: Konrad Rzeszutek Wilk konrad.wilk@oracle.com
drivers/gpu/drm/radeon/radeon_gart.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 6501611..dc04c7b 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -269,4 +269,6 @@ void radeon_gart_fini(struct radeon_device *rdev) kfree(rdev->gart.pages_addr); rdev->gart.pages = NULL; rdev->gart.pages_addr = NULL;
- radeon_dummy_page_fini(rdev);
}
1.7.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
From d22bba30a66ba95cabeed7a71c982dfa96521384 Mon Sep 17 00:00:00 2001 From: Alex Deucher alexdeucher@gmail.com Date: Thu, 24 Feb 2011 03:16:21 -0500 Subject: [PATCH] drm/radeon/kms: clean up gart dummy page handling
As per Konrad's original patch, the dummy page used by the gart code and allocated in radeon_gart_init() was not freed properly in radeon_gart_fini().
At the same time r6xx and newer allocated and freed the dummy page on their own. So to do Konrad's patch one better, just remove the allocation and freeing of the dummy page in the r6xx, 7xx, evergreen, and ni code and allocate and free in the gart_init/fini() functions for all asics.
Cc: Konrad Rzeszutek Wilk konrad.wilk@oracle.com Cc: Jerome Glisse jglisse@redhat.com Signed-off-by: Alex Deucher alexdeucher@gmail.com
drivers/gpu/drm/radeon/evergreen.c | 4 ---- drivers/gpu/drm/radeon/r600.c | 4 ---- drivers/gpu/drm/radeon/radeon_gart.c | 2 ++ drivers/gpu/drm/radeon/rv770.c | 4 ---- 4 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 789441e..9ecb2b6 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -3048,9 +3048,6 @@ int evergreen_init(struct radeon_device *rdev) { int r;
- r = radeon_dummy_page_init(rdev);
- if (r)
return r;@@ -3162,7 +3159,6 @@ void evergreen_fini(struct radeon_device *rdev) radeon_atombios_fini(rdev); kfree(rdev->bios); rdev->bios = NULL;
- radeon_dummy_page_fini(rdev);
}
static void evergreen_pcie_gen2_enable(struct radeon_device *rdev) diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index 1252198..88eaffc 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -2510,9 +2510,6 @@ int r600_init(struct radeon_device *rdev) { int r;
- r = radeon_dummy_page_init(rdev);
- if (r)
return r;@@ -2626,7 +2623,6 @@ void r600_fini(struct radeon_device *rdev) radeon_atombios_fini(rdev); kfree(rdev->bios); rdev->bios = NULL;
- radeon_dummy_page_fini(rdev);
}
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 6501611..dc04c7b 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -269,4 +269,6 @@ void radeon_gart_fini(struct radeon_device *rdev) kfree(rdev->gart.pages_addr); rdev->gart.pages = NULL; rdev->gart.pages_addr = NULL;
- radeon_dummy_page_fini(rdev);
} diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c index d8ba676..6a312e6 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -1256,9 +1256,6 @@ int rv770_init(struct radeon_device *rdev) { int r;
- r = radeon_dummy_page_init(rdev);
- if (r)
return r;@@ -1373,7 +1370,6 @@ void rv770_fini(struct radeon_device *rdev) radeon_atombios_fini(rdev); kfree(rdev->bios); rdev->bios = NULL;
- radeon_dummy_page_fini(rdev);
}
static void rv770_pcie_gen2_enable(struct radeon_device *rdev)
1.7.1.1
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Konrad Rzeszutek Wilk -
Konrad Rzeszutek Wilk