lkml.org 
[lkml]   [2007]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRE: [PATCH] DMA: Fix broken device refcounting
Date
From
>From: Haavard Skinnemoen [mailto:hskinnemoen@atmel.com] 
>
>When a DMA device is unregistered, its reference count is decremented
>twice for each channel: Once dma_class_dev_release() and once in
>dma_chan_cleanup(). This may result in the DMA device driver's
>remove() function completing before all channels have been cleaned
>up, causing lots of use-after-free fun.
>
>Fix it by incrementing the device's reference count twice for each
>channel during registration.
>
>Signed-off-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
>---
>I'm not sure if this is the correct way to solve it, but it seems to
>work. The remove() function does not hang, which indicates that the
>device's reference count does drop all the way to zero on
>unregistration, which in turn indicates that it did actually drop
>_below_ zero before.
>
> drivers/dma/dmaengine.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
>diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
>index 8248992..302eded 100644
>--- a/drivers/dma/dmaengine.c
>+++ b/drivers/dma/dmaengine.c
>@@ -397,6 +397,8 @@ int dma_async_device_register(struct
>dma_device *device)
> goto err_out;
> }
>
>+ /* One for the channel, one of the class device */
>+ kref_get(&device->refcount);
> kref_get(&device->refcount);
> kref_init(&chan->refcount);
> chan->slow_ref = 0;
>--
>1.5.2.5
>

As Dan said, we've been discussing this offline, and hadn't come to an
agreement yet. My version of the patch is the opposite of yours -
instead of adding a kref_get(), I remove one of the kref_put() calls.
--

When a channel is removed from dmaengine, too many kref_put() calls
are made and the device removal happens too soon, usually causing
a panic.

Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
---

drivers/dma/dmaengine.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 8248992..144a1b7 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -131,7 +131,6 @@ static void dma_async_device_cleanup(struct kref
*kref);
static void dma_class_dev_release(struct class_device *cd)
{
struct dma_chan *chan = container_of(cd, struct dma_chan,
class_dev);
- kref_put(&chan->device->refcount, dma_async_device_cleanup);
}

static struct class dma_devclass = {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2007-10-26 19:07    [W:0.083 / U:0.568 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site