From: Robin Murphy <robin.murphy@arm.com>
To: Tony Battersby <tonyb@cybernetics.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Cc: iommu@lists.linux-foundation.org, kernel-team@fb.com,
Matthew Wilcox <willy@infradead.org>,
Keith Busch <kbusch@kernel.org>,
Andy Shevchenko <andy.shevchenko@gmail.com>,
Tony Lindgren <tony@atomide.com>
Subject: Re: [PATCH 01/10] dmapool: remove checks for dev == NULL
Date: Tue, 31 May 2022 19:23:49 +0100 [thread overview]
Message-ID: <8a86ff8b-ea16-4004-82e4-d20e46501c75@arm.com> (raw)
In-Reply-To: <7f6f9ff5-cdb9-e386-988d-fa013538dee7@cybernetics.com>
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;
> }
> }
next prev parent reply other threads:[~2022-05-31 18:24 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-31 18:11 [PATCH 00/10] mpt3sas and dmapool scalability Tony Battersby
2022-05-31 18:12 ` [PATCH 01/10] dmapool: remove checks for dev == NULL Tony Battersby
2022-05-31 18:23 ` Robin Murphy [this message]
2022-05-31 18:13 ` [PATCH 02/10] dmapool: cleanup integer types Tony Battersby
2022-05-31 18:14 ` [PATCH 03/10] dmapool: fix boundary comparison Tony Battersby
2022-05-31 18:17 ` [PATCH 04/10] dmapool: improve accuracy of debug statistics Tony Battersby
2022-05-31 19:48 ` Robin Murphy
2022-05-31 19:52 ` Tony Battersby
2022-05-31 21:55 ` Robin Murphy
2022-05-31 18:18 ` [PATCH 05/10] dmapool: debug: prevent endless loop in case of corruption Tony Battersby
2022-05-31 18:20 ` [PATCH 06/10] dmapool: ignore init_on_free when DMAPOOL_DEBUG enabled Tony Battersby
2022-05-31 18:21 ` [PATCH 07/10] dmapool: speedup DMAPOOL_DEBUG with init_on_alloc Tony Battersby
2022-05-31 18:22 ` [PATCH 08/10] dmapool: cleanup dma_pool_destroy Tony Battersby
2022-05-31 19:33 ` Robin Murphy
2022-05-31 21:40 ` Keith Busch
2022-05-31 18:23 ` [PATCH 09/10] dmapool: improve scalability of dma_pool_alloc Tony Battersby
2022-05-31 18:23 ` [PATCH 10/10] dmapool: improve scalability of dma_pool_free Tony Battersby
2022-05-31 21:54 ` Keith Busch
2022-05-31 22:10 ` Tony Battersby
2022-06-01 9:44 ` Robin Murphy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8a86ff8b-ea16-4004-82e4-d20e46501c75@arm.com \
--to=robin.murphy@arm.com \
--cc=andy.shevchenko@gmail.com \
--cc=iommu@lists.linux-foundation.org \
--cc=kbusch@kernel.org \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=tony@atomide.com \
--cc=tonyb@cybernetics.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).