Re: [PATCH 1/2] dmaengine: TXx9 Soc DMA Controller driver

[Atsushi Nemoto]

On Mon, 16 Mar 2009 21:52:10 -0700, Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
> > Hmm.. why idr_ref is dynamically allocated?  Just putting it in
> > dma_device makes thing more simple, no?
> 
> The sysfs device has a longer lifetime than dma_device.  See commit
> 41d5e59c [1].

The sysfs device for dma_chan (dma_chan_dev) has a shorter lifetime
than dma_device, doesn't it?

I mean something like this (only compile tested):
------------------------------------------------------
Subject: dmaengine: Add idr_ref to dma_device

This fixes memory leak on some error path.

Signed-off-by: Atsushi Nemoto <anemo@xxxxxxxxxxxxx>
---
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 280a9d2..0708931 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -153,7 +153,6 @@ static void chan_dev_release(struct device *dev)
 		mutex_lock(&dma_list_mutex);
 		idr_remove(&dma_idr, chan_dev->dev_id);
 		mutex_unlock(&dma_list_mutex);
-		kfree(chan_dev->idr_ref);
 	}
 	kfree(chan_dev);
 }
@@ -610,7 +609,6 @@ int dma_async_device_register(struct dma_device *device)
 {
 	int chancnt = 0, rc;
 	struct dma_chan* chan;
-	atomic_t *idr_ref;
 
 	if (!device)
 		return -ENODEV;
@@ -637,10 +635,7 @@ int dma_async_device_register(struct dma_device *device)
 	BUG_ON(!device->device_issue_pending);
 	BUG_ON(!device->dev);
 
-	idr_ref = kmalloc(sizeof(*idr_ref), GFP_KERNEL);
-	if (!idr_ref)
-		return -ENOMEM;
-	atomic_set(idr_ref, 0);
+	atomic_set(&device->idr_ref, 0);
  idr_retry:
 	if (!idr_pre_get(&dma_idr, GFP_KERNEL))
 		return -ENOMEM;
@@ -667,9 +662,9 @@ int dma_async_device_register(struct dma_device *device)
 		chan->dev->device.class = &dma_devclass;
 		chan->dev->device.parent = device->dev;
 		chan->dev->chan = chan;
-		chan->dev->idr_ref = idr_ref;
+		chan->dev->idr_ref = &device->idr_ref;
 		chan->dev->dev_id = device->dev_id;
-		atomic_inc(idr_ref);
+		atomic_inc(&device->idr_ref);
 		dev_set_name(&chan->dev->device, "dma%dchan%d",
 			     device->dev_id, chan->chan_id);
 
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 1956c8d..9e99d82 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -234,6 +234,7 @@ struct dma_device {
 
 	int dev_id;
 	struct device *dev;
+	atomic_t idr_ref;
 
 	int (*device_alloc_chan_resources)(struct dma_chan *chan);
 	void (*device_free_chan_resources)(struct dma_chan *chan);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/