mm: dmapool: use provided gfp flags for all dma_alloc_coherent() calls

Message ID 1352356737-14413-1-git-send-email-m.szyprowski@samsung.com
State New
Headers show

Commit Message

Marek Szyprowski Nov. 8, 2012, 6:38 a.m.
dmapool always calls dma_alloc_coherent() with GFP_ATOMIC flag, regardless
the flags provided by the caller. This causes excessive pruning of
emergency memory pools without any good reason. This patch changes the code
to correctly use gfp flags provided by the dmapool caller. This should
solve the dmapool usage on ARM architecture, where GFP_ATOMIC DMA
allocations can be served only from the special, very limited memory pool.

Reported-by: Soren Moch <smoch@web.de>
Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 mm/dmapool.c |   27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

Comments

Andrew Lunn Nov. 11, 2012, 5:22 p.m. | #1
On Thu, Nov 08, 2012 at 07:38:57AM +0100, Marek Szyprowski wrote:
> dmapool always calls dma_alloc_coherent() with GFP_ATOMIC flag, regardless
> the flags provided by the caller. This causes excessive pruning of
> emergency memory pools without any good reason. This patch changes the code
> to correctly use gfp flags provided by the dmapool caller. This should
> solve the dmapool usage on ARM architecture, where GFP_ATOMIC DMA
> allocations can be served only from the special, very limited memory pool.
> 
> Reported-by: Soren Moch <smoch@web.de>
> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Tested-by: Andrew Lunn <andrew@lunn.ch>

I tested this on a Kirkwood QNAP after removing the call to
init_dma_coherent_pool_size().

	Andrew


> ---
>  mm/dmapool.c |   27 +++++++--------------------
>  1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/mm/dmapool.c b/mm/dmapool.c
> index c5ab33b..86de9b2 100644
> --- a/mm/dmapool.c
> +++ b/mm/dmapool.c
> @@ -62,8 +62,6 @@ struct dma_page {		/* cacheable header for 'allocation' bytes */
>  	unsigned int offset;
>  };
>  
> -#define	POOL_TIMEOUT_JIFFIES	((100 /* msec */ * HZ) / 1000)
> -
>  static DEFINE_MUTEX(pools_lock);
>  
>  static ssize_t
> @@ -227,7 +225,6 @@ static struct dma_page *pool_alloc_page(struct dma_pool *pool, gfp_t mem_flags)
>  		memset(page->vaddr, POOL_POISON_FREED, pool->allocation);
>  #endif
>  		pool_initialise_page(pool, page);
> -		list_add(&page->page_list, &pool->page_list);
>  		page->in_use = 0;
>  		page->offset = 0;
>  	} else {
> @@ -315,30 +312,21 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
>  	might_sleep_if(mem_flags & __GFP_WAIT);
>  
>  	spin_lock_irqsave(&pool->lock, flags);
> - restart:
>  	list_for_each_entry(page, &pool->page_list, page_list) {
>  		if (page->offset < pool->allocation)
>  			goto ready;
>  	}
> -	page = pool_alloc_page(pool, GFP_ATOMIC);
> -	if (!page) {
> -		if (mem_flags & __GFP_WAIT) {
> -			DECLARE_WAITQUEUE(wait, current);
>  
> -			__set_current_state(TASK_UNINTERRUPTIBLE);
> -			__add_wait_queue(&pool->waitq, &wait);
> -			spin_unlock_irqrestore(&pool->lock, flags);
> +	/* pool_alloc_page() might sleep, so temporarily drop &pool->lock */
> +	spin_unlock_irqrestore(&pool->lock, flags);
>  
> -			schedule_timeout(POOL_TIMEOUT_JIFFIES);
> +	page = pool_alloc_page(pool, mem_flags);
> +	if (!page)
> +		return NULL;
>  
> -			spin_lock_irqsave(&pool->lock, flags);
> -			__remove_wait_queue(&pool->waitq, &wait);
> -			goto restart;
> -		}
> -		retval = NULL;
> -		goto done;
> -	}
> +	spin_lock_irqsave(&pool->lock, flags);
>  
> +	list_add(&page->page_list, &pool->page_list);
>   ready:
>  	page->in_use++;
>  	offset = page->offset;
> @@ -348,7 +336,6 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
>  #ifdef	DMAPOOL_DEBUG
>  	memset(retval, POOL_POISON_ALLOCATED, pool->size);
>  #endif
> - done:
>  	spin_unlock_irqrestore(&pool->lock, flags);
>  	return retval;
>  }
> -- 
> 1.7.9.5
>
Soeren Moch Nov. 12, 2012, 9:48 a.m. | #2
On 11.11.2012 18:22, Andrew Lunn wrote:
 > On Thu, Nov 08, 2012 at 07:38:57AM +0100, Marek Szyprowski wrote:
 >> dmapool always calls dma_alloc_coherent() with GFP_ATOMIC flag, 
