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 A99E3C4828F for ; Fri, 9 Feb 2024 13:44:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B42016B0071; Fri, 9 Feb 2024 08:44:32 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id ACAC06B0072; Fri, 9 Feb 2024 08:44:32 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 96B5F6B0074; Fri, 9 Feb 2024 08:44:32 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 828F16B0071 for ; Fri, 9 Feb 2024 08:44:32 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 9FF2981065 for ; Fri, 9 Feb 2024 13:44:31 +0000 (UTC) X-FDA: 81772385142.02.03BC956 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf04.hostedemail.com (Postfix) with ESMTP id E7D574000B for ; Fri, 9 Feb 2024 13:44:28 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf04.hostedemail.com: domain of robin.murphy@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=robin.murphy@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707486269; a=rsa-sha256; cv=none; b=LMiaauMDNtMLGZFLx6xFzqtKrEbwiTxraJXsvzEA6m6vSJFz8mPtQwSAQy/bRDrNUO8KR6 VMHnnTATCJWhiI9ILz/qGTDU1IGWt0fVJobhGSJYfJ9hFBuNp24s1j1xzIfCY9jHzDZ7p/ rXwQ+r6G0wvu6MVEf/AoFXx30EEth7Q= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf04.hostedemail.com: domain of robin.murphy@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=robin.murphy@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707486269; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=D7vY47pTXnGid8jsuYq3/+j1OEV/uxlcoMSDDlpxr24=; b=M9SQsKgJEk8jS4wcQHaJoBF5hUvGGwT8vCLeugM8uFjVwJOepbMqnCG5SA3+LStR+qPTZA vfq6AH72t4LIe2hzcEbL4+BJDwF212ApLJF1c8goLAXQSiMFjsSh23EMu++o24R9sCnqL5 mlSnY2GyUO22ikZdxM+3qm1HGapWkC0= 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 B9D9BDA7; Fri, 9 Feb 2024 05:45:09 -0800 (PST) Received: from [10.57.47.119] (unknown [10.57.47.119]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6B0283F762; Fri, 9 Feb 2024 05:44:21 -0800 (PST) Message-ID: <8ce2cd7b-7702-45aa-b4c8-25a01c27ed83@arm.com> Date: Fri, 9 Feb 2024 13:44:20 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v4 01/10] iommu/vt-d: add wrapper functions for page allocations Content-Language: en-GB To: Pasha Tatashin , akpm@linux-foundation.org, alim.akhtar@samsung.com, alyssa@rosenzweig.io, asahi@lists.linux.dev, baolu.lu@linux.intel.com, bhelgaas@google.com, cgroups@vger.kernel.org, corbet@lwn.net, david@redhat.com, dwmw2@infradead.org, hannes@cmpxchg.org, heiko@sntech.de, iommu@lists.linux.dev, jernej.skrabec@gmail.com, jonathanh@nvidia.com, joro@8bytes.org, krzysztof.kozlowski@linaro.org, linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-rockchip@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-sunxi@lists.linux.dev, linux-tegra@vger.kernel.org, lizefan.x@bytedance.com, marcan@marcan.st, mhiramat@kernel.org, m.szyprowski@samsung.com, paulmck@kernel.org, rdunlap@infradead.org, samuel@sholland.org, suravee.suthikulpanit@amd.com, sven@svenpeter.dev, thierry.reding@gmail.com, tj@kernel.org, tomas.mudrunka@gmail.com, vdumpa@nvidia.com, wens@csie.org, will@kernel.org, yu-cheng.yu@intel.com, rientjes@google.com, bagasdotme@gmail.com, mkoutny@suse.com References: <20240207174102.1486130-1-pasha.tatashin@soleen.com> <20240207174102.1486130-2-pasha.tatashin@soleen.com> From: Robin Murphy In-Reply-To: <20240207174102.1486130-2-pasha.tatashin@soleen.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: E7D574000B X-Stat-Signature: xwwznc9gq4weita5rk3u4kb9cegbmdc1 X-Rspam-User: X-HE-Tag: 1707486268-830718 X-HE-Meta: U2FsdGVkX1/sYsqxumE5dPGCcGkSzhGRH7AcQgSpw1TRZbhoMEnd6yMAy2ntHa1DdMCXCGfXDu97WWfGCsvcGvvWsvH/Qe0Kpxa8ImDYvg2DdGbKVbktl2wAXBKaGXDZtp0Jc8YVN1jV7zMRBp7oaxz/IfcLxAEW9iJVhvERjCV3Dsw8lMBpKmvKloayvJkHD3B1IJYHUEicF9ZzYYxdrLIY8lOfU1dtY4P1ZbagpDe9anTkAnI+tPX2Bgd9YHX04VwqpS+xwbQQN22E2YoJkcr/SwJoVQTlxP290ouBEvfzPU+Ex3GDBO+1pwK7zsJt7SWX09jo/fISWc9nx9QD3fcC/7d/EvofJiIBWHbcje5t/2LH+WLUphXlIKM0Lt5qyf7zsbwJiqOfsWJ69gNs9mxGYHf1EEDnWl38mxC68SzEufVjPm4TV5txHx0vW7f9ctwizqofKfF0dqzqXLkwqIzXrP0p+etB3BiKKzouse3rJuyDQRs54F6m+3om7OSnhN75SHiKYSGR86fo/IdWT34v09hcEFMsPvfVZIXzqIEFZbg24KRGq36sXCDThnzd65ID39r6wZHsZSMxM/uy3ox3M+WAD7uRruSSghDG3vX9iGCl2Cm2YWJCpQCP09HR+8o/xJf6saTLNF1D5LtAckRBHVGGaF/rMC5qzK3q5Mf0qJUitqSuSyHWnZ2V3PyRsmBRUrqi7EiQ2eMwWTAflK2iC2TifsRjPFFU/f8YAvq11rvLegAaVNUarUoOga/JqJLdS2Jqfpp7FY5P+basN4WZSMqUcUrRHBdJ8+U/g2Jz1NKxCjhDDM+bMM50hoZVNXd6OlSyD+LwZkQb+qbVAvo092z44zHM68Iglo5BTIW0xGomFtjQxxS8HvosBE58h0LYTiY9lAfbhG0KUfnR213cwz2yYTRVeHi0OblO14Q63uuhLlV/Em4kedr3QassA1D1/8SLcCTZMO3NiS9 NhJwAu++ NhRSmi16aPQ7FJZ2M5lXPhlGPnsSXKU04QXqGRF538GDMMSMblgO9ccrLjlJulwQCQy5lUqWtBG3Ra9KpMq1TxyGg5inIB/sSrt0h/jwpikVXYXTKCmTUy6I4rXqHLs3iZfYenQSIXEvEl52rqMXE2VdrKIQqaY7EtL6kS7ysOq+gwqxxHdDR6Oqr6ICCvQ/lqwdlLuc15g4lbpmfivcJSoxXA4zzEIo1ocM0Ar1kkqRbYxJb3WxCS24+UQwHQSyADEGE89ZNBnln2iC1oCHR7vSRFA== 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: List-Subscribe: List-Unsubscribe: On 2024-02-07 5:40 pm, Pasha Tatashin wrote: [...]> diff --git a/drivers/iommu/iommu-pages.h b/drivers/iommu/iommu-pages.h > new file mode 100644 > index 000000000000..c412d0aaa399 > --- /dev/null > +++ b/drivers/iommu/iommu-pages.h > @@ -0,0 +1,204 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2024, Google LLC. > + * Pasha Tatashin > + */ > + > +#ifndef __IOMMU_PAGES_H > +#define __IOMMU_PAGES_H > + > +#include > +#include > +#include > + > +/* > + * All page allocation that are performed in the IOMMU subsystem must use one of "All page allocations" is too broad; As before, this is only about pagetable allocations, or I guess for the full nuance, allocations of pagetables and other per-iommu_domain configuration structures which are reasonable to report as "pagetables" to userspace. > + * the functions below. This is necessary for the proper accounting as IOMMU > + * state can be rather large, i.e. multiple gigabytes in size. > + */ > + > +/** > + * __iommu_alloc_pages_node - allocate a zeroed page of a given order from > + * specific NUMA node. > + * @nid: memory NUMA node id > + * @gfp: buddy allocator flags > + * @order: page order > + * > + * returns the head struct page of the allocated page. > + */ > +static inline struct page *__iommu_alloc_pages_node(int nid, gfp_t gfp, > + int order) > +{ > + struct page *page; > + > + page = alloc_pages_node(nid, gfp | __GFP_ZERO, order); > + if (unlikely(!page)) > + return NULL; > + > + return page; > +} All 3 invocations of this only use the returned struct page to trivially derive page_address(), so we really don't need it; just clean up these callsites a bit more. > + > +/** > + * __iommu_alloc_pages - allocate a zeroed page of a given order. > + * @gfp: buddy allocator flags > + * @order: page order > + * > + * returns the head struct page of the allocated page. > + */ > +static inline struct page *__iommu_alloc_pages(gfp_t gfp, int order) > +{ > + struct page *page; > + > + page = alloc_pages(gfp | __GFP_ZERO, order); > + if (unlikely(!page)) > + return NULL; > + > + return page; > +} Same for the single invocation of this one. > + > +/** > + * __iommu_alloc_page_node - allocate a zeroed page at specific NUMA node. > + * @nid: memory NUMA node id > + * @gfp: buddy allocator flags > + * > + * returns the struct page of the allocated page. > + */ > +static inline struct page *__iommu_alloc_page_node(int nid, gfp_t gfp) > +{ > + return __iommu_alloc_pages_node(nid, gfp, 0); > +} There are no users of this at all. > + > +/** > + * __iommu_alloc_page - allocate a zeroed page > + * @gfp: buddy allocator flags > + * > + * returns the struct page of the allocated page. > + */ > +static inline struct page *__iommu_alloc_page(gfp_t gfp) > +{ > + return __iommu_alloc_pages(gfp, 0); > +} > + > +/** > + * __iommu_free_pages - free page of a given order > + * @page: head struct page of the page > + * @order: page order > + */ > +static inline void __iommu_free_pages(struct page *page, int order) > +{ > + if (!page) > + return; > + > + __free_pages(page, order); > +} > + > +/** > + * __iommu_free_page - free page > + * @page: struct page of the page > + */ > +static inline void __iommu_free_page(struct page *page) > +{ > + __iommu_free_pages(page, 0); > +} Beyond one more trivial Intel cleanup for __iommu_alloc_pages(), these 3 are then only used by tegra-smmu, so honestly I'd be inclined to just open-code there page_address()/virt_to_page() conversions as appropriate there (once again I think the whole thing could in fact be refactored to not use struct pages at all because all it's ever ultimately doing with them is page_address(), but that would be a bigger job so definitely out-of-scope for this series). > + > +/** > + * iommu_alloc_pages_node - allocate a zeroed page of a given order from > + * specific NUMA node. > + * @nid: memory NUMA node id > + * @gfp: buddy allocator flags > + * @order: page order > + * > + * returns the virtual address of the allocated page > + */ > +static inline void *iommu_alloc_pages_node(int nid, gfp_t gfp, int order) > +{ > + struct page *page = __iommu_alloc_pages_node(nid, gfp, order); > + > + if (unlikely(!page)) > + return NULL; As a general point I'd prefer to fold these checks into the accounting function itself rather than repeat them all over. > + > + return page_address(page); > +} > + > +/** > + * iommu_alloc_pages - allocate a zeroed page of a given order > + * @gfp: buddy allocator flags > + * @order: page order > + * > + * returns the virtual address of the allocated page > + */ > +static inline void *iommu_alloc_pages(gfp_t gfp, int order) > +{ > + struct page *page = __iommu_alloc_pages(gfp, order); > + > + if (unlikely(!page)) > + return NULL; > + > + return page_address(page); > +} > + > +/** > + * iommu_alloc_page_node - allocate a zeroed page at specific NUMA node. > + * @nid: memory NUMA node id > + * @gfp: buddy allocator flags > + * > + * returns the virtual address of the allocated page > + */ > +static inline void *iommu_alloc_page_node(int nid, gfp_t gfp) > +{ > + return iommu_alloc_pages_node(nid, gfp, 0); > +} TBH I'm not entirely convinced that saving 4 characters per invocation times 11 invocations makes this wrapper worthwhile :/ > + > +/** > + * iommu_alloc_page - allocate a zeroed page > + * @gfp: buddy allocator flags > + * > + * returns the virtual address of the allocated page > + */ > +static inline void *iommu_alloc_page(gfp_t gfp) > +{ > + return iommu_alloc_pages(gfp, 0); > +} > + > +/** > + * iommu_free_pages - free page of a given order > + * @virt: virtual address of the page to be freed. > + * @order: page order > + */ > +static inline void iommu_free_pages(void *virt, int order) > +{ > + if (!virt) > + return; > + > + __iommu_free_pages(virt_to_page(virt), order); > +} > + > +/** > + * iommu_free_page - free page > + * @virt: virtual address of the page to be freed. > + */ > +static inline void iommu_free_page(void *virt) > +{ > + iommu_free_pages(virt, 0); > +} > + > +/** > + * iommu_free_pages_list - free a list of pages. > + * @page: the head of the lru list to be freed. > + * > + * There are no locking requirement for these pages, as they are going to be > + * put on a free list as soon as refcount reaches 0. Pages are put on this LRU > + * list once they are removed from the IOMMU page tables. However, they can > + * still be access through debugfs. > + */ > +static inline void iommu_free_pages_list(struct list_head *page) Nit: I'd be inclined to call this iommu_put_pages_list for consistency. > +{ > + while (!list_empty(page)) { > + struct page *p = list_entry(page->prev, struct page, lru); > + > + list_del(&p->lru); > + put_page(p); > + } > +} I realise now you've also missed the common freelist freeing sites in iommu-dma. Thanks, Robin. > + > +#endif /* __IOMMU_PAGES_H */