* Re: next-20081121:WARNING: at drivers/dma/dmaengine.c:665 dma_async_device_unregister()
2008-11-21 10:40 next-20081121:WARNING: at drivers/dma/dmaengine.c:665 dma_async_device_unregister() Alexander Beregalov
@ 2008-11-21 17:17 ` Dan Williams
2008-11-22 0:53 ` Dan Williams
1 sibling, 0 replies; 3+ messages in thread
From: Dan Williams @ 2008-11-21 17:17 UTC (permalink / raw)
To: Alexander Beregalov; +Cc: linux-next, LKML, maciej.sosnowski
On Fri, Nov 21, 2008 at 3:40 AM, Alexander Beregalov
<a.beregalov@gmail.com> wrote:
> Hi
>
> It occured during shutdown process:
>
> sd 0:2:0:0: [sda] Synchronizing SCSI cache
> ioatdma 0000:00:08.0: Removing dma and dca services
> ------------[ cut here ]------------
> WARNING: at drivers/dma/dmaengine.c:665 dma_async_device_unregister+0x90/0xd0()
> dma_async_device_unregister called while 1 clients hold a reference
> Modules linked in:
> Pid: 12246, comm: reboot Tainted: G W 2.6.28-rc5-next-20081121 #8
> Call Trace:
> [<ffffffff80236b7c>] warn_slowpath+0xae/0xd5
> [<ffffffff80298414>] ? add_partial+0x1a/0x50
> [<ffffffff804588e6>] ? dma_async_device_unregister+0x22/0xd0
> [<ffffffff802576c4>] ? trace_hardirqs_on_caller+0x1f/0x153
> [<ffffffff804e4829>] ? __mutex_lock_common+0x371/0x3be
> [<ffffffff804588e6>] ? dma_async_device_unregister+0x22/0xd0
> [<ffffffff802576c4>] ? trace_hardirqs_on_caller+0x1f/0x153
> [<ffffffff80257805>] ? trace_hardirqs_on+0xd/0xf
> [<ffffffff804e4a51>] ? __mutex_unlock_slowpath+0x13c/0x145
> [<ffffffff80458954>] dma_async_device_unregister+0x90/0xd0
> [<ffffffff80459b62>] ioat_dma_remove+0x1c/0xd0
> [<ffffffff80459992>] ioat_shutdown_functionality+0x66/0x74
> [<ffffffff80388ad2>] pci_device_shutdown+0x24/0x38
> [<ffffffff803e042c>] device_shutdown+0x50/0x99
> [<ffffffff80245612>] kernel_restart_prepare+0x27/0x2e
> [<ffffffff80245652>] kernel_restart+0x11/0x44
> [<ffffffff802457af>] sys_reboot+0x11d/0x149
> [<ffffffff8024d3f1>] ? hrtimer_nanosleep+0x104/0x119
> [<ffffffff8024c9f6>] ? hrtimer_wakeup+0x0/0x21
> [<ffffffff804e4ab5>] ? do_nanosleep+0x50/0xb6
> [<ffffffff802576c4>] ? trace_hardirqs_on_caller+0x1f/0x153
> [<ffffffff804e583a>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [<ffffffff8020b75b>] system_call_fastpath+0x16/0x1b
> ---[ end trace 4eaa2a86a8e2da22 ]---
Looks like the network stack needs to drop its reference to dmaengine
at shutdown. Need to investigate if netdev_exit is the proper
location for this...
> ------------[ cut here ]------------
> WARNING: at drivers/base/core.c:122 device_release+0x66/0x68()
> Device 'dma0chan0' does not have a release() function, it is broken and must be
> fixed.
> Modules linked in:
> Pid: 12246, comm: reboot Tainted: G W 2.6.28-rc5-next-20081121 #8
> Call Trace:
> [<ffffffff80236b7c>] warn_slowpath+0xae/0xd5
> [<ffffffff802e4812>] ? release_sysfs_dirent+0x7d/0x9d
> [<ffffffff802e484a>] ? __sysfs_put+0x18/0x1a
> [<ffffffff802e4aa3>] ? sysfs_addrm_finish+0x233/0x27b
> [<ffffffff802af85f>] ? ifind+0x2c/0x84
> [<ffffffff80257805>] ? trace_hardirqs_on+0xd/0xf
> [<ffffffff804e44a4>] ? mutex_trylock+0x146/0x15a
> [<ffffffff802e4668>] ? sysfs_addrm_start+0x78/0xa5
> [<ffffffff802e4668>] ? sysfs_addrm_start+0x78/0xa5
> [<ffffffff8037657c>] ? kobj_kset_leave+0x1e/0x57
> [<ffffffff803e0b57>] device_release+0x66/0x68
> [<ffffffff8037663e>] kobject_release+0x52/0x68
> [<ffffffff803765ec>] ? kobject_release+0x0/0x68
> [<ffffffff80377429>] kref_put+0x43/0x4f
> [<ffffffff80376546>] kobject_put+0x47/0x4b
> [<ffffffff803e048a>] put_device+0x15/0x17
> [<ffffffff803e12a3>] device_unregister+0x19/0x1e
> [<ffffffff80458968>] dma_async_device_unregister+0xa4/0xd0
> [<ffffffff80459b62>] ioat_dma_remove+0x1c/0xd0
> [<ffffffff80459992>] ioat_shutdown_functionality+0x66/0x74
> [<ffffffff80388ad2>] pci_device_shutdown+0x24/0x38
> [<ffffffff803e042c>] device_shutdown+0x50/0x99
> [<ffffffff80245612>] kernel_restart_prepare+0x27/0x2e
> [<ffffffff80245652>] kernel_restart+0x11/0x44
> [<ffffffff802457af>] sys_reboot+0x11d/0x149
> [<ffffffff8024d3f1>] ? hrtimer_nanosleep+0x104/0x119
> [<ffffffff8024c9f6>] ? hrtimer_wakeup+0x0/0x21
> [<ffffffff804e4ab5>] ? do_nanosleep+0x50/0xb6
> [<ffffffff802576c4>] ? trace_hardirqs_on_caller+0x1f/0x153
> [<ffffffff804e583a>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [<ffffffff8020b75b>] system_call_fastpath+0x16/0x1b
> ---[ end trace 4eaa2a86a8e2da22 ]---
With the old reference counting scheme we apparently never released
the dma class devices. I'll address this.
Strange that this did not trigger on my iop13xx platform...
Thanks for the report,
Dan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: next-20081121:WARNING: at drivers/dma/dmaengine.c:665 dma_async_device_unregister()
2008-11-21 10:40 next-20081121:WARNING: at drivers/dma/dmaengine.c:665 dma_async_device_unregister() Alexander Beregalov
2008-11-21 17:17 ` Dan Williams
@ 2008-11-22 0:53 ` Dan Williams
1 sibling, 0 replies; 3+ messages in thread
From: Dan Williams @ 2008-11-22 0:53 UTC (permalink / raw)
To: Alexander Beregalov; +Cc: linux-next@vger.kernel.org, LKML, Sosnowski, Maciej
On Fri, 2008-11-21 at 03:40 -0700, Alexander Beregalov wrote:
> Hi
>
> It occured during shutdown process:
>
> sd 0:2:0:0: [sda] Synchronizing SCSI cache
> ioatdma 0000:00:08.0: Removing dma and dca services
> ------------[ cut here ]------------
> WARNING: at drivers/dma/dmaengine.c:665 dma_async_device_unregister+0x90/0xd0()
> dma_async_device_unregister called while 1 clients hold a reference
...
> WARNING: at drivers/base/core.c:122 device_release+0x66/0x68()
> Device 'dma0chan0' does not have a release() function, it is broken and must be
> fixed.
The following fixes these up for me. Maciej, do these look alright?:
commit 16f5de60e2406c0c879627177aecd7ef489c30e5
Author: Dan Williams <dan.j.williams@intel.com>
Date: Fri Nov 21 17:48:55 2008 -0700
dmaengine: add a release for dma class devices
Resolves:
WARNING: at drivers/base/core.c:122 device_release+0x4d/0x52()
Device 'dma0chan0' does not have a release() function, it is broken and must be fixed.
Reported-by: Alexander Beregalov <a.beregalov@gmail.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 6fc6595..94dcc64 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -103,9 +103,16 @@ static struct device_attribute dma_attrs[] = {
__ATTR_NULL
};
+static void chan_dev_release(struct device *dev)
+{
+ /* nothing to do */
+ do { } while (0);
+}
+
static struct class dma_devclass = {
.name = "dma",
.dev_attrs = dma_attrs,
+ .dev_release = chan_dev_release,
};
/* --- client and device registration --- */
commit 34323672c38c8e4e8fdd3aa5fa37daa617b51a29
Author: Dan Williams <dan.j.williams@intel.com>
Date: Fri Nov 21 17:39:18 2008 -0700
ioat: do not perform removal actions at shutdown
Unregistering services should only happen at "remove" time. This prevents
the device from being unregistered while dmaengine clients are still
active. Also, the comment on ioat_remove is stale since removal is prevented
while a channel may be in use.
Reported-by: Alexander Beregalov <a.beregalov@gmail.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
diff --git a/drivers/dma/ioat.c b/drivers/dma/ioat.c
index 9b16a3a..4105d65 100644
--- a/drivers/dma/ioat.c
+++ b/drivers/dma/ioat.c
@@ -75,60 +75,10 @@ static int ioat_dca_enabled = 1;
module_param(ioat_dca_enabled, int, 0644);
MODULE_PARM_DESC(ioat_dca_enabled, "control support of dca service (default: 1)");
-static int ioat_setup_functionality(struct pci_dev *pdev, void __iomem *iobase)
-{
- struct ioat_device *device = pci_get_drvdata(pdev);
- u8 version;
- int err = 0;
-
- version = readb(iobase + IOAT_VER_OFFSET);
- switch (version) {
- case IOAT_VER_1_2:
- device->dma = ioat_dma_probe(pdev, iobase);
- if (device->dma && ioat_dca_enabled)
- device->dca = ioat_dca_init(pdev, iobase);
- break;
- case IOAT_VER_2_0:
- device->dma = ioat_dma_probe(pdev, iobase);
- if (device->dma && ioat_dca_enabled)
- device->dca = ioat2_dca_init(pdev, iobase);
- break;
- case IOAT_VER_3_0:
- device->dma = ioat_dma_probe(pdev, iobase);
- if (device->dma && ioat_dca_enabled)
- device->dca = ioat3_dca_init(pdev, iobase);
- break;
- default:
- err = -ENODEV;
- break;
- }
- if (!device->dma)
- err = -ENODEV;
- return err;
-}
-
-static void ioat_shutdown_functionality(struct pci_dev *pdev)
-{
- struct ioat_device *device = pci_get_drvdata(pdev);
-
- dev_err(&pdev->dev, "Removing dma and dca services\n");
- if (device->dca) {
- unregister_dca_provider(device->dca);
- free_dca_provider(device->dca);
- device->dca = NULL;
- }
-
- if (device->dma) {
- ioat_dma_remove(device->dma);
- device->dma = NULL;
- }
-}
-
static struct pci_driver ioat_pci_driver = {
.name = "ioatdma",
.id_table = ioat_pci_tbl,
.probe = ioat_probe,
- .shutdown = ioat_shutdown_functionality,
.remove = __devexit_p(ioat_remove),
};
@@ -179,7 +129,29 @@ static int __devinit ioat_probe(struct pci_dev *pdev,
pci_set_master(pdev);
- err = ioat_setup_functionality(pdev, iobase);
+ switch (readb(iobase + IOAT_VER_OFFSET)) {
+ case IOAT_VER_1_2:
+ device->dma = ioat_dma_probe(pdev, iobase);
+ if (device->dma && ioat_dca_enabled)
+ device->dca = ioat_dca_init(pdev, iobase);
+ break;
+ case IOAT_VER_2_0:
+ device->dma = ioat_dma_probe(pdev, iobase);
+ if (device->dma && ioat_dca_enabled)
+ device->dca = ioat2_dca_init(pdev, iobase);
+ break;
+ case IOAT_VER_3_0:
+ device->dma = ioat_dma_probe(pdev, iobase);
+ if (device->dma && ioat_dca_enabled)
+ device->dca = ioat3_dca_init(pdev, iobase);
+ break;
+ default:
+ err = -ENODEV;
+ break;
+ }
+ if (!device->dma)
+ err = -ENODEV;
+
if (err)
goto err_version;
@@ -198,17 +170,21 @@ err_enable_device:
return err;
}
-/*
- * It is unsafe to remove this module: if removed while a requested
- * dma is outstanding, esp. from tcp, it is possible to hang while
- * waiting for something that will never finish. However, if you're
- * feeling lucky, this usually works just fine.
- */
static void __devexit ioat_remove(struct pci_dev *pdev)
{
struct ioat_device *device = pci_get_drvdata(pdev);
- ioat_shutdown_functionality(pdev);
+ dev_err(&pdev->dev, "Removing dma and dca services\n");
+ if (device->dca) {
+ unregister_dca_provider(device->dca);
+ free_dca_provider(device->dca);
+ device->dca = NULL;
+ }
+
+ if (device->dma) {
+ ioat_dma_remove(device->dma);
+ device->dma = NULL;
+ }
kfree(device);
}
^ permalink raw reply related [flat|nested] 3+ messages in thread