regardless
 >> the flags provided by the caller. This causes excessive pruning of
 >> emergency memory pools without any good reason. This patch changes 
the code
 >> to correctly use gfp flags provided by the dmapool caller. This should
 >> solve the dmapool usage on ARM architecture, where GFP_ATOMIC DMA
 >> allocations can be served only from the special, very limited memory 
pool.
 >>
 >> Reported-by: Soren Moch <smoch@web.de>
Please use
Reported-by: Soeren Moch <smoch@web.de>

 >> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
 >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
 >
 > Tested-by: Andrew Lunn <andrew@lunn.ch>
 >
 > I tested this on a Kirkwood QNAP after removing the call to
 > init_dma_coherent_pool_size().
 >
 >     Andrew

Tested-by: Soeren Moch <smoch@web.de>

Now I had a chance to test this patch on my Kirkwood guruplug
system with linux-3.6.6 . It is running much better now, but with the
original 256K coherent pool size I still see errors after several hours
of runtime:

Nov 12 09:42:32 guru kernel: ERROR: 256 KiB atomic DMA coherent pool is 
too small!
Nov 12 09:42:32 guru kernel: Please increase it with coherent_pool= 
kernel parameter!

   Soeren

 >> ---
 >>  mm/dmapool.c |   27 +++++++--------------------
 >>  1 file changed, 7 insertions(+), 20 deletions(-)
 >>
 >> diff --git a/mm/dmapool.c b/mm/dmapool.c
 >> index c5ab33b..86de9b2 100644
 >> --- a/mm/dmapool.c
 >> +++ b/mm/dmapool.c
 >> @@ -62,8 +62,6 @@ struct dma_page {        /* cacheable header for 
'allocation' bytes */
 >>      unsigned int offset;
 >>  };
 >>
 >> -#define    POOL_TIMEOUT_JIFFIES    ((100 /* msec */ * HZ) / 1000)
 >> -
 >>  static DEFINE_MUTEX(pools_lock);
 >>
 >>  static ssize_t
 >> @@ -227,7 +225,6 @@ static struct dma_page *pool_alloc_page(struct 
dma_pool *pool, gfp_t mem_flags)
 >>          memset(page->vaddr, POOL_POISON_FREED, pool->allocation);
 >>  #endif
 >>          pool_initialise_page(pool, page);
 >> -        list_add(&page->page_list, &pool->page_list);
 >>          page->in_use = 0;
 >>          page->offset = 0;
 >>      } else {
 >> @@ -315,30 +312,21 @@ void *dma_pool_alloc(struct dma_pool *pool, 
gfp_t mem_flags,
 >>      might_sleep_if(mem_flags & __GFP_WAIT);
 >>
 >>      spin_lock_irqsave(&pool->lock, flags);
 >> - restart:
 >>      list_for_each_entry(page, &pool->page_list, page_list) {
 >>          if (page->offset < pool->allocation)
 >>              goto ready;
 >>      }
 >> -    page = pool_alloc_page(pool, GFP_ATOMIC);
 >> -    if (!page) {
 >> -        if (mem_flags & __GFP_WAIT) {
 >> -            DECLARE_WAITQUEUE(wait, current);
 >>
 >> -            __set_current_state(TASK_UNINTERRUPTIBLE);
 >> -            __add_wait_queue(&pool->waitq, &wait);
 >> -            spin_unlock_irqrestore(&pool->lock, flags);
 >> +    /* pool_alloc_page() might sleep, so temporarily drop 
&pool->lock */
 >> +    spin_unlock_irqrestore(&pool->lock, flags);
 >>
 >> -            schedule_timeout(POOL_TIMEOUT_JIFFIES);
 >> +    page = pool_alloc_page(pool, mem_flags);
 >> +    if (!page)
 >> +        return NULL;
 >>
 >> -            spin_lock_irqsave(&pool->lock, flags);
 >> -            __remove_wait_queue(&pool->waitq, &wait);
 >> -            goto restart;
 >> -        }
 >> -        retval = NULL;
 >> -        goto done;
 >> -    }
 >> +    spin_lock_irqsave(&pool->lock, flags);
 >>
 >> +    list_add(&page->page_list, &pool->page_list);
 >>   ready:
 >>      page->in_use++;
 >>      offset = page->offset;
 >> @@ -348,7 +336,6 @@ void *dma_pool_alloc(struct dma_pool *pool, 
gfp_t mem_flags,
 >>  #ifdef    DMAPOOL_DEBUG
 >>      memset(retval, POOL_POISON_ALLOCATED, pool->size);
 >>  #endif
 >> - done:
 >>      spin_unlock_irqrestore(&pool->lock, flags);
 >>      return retval;
 >>  }
 >> --
 >> 1.7.9.5
 >>
Andrew Lunn Nov. 12, 2012, 10:38 a.m. | #3
On Mon, Nov 12, 2012 at 10:48:02AM +0100, Soeren Moch wrote:
> On 11.11.2012 18:22, Andrew Lunn wrote:
> > On Thu, Nov 08, 2012 at 07:38:57AM +0100, Marek Szyprowski wrote:
> >> dmapool always calls dma_alloc_coherent() with GFP_ATOMIC flag,
> regardless
> >> the flags provided by the caller. This causes excessive pruning of
> >> emergency memory pools without any good reason. This patch
> changes the code
> >> to correctly use gfp flags provided by the dmapool caller. This should
> >> solve the dmapool usage on ARM architecture, where GFP_ATOMIC DMA
> >> allocations can be served only from the special, very limited
> memory pool.
> >>
> >> Reported-by: Soren Moch <smoch@web.de>
> Please use
> Reported-by: Soeren Moch <smoch@web.de>
> 
> >> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >
> > Tested-by: Andrew Lunn <andrew@lunn.ch>
> >
> > I tested this on a Kirkwood QNAP after removing the call to
> > init_dma_coherent_pool_size().
> >
> >     Andrew
> 
> Tested-by: Soeren Moch <smoch@web.de>
> 
> Now I had a chance to test this patch on my Kirkwood guruplug
> system with linux-3.6.6 . It is running much better now, but with the
> original 256K coherent pool size I still see errors after several hours
> of runtime:
> 
> Nov 12 09:42:32 guru kernel: ERROR: 256 KiB atomic DMA coherent pool
> is too small!
> Nov 12 09:42:32 guru kernel: Please increase it with coherent_pool=
> kernel parameter!

Hi Soeren

Could you tell us what DVB devices you are using.

Thanks
	Andrew
Soeren Moch Nov. 12, 2012, 11:03 a.m. | #4
On 12.11.2012 11:38, Andrew Lunn wrote:
> On Mon, Nov 12, 2012 at 10:48:02AM +0100, Soeren Moch wrote:
>> On 11.11.2012 18:22, Andrew Lunn wrote:
>>> On Thu, Nov 08, 2012 at 07:38:57AM +0100, Marek Szyprowski wrote:
>>>> dmapool always calls dma_alloc_coherent() with GFP_ATOMIC flag,
>> regardless
>>>> the flags provided by the caller. This causes excessive pruning of
>>>> emergency memory pools without any good reason. This patch
>> changes the code
>>>> to correctly use gfp flags provided by the dmapool caller. This should
>>>> solve the dmapool usage on ARM architecture, where GFP_ATOMIC DMA
>>>> allocations can be served only from the special, very limited
>> memory pool.
>>>> Reported-by: Soren Moch <smoch@web.de>
>> Please use
>> Reported-by: Soeren Moch <smoch@web.de>
>>
>>>> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Tested-by: Andrew Lunn <andrew@lunn.ch>
>>>
>>> I tested this on a Kirkwood QNAP after removing the call to
>>> init_dma_coherent_pool_size().
>>>
>>>      Andrew
>> Tested-by: Soeren Moch <smoch@web.de>
>>
>> Now I had a chance to test this patch on my Kirkwood guruplug
>> system with linux-3.6.6 . It is running much better now, but with the
>> original 256K coherent pool size I still see errors after several hours
>> of runtime:
>>
>> Nov 12 09:42:32 guru kernel: ERROR: 256 KiB atomic DMA coherent pool
>> is too small!
>> Nov 12 09:42:32 guru kernel: Please increase it with coherent_pool=
>> kernel parameter!
> Hi Soeren
>
> Could you tell us what DVB devices you are using.
>
> Thanks
> 	Andrew

from lsusb:
Bus 001 Device 005: ID 0ccd:00b2 TerraTec Electronic GmbH
Bus 001 Device 006: ID 2040:5200 Hauppauge
Bus 001 Device 009: ID 2304:0242 Pinnacle Systems, Inc.

If you want to check the drivers, I recommend to start with "em28xx".

Regards,
Soeren
Jason Cooper Nov. 19, 2012, 12:18 a.m. | #5
Marek,

I've added the maintainers for mm/*.  Hopefully they can let us know if
this is good for v3.8...

thx,

Jason.

On Thu, Nov 08, 2012 at 07:38:57AM +0100, Marek Szyprowski wrote:
> dmapool always calls dma_alloc_coherent() with GFP_ATOMIC flag, regardless
> the flags provided by the caller. This causes excessive pruning of
> emergency memory pools without any good reason. This patch changes the code
> to correctly use gfp flags provided by the dmapool caller. This should
> solve the dmapool usage on ARM architecture, where GFP_ATOMIC DMA
> allocations can be served only from the special, very limited memory pool.
> 
> Reported-by: Soren Moch <smoch@web.de>
> Reported-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  mm/dmapool.c |   27 +++++++--------------------
>  1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/mm/dmapool.c b/mm/dmapool.c
> index c5ab33b..86de9b2 100644
> --- a/mm/dmapool.c
> +++ b/mm/dmapool.c
> @@ -62,8 +62,6 @@ struct dma_page {		/* cacheable header for 'allocation' bytes */
>  	unsigned int offset;
>  };
>  
> -#define	POOL_TIMEOUT_JIFFIES	((100 /* msec */ * HZ) / 1000)
> -
>  static DEFINE_MUTEX(pools_lock);
>  
>  static ssize_t
> @@ -227,7 +225,6 @@ static struct dma_page *pool_alloc_page(struct dma_pool *pool, gfp_t mem_flags)
>  		memset(page->vaddr, POOL_POISON_FREED, pool->allocation);
>  #endif
>  		pool_initialise_page(pool, page);
> -		list_add(&page->page_list, &pool->page_list);
>  		page->in_use = 0;
>  		page->offset = 0;
>  	} else {
> @@ -315,30 +312,21 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
>  	might_sleep_if(mem_flags & __GFP_WAIT);
>  
>  	spin_lock_irqsave(&pool->lock, flags);
> - restart:
>  	list_for_each_entry(page, &pool->page_list, page_list) {
>  		if (page->offset < pool->allocation)
>  			goto ready;
>  	}
> -	page = pool_alloc_page(pool, GFP_ATOMIC);
> -	if (!page) {
> -		if (mem_flags & __GFP_WAIT) {
> -			DECLARE_WAITQUEUE(wait, current);
>  
> -			__set_current_state(TASK_UNINTERRUPTIBLE);
> -			__add_wait_queue(&pool->waitq, &wait);
> -			spin_unlock_irqrestore(&pool->lock, flags);
> +	/* pool_alloc_page() might sleep, so temporarily drop &pool->lock */
> +	spin_unlock_irqrestore(&pool->lock, flags);
>  
> -			schedule_timeout(POOL_TIMEOUT_JIFFIES);
> +	page = pool_alloc_page(pool, mem_flags);
> +	if (!page)
> +		return NULL;
>  
> -			spin_lock_irqsave(&pool->lock, flags);
> -			__remove_wait_queue(&pool->waitq, &wait);
> -			goto restart;
> -		}
> -		retval = NULL;
> -		goto done;
> -	}
> +	spin_lock_irqsave(&pool->lock, flags);
>  
> +	list_add(&page->page_list, &pool->page_list);
>   ready:
>  	page->in_use++;
>  	offset = page->offset;
> @@ -348,7 +336,6 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
>  #ifdef	DMAPOOL_DEBUG
>  	memset(retval, POOL_POISON_ALLOCATED, pool->size);
>  #endif
> - done:
>  	spin_unlock_irqrestore(&pool->lock, flags);
>  	return retval;
>  }
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Andrew Morton Nov. 19, 2012, 10:48 p.m. | #6
On Sun, 18 Nov 2012 19:18:46 -0500
Jason Cooper <jason@lakedaemon.net> wrote:

> I've added the maintainers for mm/*.  Hopefully they can let us know if
> this is good for v3.8...

As Marek has inexplicably put this patch into linux-next via his tree,
we don't appear to be getting a say in the matter!

The patch looks good to me.  That open-coded wait loop predates the
creation of bitkeeper tree(!) but doesn't appear to be needed.  There
will perhaps be some behavioural changes observable for GFP_KERNEL
callers as dma_pool_alloc() will no longer dip into page reserves but I
see nothing special about dma_pool_alloc() which justifies doing that
anyway.

The patch makes pool->waitq and its manipulation obsolete, but it
failed to remove all that stuff.

The changelog failed to describe the problem which Soren reported. 
That should be included, and as the problem sounds fairly serious we
might decide to backport the fix into -stable kernels.

dma_pool_alloc()'s use of a local "struct dma_page *page" is
distressing - MM developers very much expect a local called "page" to
have type "struct page *".  But that's a separate issue.

As this patch is already in -next and is stuck there for two more
weeks I can't (or at least won't) merge this patch, so I can't help
with any of the above.
Marek Szyprowski Nov. 20, 2012, 10:48 a.m. | #7
Hello,

