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 EC5B4C48295 for ; Mon, 5 Feb 2024 15:31:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 868566B007B; Mon, 5 Feb 2024 10:31:29 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 818946B007D; Mon, 5 Feb 2024 10:31:29 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6E0586B007E; Mon, 5 Feb 2024 10:31:29 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 5DD756B007B for ; Mon, 5 Feb 2024 10:31:29 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id EF918C04CD for ; Mon, 5 Feb 2024 15:31:28 +0000 (UTC) X-FDA: 81758139456.11.8BAB43E Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf03.hostedemail.com (Postfix) with ESMTP id B360120020 for ; Mon, 5 Feb 2024 15:31:26 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf03.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=1707147087; 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=mp3IbGdobr8uzkQzYSbSLbVOWucq7keRuo96uT1ChWI=; b=Qm+TQHQnVsJtT5V4S1leIo6GHrny48mHHiO7D7TbMg46AtzaCehxPPc2lk9vuGg2C9DUiz I5nEEOnbR63CWEBctYZ/pdhaGzDwx/pveNLjcVwGnKkpzzNJYVmNGR47zBN6khPuSvrYig NG2c3XRh7ffLw9iFa385nWXmKE5EPGo= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf03.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=1707147087; a=rsa-sha256; cv=none; b=JzxHVFaX8XPRv6IN1cIlsGb8Q3IKolrQCGu5HKmYX5JPQrqNTf5hy2IEPIi3neL5WKBD6X TH8WBciBf7d1I1r7z0bn/Dr0w56kee8MdCZNIaU954lHdxrb11zYQiQC61JC+wgZ0GZ6eP qSLrKnWwxxIOHCVd+zn8N6Hfb1hZyY8= 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 365A01FB; Mon, 5 Feb 2024 07:32:08 -0800 (PST) Received: from [10.57.48.140] (unknown [10.57.48.140]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 065C03F5A1; Mon, 5 Feb 2024 07:31:23 -0800 (PST) Message-ID: Date: Mon, 5 Feb 2024 15:31:21 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] iommu/iova: use named kmem_cache for iova magazines Content-Language: en-GB To: Pasha Tatashin , joro@8bytes.org, will@kernel.org, iommu@lists.linux.dev, linux-kernel@vger.kernel.org, linux-mm@kvack.org, rientjes@google.com, yosryahmed@google.com References: <20240202192820.536408-1-pasha.tatashin@soleen.com> From: Robin Murphy In-Reply-To: <20240202192820.536408-1-pasha.tatashin@soleen.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: B360120020 X-Stat-Signature: 6xu4whqsy9rc6zwcxu8hkwaa1i8b4ckx X-HE-Tag: 1707147086-131107 X-HE-Meta: U2FsdGVkX1/OmevbLIbLf8I+woGdFAjgMQXTJIQvcwQ+QdCDEXLZ5VgK0A9T20pQH/7w0rL+WttFkVvwQf7JPkAPxCq81722j5rTUlhFinJfVW7YwU1VqF/5OS59hrna7xBH3D3Ekv/n7wQ/cs82aTY8lho0zuVDntHW52BWfibNEWU60Wy/a6NTN5aitMWGxNePz1KcKK+fm9xEBBoskhVvV6nRdv5l6R7761ERiGu9wjpxyPr3RCj/K+ASeta81+iYmxoik9zmWd1nGvNyFwm7LXt+eQeJkuaZnTD3FXepLBwx66Cu26yu7gOj9S+LK0O8jYkz8wS0saOknEebmO3O/Eolx2wnpRt5ZAoWPeZKw6bOxhZY0E4z90dUpNQH7ZTG5jyQpzutXNzAqARyqphOsJb8HTqsE3dEdq+h+hr3ZVzRY8xLWJTWsI12mzNUd5bgvpLd36PlE/TcMsLyVpuLIH6fiALdyO88+16VOxRUXm5BPn8RRtrXn08tf+Ghzg750Qq+L/LTPY0XbdpGUjApMvr6XPbDJuJtiTmbtL2IweVIQViNG9lleEVvEA5DFN9C3xd/2s3Xn7PyCBoFkB93LXpLnjA4iYxM94QrZXS0mmyNS3IvxeIjdHakmF0LjsBmSZZmE82pjXlaXyr974dv0WosAV7bYwtpz/k2+XnbFyrLEzscQBqLanTAs/pRFwgRoLaQZXjoBzN6bVyNK6mQrHV9bF2VGHkLUNMaAm7bgJRAsEdMK4vOyNtLEoS1HeQk/TY+YjTDCKtRbysL79xlsEARUzz79rcX/KQyHe6RkA2sh+ZmYYMEY+fOud0uCl9OzroO02M6C+zw6LIVpPGNVHFsKO76EYCrDyEmtcOKRADdAn6Oi7Q7B2jlzj65tNKdhu/ZN7YIYK3TF3xLIJBG1/lGMRpUcnTbXTXJhH//NILuhv3wKWSqIz/0afHp5LHqkrxfThiqaHpKisK g1v7FMQI FFYNIs8xazWft7zDz3ZZARhOjn5TPzlqbPAn0H1wNlkU2Sa9BzoA8p/LFrUSDmjQlmiv44W5eBo7BpfVC6rxKjPt/3JjeFrB5WcI3t3Nd0ZOL9HRmRXKOHxNBxhwBWgslGgGPcCPsiiRkdqb5D4QD8JK8Gmx0m+QNUGNCOMhjjbq9EvnjsWon1vCFeW1cjknhbrSWOTxLiMZN00yN/FR6u/P49Eb36P8gMlRh/zREqjQ1h4Y25y3UELgLtEkLvDbd5iBE 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-02 7:28 pm, Pasha Tatashin wrote: > The magazine buffers can take gigabytes of kmem memory, dominating all > other allocations. For observability purpose create named slab cache so > the iova magazine memory overhead can be clearly observed. > > With this change: > >> slabtop -o | head > Active / Total Objects (% used) : 869731 / 952904 (91.3%) > Active / Total Slabs (% used) : 103411 / 103974 (99.5%) > Active / Total Caches (% used) : 135 / 211 (64.0%) > Active / Total Size (% used) : 395389.68K / 411430.20K (96.1%) > Minimum / Average / Maximum Object : 0.02K / 0.43K / 8.00K > > OBJS ACTIVE USE OBJ SIZE SLABS OBJ/SLAB CACHE SIZE NAME > 244412 244239 99% 1.00K 61103 4 244412K iommu_iova_magazine > 91636 88343 96% 0.03K 739 124 2956K kmalloc-32 > 75744 74844 98% 0.12K 2367 32 9468K kernfs_node_cache > > On this machine it is now clear that magazine use 242M of kmem memory. > > Signed-off-by: Pasha Tatashin > --- > drivers/iommu/iova.c | 70 ++++++++++++++++++++++++++------------------ > 1 file changed, 42 insertions(+), 28 deletions(-) > > Changelog: > v2: - Use iova_cache_get/iova_cache_put to allocate/free > "iova_magazine_cache" as suggested by Robin Murphy > - Minor fix in the commit log. > > diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c > index d30e453d0fb4..88255f9443b5 100644 > --- a/drivers/iommu/iova.c > +++ b/drivers/iommu/iova.c > @@ -237,6 +237,35 @@ static int __alloc_and_insert_iova_range(struct iova_domain *iovad, > return -ENOMEM; > } > > +/* > + * Magazine caches for IOVA ranges. For an introduction to magazines, > + * see the USENIX 2001 paper "Magazines and Vmem: Extending the Slab > + * Allocator to Many CPUs and Arbitrary Resources" by Bonwick and Adams. > + * For simplicity, we use a static magazine size and don't implement the > + * dynamic size tuning described in the paper. > + */ > + > +/* > + * As kmalloc's buffer size is fixed to power of 2, 127 is chosen to > + * assure size of 'iova_magazine' to be 1024 bytes, so that no memory > + * will be wasted. Since only full magazines are inserted into the depot, > + * we don't need to waste PFN capacity on a separate list head either. > + */ > +#define IOVA_MAG_SIZE 127 > + > +#define IOVA_DEPOT_DELAY msecs_to_jiffies(100) > + > +struct iova_magazine { > + union { > + unsigned long size; > + struct iova_magazine *next; > + }; > + unsigned long pfns[IOVA_MAG_SIZE]; > +}; > + > +static_assert(!(sizeof(struct iova_magazine) & (sizeof(struct iova_magazine) - 1))); Hmm, moving all this lot is a bit nasty, but I can see the current code layout forces you to need sizeof(struct iova_magazine) earlier... :( > + > +static struct kmem_cache *iova_magazine_cache; > static struct kmem_cache *iova_cache; > static unsigned int iova_cache_users; > static DEFINE_MUTEX(iova_cache_mutex); > @@ -275,6 +304,16 @@ int iova_cache_get(void) > pr_err("Couldn't create iova cache\n"); > return -ENOMEM; > } > + > + iova_magazine_cache = kmem_cache_create("iommu_iova_magazine", > + sizeof(struct iova_magazine), > + 0, SLAB_HWCACHE_ALIGN, NULL); > + if (!iova_magazine_cache) { > + cpuhp_remove_multi_state(CPUHP_IOMMU_IOVA_DEAD); > + mutex_unlock(&iova_cache_mutex); > + pr_err("Couldn't create iova magazine cache\n"); > + return -ENOMEM; And you're also strictly missing cleanup of iova_cache here. However again I think that's more the fault of the existing code for being clunky. Rather than drag out this review any more, though, I figured I'd just sort it all out - patches incoming :) Cheers, Robin. > + } > } > > iova_cache_users++; > @@ -295,6 +334,7 @@ void iova_cache_put(void) > if (!iova_cache_users) { > cpuhp_remove_multi_state(CPUHP_IOMMU_IOVA_DEAD); > kmem_cache_destroy(iova_cache); > + kmem_cache_destroy(iova_magazine_cache); > } > mutex_unlock(&iova_cache_mutex); > } > @@ -612,32 +652,6 @@ reserve_iova(struct iova_domain *iovad, > } > EXPORT_SYMBOL_GPL(reserve_iova); > > -/* > - * Magazine caches for IOVA ranges. For an introduction to magazines, > - * see the USENIX 2001 paper "Magazines and Vmem: Extending the Slab > - * Allocator to Many CPUs and Arbitrary Resources" by Bonwick and Adams. > - * For simplicity, we use a static magazine size and don't implement the > - * dynamic size tuning described in the paper. > - */ > - > -/* > - * As kmalloc's buffer size is fixed to power of 2, 127 is chosen to > - * assure size of 'iova_magazine' to be 1024 bytes, so that no memory > - * will be wasted. Since only full magazines are inserted into the depot, > - * we don't need to waste PFN capacity on a separate list head either. > - */ > -#define IOVA_MAG_SIZE 127 > - > -#define IOVA_DEPOT_DELAY msecs_to_jiffies(100) > - > -struct iova_magazine { > - union { > - unsigned long size; > - struct iova_magazine *next; > - }; > - unsigned long pfns[IOVA_MAG_SIZE]; > -}; > -static_assert(!(sizeof(struct iova_magazine) & (sizeof(struct iova_magazine) - 1))); > > struct iova_cpu_rcache { > spinlock_t lock; > @@ -658,7 +672,7 @@ static struct iova_magazine *iova_magazine_alloc(gfp_t flags) > { > struct iova_magazine *mag; > > - mag = kmalloc(sizeof(*mag), flags); > + mag = kmem_cache_alloc(iova_magazine_cache, flags); > if (mag) > mag->size = 0; > > @@ -667,7 +681,7 @@ static struct iova_magazine *iova_magazine_alloc(gfp_t flags) > > static void iova_magazine_free(struct iova_magazine *mag) > { > - kfree(mag); > + kmem_cache_free(iova_magazine_cache, mag); > } > > static void