* [PATCH] bug fix for radeon driver in 2.6.38-rc6 w/ATI ES1000 @ 2011-02-23 20:48 Konrad Rzeszutek Wilk 2011-02-23 20:48 ` [PATCH] radeon: DMA unmap dummy page during unload/unbind Konrad Rzeszutek Wilk 0 siblings, 1 reply; 4+ messages in thread From: Konrad Rzeszutek Wilk @ 2011-02-23 20:48 UTC (permalink / raw) To: Dave Airlie, dri-devel, Jerome Glisse; +Cc: linux-kernel 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). ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] radeon: DMA unmap dummy page during unload/unbind. 2011-02-23 20:48 [PATCH] bug fix for radeon driver in 2.6.38-rc6 w/ATI ES1000 Konrad Rzeszutek Wilk @ 2011-02-23 20:48 ` Konrad Rzeszutek Wilk 2011-02-24 8:25 ` Alex Deucher 0 siblings, 1 reply; 4+ messages in thread From: Konrad Rzeszutek Wilk @ 2011-02-23 20:48 UTC (permalink / raw) To: Dave Airlie, dri-devel, Jerome Glisse; +Cc: linux-kernel, Konrad Rzeszutek Wilk 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); } -- 1.7.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] radeon: DMA unmap dummy page during unload/unbind. 2011-02-23 20:48 ` [PATCH] radeon: DMA unmap dummy page during unload/unbind Konrad Rzeszutek Wilk @ 2011-02-24 8:25 ` Alex Deucher 2011-02-24 15:45 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 4+ messages in thread From: Alex Deucher @ 2011-02-24 8:25 UTC (permalink / raw) To: Konrad Rzeszutek Wilk; +Cc: Dave Airlie, dri-devel, Jerome Glisse, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2440 bytes --] 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 > [-- Attachment #2: 0001-drm-radeon-kms-clean-up-gart-dummy-page-handling.patch --] [-- Type: text/x-patch, Size: 3338 bytes --] 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; /* This don't do much */ r = radeon_gem_init(rdev); if (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; if (r600_debugfs_mc_info_init(rdev)) { DRM_ERROR("Failed to register debugfs file for mc !\n"); } @@ -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; /* This don't do much */ r = radeon_gem_init(rdev); if (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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] radeon: DMA unmap dummy page during unload/unbind. 2011-02-24 8:25 ` Alex Deucher @ 2011-02-24 15:45 ` Konrad Rzeszutek Wilk 0 siblings, 0 replies; 4+ messages in thread From: Konrad Rzeszutek Wilk @ 2011-02-24 15:45 UTC (permalink / raw) To: Alex Deucher; +Cc: Dave Airlie, dri-devel, Jerome Glisse, linux-kernel 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; > /* This don't do much */ > r = radeon_gem_init(rdev); > if (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; > if (r600_debugfs_mc_info_init(rdev)) { > DRM_ERROR("Failed to register debugfs file for mc !\n"); > } > @@ -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; > /* This don't do much */ > r = radeon_gem_init(rdev); > if (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 > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-02-24 15:46 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-23 20:48 [PATCH] bug fix for radeon driver in 2.6.38-rc6 w/ATI ES1000 Konrad Rzeszutek Wilk 2011-02-23 20:48 ` [PATCH] radeon: DMA unmap dummy page during unload/unbind Konrad Rzeszutek Wilk 2011-02-24 8:25 ` Alex Deucher 2011-02-24 15:45 ` 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