On 11/19/2012 11:48 PM, Andrew Morton wrote:
> On Sun, 18 Nov 2012 19:18:46 -0500
> Jason Cooper <jason@lakedaemon.net> wrote:
>
> > I've added the maintainers for mm/*.  Hopefully they can let us know if
> > this is good for v3.8...
>
> As Marek has inexplicably put this patch into linux-next via his tree,
> we don't appear to be getting a say in the matter!

I've just put this patch to linux-next via my dma-mapping tree to give it
some testing asap to check if other changes to arm dma-mapping are required
or not.

> The patch looks good to me.  That open-coded wait loop predates the
> creation of bitkeeper tree(!) but doesn't appear to be needed.  There
> will perhaps be some behavioural changes observable for GFP_KERNEL
> callers as dma_pool_alloc() will no longer dip into page reserves but I
> see nothing special about dma_pool_alloc() which justifies doing that
> anyway.
>
> The patch makes pool->waitq and its manipulation obsolete, but it
> failed to remove all that stuff.

Right, I missed that part, I will update it asap.

> The changelog failed to describe the problem which Soren reported.
> That should be included, and as the problem sounds fairly serious we
> might decide to backport the fix into -stable kernels.

Ok, I will extend the changelog.

> dma_pool_alloc()'s use of a local "struct dma_page *page" is
> distressing - MM developers very much expect a local called "page" to
> have type "struct page *".  But that's a separate issue.

