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 6A3EAC83F01 for ; Wed, 30 Aug 2023 15:32:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C8BA5440152; Wed, 30 Aug 2023 11:32:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C14CE440009; Wed, 30 Aug 2023 11:32:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AB453440152; Wed, 30 Aug 2023 11:32:31 -0400 (EDT) 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 9932F440009 for ; Wed, 30 Aug 2023 11:32:31 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 55AD4A02AF for ; Wed, 30 Aug 2023 15:32:31 +0000 (UTC) X-FDA: 81181162902.12.E6E4823 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf06.hostedemail.com (Postfix) with ESMTP id 500AE18001D for ; Wed, 30 Aug 2023 15:32:29 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf06.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1693409549; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jdCjwmeq4PGFYJ7IUHYzWdqw0cwH9VmnPZPplawnJKk=; b=3/Qct9zC1HrAoDa6D/5WWX6WXLdBAAQJilsl5Gg1rJ1GXM6A/Gjnh5Ve7QMznizBaeu4IC /HG7XDhNNZZazMtFkDsXRq4mmt2oN7ZeH+Gp3bawLwh/huo8jLTUGjoTJOnwl5xJd1Hjml XWOeMiOx4N2ZxR/95PuREGqm/qHGUlI= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf06.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1693409549; a=rsa-sha256; cv=none; b=S8QpteBBlvQAoG5czb2th48dShwqgcE8KSrDRLIwbUHVz/Rb9+7UZrIbz9sD2Jv0AZixmp P22EVAsiQEFzEs2LwgLsu+qQIqcb8oAcMFrWMydIMA+nPNZFmy8xfBeTQb0tjlfj37BI8e G82JeKqnztTY5Agg7CHbyF50Bv16+pw= 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 9280E2F4; Wed, 30 Aug 2023 08:33:07 -0700 (PDT) Received: from [10.57.64.173] (unknown [10.57.64.173]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 896133F64C; Wed, 30 Aug 2023 08:32:25 -0700 (PDT) Message-ID: Date: Wed, 30 Aug 2023 16:32:24 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 5/5] mm/mmu_gather: Store and process pages in contig ranges Content-Language: en-GB To: Matthew Wilcox Cc: Will Deacon , "Aneesh Kumar K.V" , Andrew Morton , Nick Piggin , Peter Zijlstra , Christian Borntraeger , Sven Schnelle , Arnd Bergmann , David Hildenbrand , Yu Zhao , "Kirill A. Shutemov" , Yin Fengwei , Yang Shi , "Huang, Ying" , Zi Yan , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20230830095011.1228673-1-ryan.roberts@arm.com> <20230830095011.1228673-6-ryan.roberts@arm.com> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 500AE18001D X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: enh6qm6bprgfrc81hsp3oabiwwuqo873 X-HE-Tag: 1693409549-345017 X-HE-Meta: U2FsdGVkX1/wZymZhqL4Tf1aU9PDuWC9+R3AH3yzUursxSrqOna3sEMOxdkOUFgJGgPbdk+WjBbnuBfwMkdwF6RpTQImiOQH2jWhMK29nx1fI1bjafq/L3dzk99GgOoexkRUGABy8u+DQ0paRSe2UbM+qQ+lIxO6wj+oTFQ1spFTepN5mZS8Jl883exMnek2Aq2w0ssjJQEsB3QbdXrjyeG50mpgjvlyYqjvYNr4lGjcC4nPvykuFdaxBtpsjGIo/+Zv8F48IMGK4tUeT32qyWFKpDQ2tY2dMsbxoDx9iQI/BUHyyTUbIzTAadBvllgtOvn5IJXWAen1YrGzQHQFSViCcXZ9YAK6RaJ3dWfGgWkTFE2tTiMYHeMil6eQ1f7n32YBCzj3eR8rMYUOkeA5JwpTJcCZ1yO2xBTj0Gh8cjKQAbkHuFIEfx88JyOfY7bVCWVWFtJKPsRPNKWfbIE4cf0AVnWjOheN/6s5tlpuCtCth9IRmVWrSiE8UKmNeHACfZZaGay+fC67UfTwaCBXC/WIxarh4+JSEuTP2PrDFXBjyYVzWQUhEPUZGK5+GZ1ndBXCtG5yQtbnnyg87+PLlf5Not02LfxMPMlWvOGele2KGuV4jtFcuI6KHo/gxCX3lMHJS5zCk+44+j2/EkoJhybCqxvwhg9eKiWOWGit1kZoHoXA8P7fMKtGsPezDO2YPJA5U0ZbaaErqB1W0J702J/SpodbV7ZjbvYJj42gPdgpvU2xuWF/UNc0HXjofm2aVTflDCLJYpgB44W9ExP3fqq8nC/S4zcjYq57Gm9hOqRi3rGPFoKHTc7Cqio2d6rbWsMdZ6j+D7DzPtONvTjVYZusUu2ZUufS7LEfJ3Is/oEBXJXkvAHw98R93crHU3UiPGLplxT9CZFm3UONYD6SPm8YReYcxW0oOFWYOWDFpmTPwiOLVBlaAoZT5/s5q85D2D5sSb2zU7e0yLNZqFL cpC8C2+K kPjKob3GbyOfbhO26qV5jbBKgiQHdqvWnTjOG3TQlyT0MZIfhqIU8DVNcLomsfTBDzyLTJvSWHmFXiqvpeYdH9zwFtv3iyfqDQsT80n7ym/S2bPhqAas1fS5yGC46b0aqGtPLa/3ZrD4aWrKdyLoCcKiSk31Wq6vz7fKBsiFXHBDVlh4NYeCvdaK3iY6LNkNNB9IT 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 30/08/2023 16:07, Matthew Wilcox wrote: > On Wed, Aug 30, 2023 at 10:50:11AM +0100, Ryan Roberts wrote: >> +++ b/include/asm-generic/tlb.h >> @@ -246,11 +246,11 @@ struct mmu_gather_batch { >> struct mmu_gather_batch *next; >> unsigned int nr; >> unsigned int max; >> - struct page *pages[]; >> + struct pfn_range folios[]; > > I think it's dangerous to call this 'folios' as it lets you think that > each entry is a single folio. But as I understand this patch, you can > coagulate contiguous ranges across multiple folios. No that's not quite the case; each contiguous range only ever spans a *single* folio. If there are 2 contiguous folios, they will be represented as separate ranges. This is done so that we can subsequently do the per-folio operations without having to figure out how many folios are within each range - one range = one (contiguous part of a) folio. On naming, I was calling this variable "ranges" in v1 but thought folios was actually clearer. How about "folio_regions"? > >> -void free_pages_and_swap_cache(struct page **pages, int nr) >> +void free_folios_and_swap_cache(struct pfn_range *folios, int nr) >> { >> lru_add_drain(); >> for (int i = 0; i < nr; i++) >> - free_swap_cache(pages[i]); >> - release_pages(pages, nr); >> + free_swap_cache(pfn_to_page(folios[i].start)); > > ... but here, you only put the swapcache for the first folio covered by > the range, not for each folio. Yes that's intentional - one range only ever covers one folio, so I only need to call free_swap_cache() once for the folio. Unless I've misunderstood and free_swap_cache() is actually decrementing a reference count and needs to be called for every page? (but it doesn't look like that in the code). > >> + folios_put_refs(folios, nr); > > It's kind of confusing to have folios_put() which takes a struct folio * > and then folios_put_refs() which takes a struct pfn_range *. > pfn_range_put()? I think it's less confusing if you know that each pfn_range represents a single contig range of pages within a *single* folio. pfn_range_put() would make it sound like its ok to pass a pfn_range that spans multiple folios (this would break). I could rename `struct pfn_range` to `struct sub_folio` or something like that. Would that help make the semantic clearer?