Messages in this thread | | | Date | Tue, 31 May 2022 19:23:49 +0100 | Subject | Re: [PATCH 01/10] dmapool: remove checks for dev == NULL | From | Robin Murphy <> |
| |
On 2022-05-31 19:12, Tony Battersby wrote: > dmapool originally tried to support pools without a device because > dma_alloc_coherent() supports allocations without a device. But nobody > ended up using dma pools without a device, so the current checks in > dmapool.c for pool->dev == NULL are both insufficient and causing bloat. > Remove them.
Furthermore, dma_pool_create() already dereferences the dev argument unconditionally, so there's no way we could possibly get as far as these checks even if a caller did ever pass NULL.
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Tony Battersby <tonyb@cybernetics.com> > --- > mm/dmapool.c | 42 +++++++++++------------------------------- > 1 file changed, 11 insertions(+), 31 deletions(-) > > diff --git a/mm/dmapool.c b/mm/dmapool.c > index a7eb5d0eb2da..0f89de408cbe 100644 > --- a/mm/dmapool.c > +++ b/mm/dmapool.c > @@ -275,7 +275,7 @@ void dma_pool_destroy(struct dma_pool *pool) > mutex_lock(&pools_reg_lock); > mutex_lock(&pools_lock); > list_del(&pool->pools); > - if (pool->dev && list_empty(&pool->dev->dma_pools)) > + if (list_empty(&pool->dev->dma_pools)) > empty = true; > mutex_unlock(&pools_lock); > if (empty) > @@ -284,12 +284,8 @@ void dma_pool_destroy(struct dma_pool *pool) > > list_for_each_entry_safe(page, tmp, &pool->page_list, page_list) { > if (is_page_busy(page)) { > - if (pool->dev) > - dev_err(pool->dev, "%s %s, %p busy\n", __func__, > - pool->name, page->vaddr); > - else > - pr_err("%s %s, %p busy\n", __func__, > - pool->name, page->vaddr); > + 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); > @@ -351,12 +347,8 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags, > for (i = sizeof(page->offset); i < pool->size; i++) { > if (data[i] == POOL_POISON_FREED) > continue; > - if (pool->dev) > - dev_err(pool->dev, "%s %s, %p (corrupted)\n", > - __func__, pool->name, retval); > - else > - pr_err("%s %s, %p (corrupted)\n", > - __func__, pool->name, retval); > + dev_err(pool->dev, "%s %s, %p (corrupted)\n", > + __func__, pool->name, retval); > > /* > * Dump the first 4 bytes even if they are not > @@ -411,12 +403,8 @@ void dma_pool_free(struct dma_pool *pool, void *vaddr, dma_addr_t dma) > page = pool_find_page(pool, dma); > if (!page) { > spin_unlock_irqrestore(&pool->lock, flags); > - if (pool->dev) > - dev_err(pool->dev, "%s %s, %p/%pad (bad dma)\n", > - __func__, pool->name, vaddr, &dma); > - else > - pr_err("%s %s, %p/%pad (bad dma)\n", > - __func__, pool->name, vaddr, &dma); > + dev_err(pool->dev, "%s %s, %p/%pad (bad dma)\n", > + __func__, pool->name, vaddr, &dma); > return; > } > > @@ -426,12 +414,8 @@ void dma_pool_free(struct dma_pool *pool, void *vaddr, dma_addr_t dma) > #ifdef DMAPOOL_DEBUG > if ((dma - page->dma) != offset) { > spin_unlock_irqrestore(&pool->lock, flags); > - if (pool->dev) > - dev_err(pool->dev, "%s %s, %p (bad vaddr)/%pad\n", > - __func__, pool->name, vaddr, &dma); > - else > - pr_err("%s %s, %p (bad vaddr)/%pad\n", > - __func__, pool->name, vaddr, &dma); > + dev_err(pool->dev, "%s %s, %p (bad vaddr)/%pad\n", > + __func__, pool->name, vaddr, &dma); > return; > } > { > @@ -442,12 +426,8 @@ void dma_pool_free(struct dma_pool *pool, void *vaddr, dma_addr_t dma) > continue; > } > spin_unlock_irqrestore(&pool->lock, flags); > - if (pool->dev) > - dev_err(pool->dev, "%s %s, dma %pad already free\n", > - __func__, pool->name, &dma); > - else > - pr_err("%s %s, dma %pad already free\n", > - __func__, pool->name, &dma); > + dev_err(pool->dev, "%s %s, dma %pad already free\n", > + __func__, pool->name, &dma); > return; > } > }
| |