I will prepare a separate patch cleaning it. I was also a bit surprised
by such naming scheme, but it is probably related to the fact that this
come has not been touched much since a very ancient times.

> As this patch is already in -next and is stuck there for two more
> weeks I can't (or at least won't) merge this patch, so I can't help
> with any of the above.

I will fix both issues in the next version of the patch. Would like to
merge it to your tree or should I keep it in my dma-mapping tree?

Best regards
Andrew Morton Nov. 20, 2012, 7:52 p.m. | #8
On Tue, 20 Nov 2012 11:48:44 +0100
Marek Szyprowski <m.szyprowski@samsung.com> wrote:

> > As this patch is already in -next and is stuck there for two more
> > weeks I can't (or at least won't) merge this patch, so I can't help
> > with any of the above.
> 
> I will fix both issues in the next version of the patch. Would like to
> merge it to your tree or should I keep it in my dma-mapping tree?

The patch looks OK to me now (still wondering about the -stable
question though).

It would be a bit of a pain for me to carry a patch which conflicts
with one in linux-next, and this patch doesn't appear to conflict with
the other pending dmapool.c patches in -mm so you may as well keep it
in the dma-mapping tree now.

Patch hide | download patch | download mbox

diff --git a/mm/dmapool.c b/mm/dmapool.c
index c5ab33b..86de9b2 100644
--- a/mm/dmapool.c
+++ b/mm/dmapool.c
@@ -62,8 +62,6 @@  struct dma_page {		/* cacheable header for 'allocation' bytes */
 	unsigned int offset;
 };
 
