From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 251172EA75E; Mon, 3 Aug 2026 14:30:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785767424; cv=none; b=ofWlu6LX6BMspcCEPXKG0AovDr+1yesiS4ZqiOjfLg1k5m6e2pn0zlQXXS6vmxcLWCJvT1cu6rBUKQ59a9VQZi/sLpJyv3XOTZq7H+cMu7hxU5fCtTPARk/s4C6q9SXP7J2ujUiSrTQc8quFhYz/reQ4GKtXWEyMCk6bwTZ6jcw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785767424; c=relaxed/simple; bh=ULVEe+C6M7F486Svj0YzXW90p+8vTLnXHm6Y+HcEDAA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=MJ3+KldBVv0zKk9c55dD1U8/NTSV/Y5wlJQfn8+388XVKEJXZY1ABANqQaOALeFA3PytRaZzvJuaaCHnGHhleJqk6fsVBmpe7nhpmJU8RJSNnOyyTu7KD//r31wi388x0366AlJpT9rbnX0oLGK/LCG099aZwT1knM3D3CfRVNM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=KunjWBf2; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="KunjWBf2" Received: by smtp.kernel.org (Postfix) with ESMTPSA id CF6D91F00A3D; Mon, 3 Aug 2026 14:30:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1785767422; bh=tBUO0sI60vSvUUULyUBjZ5PO/NpGB9Aqguv7Qzws6Zw=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=KunjWBf2HfEayrRWcgqlNF4dW8kZwEqvN9UMQVHbeVCf0bXTvc+9at69v/s86hVAy koxpSd5erKYHoQX6nU7pnxFVeAtOusP96Z+voCXciMEyecY4S+LM9crnVyh2EyWsJs ZGRosrBTJnaKMq/d21m372wXHZWkB7RqQOG8oYWhA5vCaKtJoEHCZZ8ByMRdEjQlkN pqiWrjr6BscxP4XUIMy8ofhjFcI8Ie4RbM+d8A5AO4eUk3TihD5OvMlO5qzUrV/nnQ yvjunAW8aAvCp/IczAYDcLvAXMtPV+pZelWzN4EzWjCR1M8tEGuY0Hyp0KY2Z9+Trw OoTsIb5WR/IgA== Date: Mon, 3 Aug 2026 15:30:00 +0100 From: "Lorenzo Stoakes (ARM)" To: "David Hildenbrand (Arm)" Cc: Andrew Morton , "Liam R. Howlett" , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , Jann Horn , Pedro Falcato , "Matthew Wilcox (Oracle)" , Jan Kara , Miaohe Lin , Naoya Horiguchi , Rik van Riel , Harry Yoo , Lance Yang , Kees Cook , Zi Yan , Baolin Wang , Nico Pache , Ryan Roberts , Dev Jain , Barry Song , Usama Arif , Matthew Brost , Joshua Hahn , Rakie Kim , Byungchul Park , Gregory Price , Ying Huang , Alistair Popple , Peter Xu , Xu Xin , Chengming Zhou , Arnd Bergmann , Greg Kroah-Hartman , linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH v3 08/15] mm: introduce and use linear_folio_page_index() Message-ID: References: <20260729-b4-scalable-cow-virt-pgoff-v3-0-e8ecfefea812@kernel.org> <20260729-b4-scalable-cow-virt-pgoff-v3-8-e8ecfefea812@kernel.org> <0d9f3040-825d-49af-9d07-1e7945dd3e9d@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0d9f3040-825d-49af-9d07-1e7945dd3e9d@kernel.org> On Mon, Aug 03, 2026 at 01:27:09PM +0200, David Hildenbrand (Arm) wrote: > On 7/29/26 18:48, Lorenzo Stoakes (ARM) wrote: > > This function is, for now, a placeholder; it will be used in future to > > determine whether to use the anonymous page index or not, based on whether > > the folio is anonymous or not. > > > > Currently it simply wraps linear_page_index(), so this does not change > > behaviour. > > > > We update callers that will, once the change is introduced to track > > anonymous folios by anonymous page offset if MAP_PRIVATE file-backed, need > > to determine which index to use based on folio type. > > > > No functional change intended. > > > > Signed-off-by: Lorenzo Stoakes (ARM) > > --- > > include/linux/pagemap.h | 18 ++++++++++++++++++ > > mm/huge_memory.c | 3 ++- > > mm/migrate.c | 6 ++++-- > > mm/userfaultfd.c | 6 ++++-- > > 4 files changed, 28 insertions(+), 5 deletions(-) > > > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h > > index 259177544b03..6eb8d811ba4c 100644 > > --- a/include/linux/pagemap.h > > +++ b/include/linux/pagemap.h > > @@ -1143,6 +1143,24 @@ static inline pgoff_t linear_anon_page_index(const struct vm_area_struct *vma, > > return pgoff; > > } > > > > +/** > > + * linear_folio_page_index() - Determine the absolute page offset of > > + * @address within @vma from @folio. > > + * @folio: The folio whose linear page index is sought. > > + * @vma: The VMA in which @address resides. > > + * @address: The address whose absolute page offset is required. > > + * > > + * For compatibility, currently identical to linear_page_index(). > > + * > > + * Returns: The absolute page offset of @address within @vma. > > + */ > > +static inline pgoff_t linear_folio_page_index(const struct folio *folio, > > + const struct vm_area_struct *vma, > > + const unsigned long address) > > +{ > > + return linear_page_index(vma, address); > > +} > > > I found this to be rather confusing, given that we now have a "folio" helper that > receives a folio and a "page" helper that doesn't receive a page ... This is just to avoid having to duplicate the if (folio_test_anon()) { ... } else { ... } stuff. Agreed it's a bit confusing! Really you are figuring things out from (vma, address) - 'what is the correct page offset based on the VMA'. And yeah it seems migrate can do it via PFN as you suggest, it really is just trying to find the page offset in the folio. But... > > I guess the problem is the "page" in "linear_page_index", as it > reminds of legacy page->index. > > > I wonder if it would be better to have a linear_folio_index() and > force that address points at the start of the folio. > > Looking below, this is exactly what we want for all except one case: ...I don't think this is true. The uffd code uses this too in move_present_ptes(): src_folio->index = linear_folio_page_index(src_folio, dst_vma, dst_addr); And it's now _updating_ the source folio index to the offset in the destination VMA, so it doesn't even relate to the source folio's offset at all? (move_swap_pte() calls linear_folio_page_index() but obviously has to be anon, so that can just use linear_anon_page_index() there instead, will update.) With your change we can just eliminate the linear_folio_page_index() function and open-code the uffd case. It's a bit of a special case anyway and is neatly the one place where you actually don't know if it's anon or file-backed (well anon or shmem specifically I think). > > > > /* pgoff is invalid for ksm pages, but they are never large */ > > - if (folio_test_large(folio) && !folio_test_hugetlb(folio)) > > - idx = linear_page_index(vma, pvmw.address) - pvmw.pgoff; > > + if (folio_test_large(folio) && !folio_test_hugetlb(folio)) { > > + idx += linear_folio_page_index(folio, vma, pvmw.address); > > + idx -= pvmw.pgoff; > > + } > > new = folio_page(folio, idx); > > I think we could avoid this index work entirely by using the pfn, which is much > clearer to me, and similar to how we handle it during other rmap operations. > > diff --git a/mm/migrate.c b/mm/migrate.c > index 222c8c15f782f..686351d353203 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -362,17 +362,12 @@ static bool remove_migration_pte(struct folio *folio, > struct page *new; > unsigned long idx = 0; > > - /* pgoff is invalid for ksm pages, but they are never large */ > - if (folio_test_large(folio) && !folio_test_hugetlb(folio)) > - idx = linear_page_index(vma, pvmw.address) - pvmw.pgoff; > - new = folio_page(folio, idx); > - > #ifdef CONFIG_ARCH_HAS_PMD_SOFTLEAVES > /* PMD-mapped THP migration entry */ > if (!pvmw.pte) { > VM_BUG_ON_FOLIO(folio_test_hugetlb(folio) || > !folio_test_pmd_mappable(folio), folio); > - remove_migration_pmd(&pvmw, new); > + remove_migration_pmd(&pvmw, folio_page(folio, idx)); I think we'd need the idx code below to go above where the idx code is now as otherwise this will be incorrect right? But then again, if it's a PMD softleaf it'd have to be aligned right, so couldn't we just update that function to be passed a folio instead and avoid the idx here at all? > continue; > } > #endif > @@ -385,10 +380,14 @@ static bool remove_migration_pte(struct folio *folio, > try_to_map_unused_to_zeropage(&pvmw, folio, old_pte, idx)) > continue; > > + entry = softleaf_from_pte(old_pte); > + if (folio_test_large(folio) && !folio_test_hugetlb(folio)) > + idx = softleaf_to_pfn(entry) - folio_pfn(rmap_walk_arg->folio); Could actually be softleaf_to_pfn(entry) - pvmw.pfn even? > + new = folio_page(folio, idx); > + > folio_get(folio); > pte = mk_pte(new, READ_ONCE(vma->vm_page_prot)); > > - entry = softleaf_from_pte(old_pte); > if (!softleaf_is_migration_young(entry)) > pte = pte_mkold(pte); > if (folio_test_dirty(folio) && softleaf_is_migration_dirty(entr > > > remove_migration_pmd() will work for now. Later it should just receive the folio > and do the same thing through softleaf_from_pmd() -> softleaf_to_pfn(). Ah you already addressed it. But since this would move the idx code above, I think I should just change it to accept a folio instead? Or for less churn &folio->page... > > -- > Cheers, > > David -- Cheers, Lorenzo