From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 86F17C433F5 for ; Tue, 31 May 2022 18:23:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 232BE6B0074; Tue, 31 May 2022 14:23:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1E18E6B0075; Tue, 31 May 2022 14:23:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0CF256B0078; Tue, 31 May 2022 14:23:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id F28166B0074 for ; Tue, 31 May 2022 14:23:57 -0400 (EDT) Received: from smtpin31.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id C1A3833A34 for ; Tue, 31 May 2022 18:23:57 +0000 (UTC) X-FDA: 79526862114.31.822F142 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf28.hostedemail.com (Postfix) with ESMTP id ECC72C0067 for ; Tue, 31 May 2022 18:23:18 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3012523A; Tue, 31 May 2022 11:23:56 -0700 (PDT) Received: from [10.57.81.38] (unknown [10.57.81.38]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3D3183F766; Tue, 31 May 2022 11:23:54 -0700 (PDT) Message-ID: <8a86ff8b-ea16-4004-82e4-d20e46501c75@arm.com> Date: Tue, 31 May 2022 19:23:49 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH 01/10] dmapool: remove checks for dev == NULL Content-Language: en-GB To: Tony Battersby , linux-mm@kvack.org, linux-kernel@vger.kernel.org Cc: iommu@lists.linux-foundation.org, kernel-team@fb.com, Matthew Wilcox , Keith Busch , Andy Shevchenko , Tony Lindgren References: <9b08ab7c-b80b-527d-9adf-7716b0868fbc@cybernetics.com> <7f6f9ff5-cdb9-e386-988d-fa013538dee7@cybernetics.com> From: Robin Murphy In-Reply-To: <7f6f9ff5-cdb9-e386-988d-fa013538dee7@cybernetics.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: ECC72C0067 X-Stat-Signature: dtd4w6demg5xn5ac8ubnnh7yiahmpgzr Authentication-Results: imf28.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf28.hostedemail.com: domain of robin.murphy@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=robin.murphy@arm.com X-HE-Tag: 1654021398-725405 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: 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 > Signed-off-by: Tony Battersby > --- > 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; > } > }