-#define	POOL_TIMEOUT_JIFFIES	((100 /* msec */ * HZ) / 1000)
-
 static DEFINE_MUTEX(pools_lock);
 
 static ssize_t
@@ -227,7 +225,6 @@  static struct dma_page *pool_alloc_page(struct dma_pool *pool, gfp_t mem_flags)
 		memset(page->vaddr, POOL_POISON_FREED, pool->allocation);
 #endif
 		pool_initialise_page(pool, page);
-		list_add(&page->page_list, &pool->page_list);
 		page->in_use = 0;
 		page->offset = 0;
 	} else {
@@ -315,30 +312,21 @@  void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
 	might_sleep_if(mem_flags & __GFP_WAIT);
 
 	spin_lock_irqsave(&pool->lock, flags);
- restart:
 	list_for_each_entry(page, &pool->page_list, page_list) {
 		if (page->offset < pool->allocation)
 			goto ready;
 	}
-	page = pool_alloc_page(pool, GFP_ATOMIC);
-	if (!page) {
-		if (mem_flags & __GFP_WAIT) {
-			DECLARE_WAITQUEUE(wait, current);
 
-			__set_current_state(TASK_UNINTERRUPTIBLE);
-			__add_wait_queue(&pool->waitq, &wait);
-			spin_unlock_irqrestore(&pool->lock, flags);
+	/* pool_alloc_page() might sleep, so temporarily drop &pool->lock */
+	spin_unlock_irqrestore(&pool->lock, flags);
 
-			schedule_timeout(POOL_TIMEOUT_JIFFIES);
+	page = pool_alloc_page(pool, mem_flags);
+	if (!page)
+		return NULL;
 
-			spin_lock_irqsave(&pool->lock, flags);
-			__remove_wait_queue(&pool->waitq, &wait);
-			goto restart;
-		}
-		retval = NULL;
-		goto done;
-	}
+	spin_lock_irqsave(&pool->lock, flags);
 
+	list_add(&page->page_list, &pool->page_list);
  ready:
 	page->in_use++;
 	offset = page->offset;
@@ -348,7 +336,6 @@  void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
 #ifdef	DMAPOOL_DEBUG
 	memset(retval, POOL_POISON_ALLOCATED, pool->size);
 #endif
- done:
 	spin_unlock_irqrestore(&pool->lock, flags);
 	return retval;
 }