[PATCHv3 09/12] dmapool: simplify freeing

Keith Busch posted 12 patches 2 years, 8 months ago
There is a newer version of this series
[PATCHv3 09/12] dmapool: simplify freeing
Posted by Keith Busch 2 years, 8 months ago
From: Keith Busch <kbusch@kernel.org>

The actions for busy and not busy are mostly the same, so combine these
and remove the unnecessary function. Also, the pool is about to be freed
so there's no need to poison the page data since we only check for
poison on alloc, which can't be done on a freed pool.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 mm/dmapool.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/mm/dmapool.c b/mm/dmapool.c
index 6862b4e763891..4dab48e7e0d75 100644
--- a/mm/dmapool.c
+++ b/mm/dmapool.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * DMA Pool allocator
+* DMA Pool allocator
  *
  * Copyright 2001 David Brownell
  * Copyright 2007 Intel Corporation
@@ -241,18 +241,6 @@ static inline bool is_page_busy(struct dma_page *page)
 	return page->in_use != 0;
 }
 
-static void pool_free_page(struct dma_pool *pool, struct dma_page *page)
-{
-	dma_addr_t dma = page->dma;
-
-#ifdef	DMAPOOL_DEBUG
-	memset(page->vaddr, POOL_POISON_FREED, pool->allocation);
-#endif
-	dma_free_coherent(pool->dev, pool->allocation, page->vaddr, dma);
-	list_del(&page->page_list);
-	kfree(page);
-}
-
 /**
  * dma_pool_destroy - destroys a pool of dma memory blocks.
  * @pool: dma pool that will be destroyed
@@ -280,14 +268,14 @@ void dma_pool_destroy(struct dma_pool *pool)
 	mutex_unlock(&pools_reg_lock);
 
 	list_for_each_entry_safe(page, tmp, &pool->page_list, page_list) {
-		if (is_page_busy(page)) {
+		if (!is_page_busy(page))
+			dma_free_coherent(pool->dev, pool->allocation,
+					  page->vaddr, page->dma);
+		else
 			dev_err(pool->dev, "%s %s, %p busy\n", __func__,
 				pool->name, page->vaddr);
-			/* leak the still-in-use consistent memory */
-			list_del(&page->page_list);
-			kfree(page);
-		} else
-			pool_free_page(pool, page);
+		list_del(&page->page_list);
+		kfree(page);
 	}
 
 	kfree(pool);
-- 
2.30.2
Re: [PATCHv3 09/12] dmapool: simplify freeing
Posted by Christoph Hellwig 2 years, 8 months ago
> - * DMA Pool allocator
> +* DMA Pool allocator

This got corrupted somehow.

> +		if (!is_page_busy(page))
> +			dma_free_coherent(pool->dev, pool->allocation,
> +					  page->vaddr, page->dma);
> +		else
>  			dev_err(pool->dev, "%s %s, %p busy\n", __func__,
>  				pool->name, page->vaddr);
> +		list_del(&page->page_list);
> +		kfree(page);

I'm still not sure what the point of leaking the page in case it is
busy vs letting KASAN and friends actually catch it, but the pure
rearrangement is an improvement over the previous state, so:

Reviewed-by: Christoph Hellwig <hch@lst.de>