From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 10DBA261B9E for ; Wed, 11 Mar 2026 04:52:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773204768; cv=none; b=fNPptTUcgla/brhq062kLndB5YORmA9a5RAmlG2zow7eIs69hYZK80PUePffaccCkHEb8xG+aqJ39yETO3RPW9oENKlsi+R2Uq8Zf/8ZdoEoDffK/eObw8wbU6dKZCb61QHTDIZ/eNEHld0Edk9s2BNO1PrLal9wj2YY6bgrFhM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773204768; c=relaxed/simple; bh=Hv8/bOWlaSdoaHerq87nhK/NV1iNBb+Em4ox56/66Uc=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=AToShtC5jxYc/W4FnmAvDS8nExBjmEGksPfgkC1IA6vtQ+RkQ/TjHaHzFHvLLAauDqfHC8agtK/HD27j7jiDI/v9acG+OXMTvoa22NFvdEzbOTKwgm79DAIKq+Qd5URPWYrWiOY/6rn79AY10qTNULGuspdTG4ytVKgpZ1KkEls= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com 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 D5C93165C; Tue, 10 Mar 2026 21:52:37 -0700 (PDT) Received: from [10.164.148.48] (unknown [10.164.148.48]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 09EB13F73B; Tue, 10 Mar 2026 21:52:33 -0700 (PDT) Message-ID: <2b9146bb-31e9-4bbd-843e-221f13a7097c@arm.com> Date: Wed, 11 Mar 2026 10:22:24 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 5/9] mm/rmap: batch unmap folios belonging to uffd-wp VMAs To: Barry Song <21cnbao@gmail.com>, "Lorenzo Stoakes (Oracle)" Cc: akpm@linux-foundation.org, axelrasmussen@google.com, yuanchu@google.com, david@kernel.org, hughd@google.com, chrisl@kernel.org, kasong@tencent.com, weixugc@google.com, Liam.Howlett@oracle.com, vbabka@kernel.org, rppt@kernel.org, surenb@google.com, mhocko@suse.com, riel@surriel.com, harry.yoo@oracle.com, jannh@google.com, pfalcato@suse.de, baolin.wang@linux.alibaba.com, shikemeng@huaweicloud.com, nphamcs@gmail.com, bhe@redhat.com, youngjun.park@lge.com, ziy@nvidia.com, kas@kernel.org, willy@infradead.org, yuzhao@google.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, ryan.roberts@arm.com, anshuman.khandual@arm.com References: <20260310073013.4069309-1-dev.jain@arm.com> <20260310073013.4069309-6-dev.jain@arm.com> <00f65edd-5b16-4f9e-a9fa-b923eba052b7@lucifer.local> Content-Language: en-US From: Dev Jain In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 11/03/26 9:44 am, Barry Song wrote: > On Wed, Mar 11, 2026 at 7:32 AM Barry Song <21cnbao@gmail.com> wrote: >> >> On Tue, Mar 10, 2026 at 4:34 PM Lorenzo Stoakes (Oracle) wrote: >>> >>> On Tue, Mar 10, 2026 at 01:00:09PM +0530, Dev Jain wrote: >>>> The ptes are all the same w.r.t belonging to the same type of VMA, and >>>> being marked with uffd-wp or all being not marked. Therefore we can batch >>>> set uffd-wp markers through install_uffd_wp_ptes_if_needed, and enable >>>> batched unmapping of folios belonging to uffd-wp VMAs by dropping that >>>> condition from folio_unmap_pte_batch. >>>> >>>> It may happen that we don't batch over the entire folio in one go, in which >>>> case, we must skip over the current batch. Add a helper to do that - >>>> page_vma_mapped_walk_jump() will increment the relevant fields of pvmw >>>> by nr pages. >>>> >>>> I think that we can get away with just incrementing pvmw->pte >>>> and pvmw->address, since looking at the code in page_vma_mapped.c, >>>> pvmw->pfn and pvmw->nr_pages are used in conjunction, and pvmw->pgoff >>>> and pvmw->nr_pages (in vma_address_end()) are used in conjunction, >>>> cancelling out the increment and decrement in the respective fields. But >>>> let us not rely on the pvmw implementation and keep this simple. >>> >>> This isn't simple... >>> >>>> >>>> Export this function to rmap.h to enable future reuse. >>>> >>>> Signed-off-by: Dev Jain >>>> --- >>>> include/linux/rmap.h | 10 ++++++++++ >>>> mm/rmap.c | 8 +++----- >>>> 2 files changed, 13 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/include/linux/rmap.h b/include/linux/rmap.h >>>> index 8dc0871e5f001..1b7720c66ac87 100644 >>>> --- a/include/linux/rmap.h >>>> +++ b/include/linux/rmap.h >>>> @@ -892,6 +892,16 @@ static inline void page_vma_mapped_walk_done(struct page_vma_mapped_walk *pvmw) >>>> spin_unlock(pvmw->ptl); >>>> } >>>> >>>> +static inline void page_vma_mapped_walk_jump(struct page_vma_mapped_walk *pvmw, >>>> + unsigned int nr) >>> >>> unsigned long nr_pages... 'nr' is meaningless and you're mixing + matching types >>> for no reason. >>> >>>> +{ >>>> + pvmw->pfn += nr; >>>> + pvmw->nr_pages -= nr; >>>> + pvmw->pgoff += nr; >>>> + pvmw->pte += nr; >>>> + pvmw->address += nr * PAGE_SIZE; >>>> +} >>> >>> I absolutely hate this. It's extremely confusing, especially since you're now >>> going from looking at 1 page to nr_pages - 1, jump doesn't really mean anything >>> here, you're losing sight of the batch size and exposing a silly detail to the >>> caller, and I really don't want to 'export' this at this time. >> >> I’m fairly sure I raised the same concern when Dev first suggested this, >> but somehow it seems my comment was completely overlooked. :-) I do recall, perhaps I was lazy to look at the pvmw code :) But I should have just looked at this earlier, it's fairly simple. See below. >> >>> >>> If we must have this, can you please make it static in rmap.c at least for the >>> time being. >>> >>> Or perhaps instead, have a batched variant of page_vma_mapped_walk(), like >>> page_vma_mapped_walk_batch()? >> >> Right now, for non-anon pages we face the same issues, but >> page_vma_mapped_walk() can skip those PTEs once it finds that >> nr - 1 PTEs are none. >> >> next_pte: >> do { >> pvmw->address += PAGE_SIZE; >> if (pvmw->address >= end) >> return not_found(pvmw); >> /* Did we cross page table boundary? */ >> if ((pvmw->address & (PMD_SIZE - PAGE_SIZE)) == 0) { >> if (pvmw->ptl) { >> spin_unlock(pvmw->ptl); >> pvmw->ptl = NULL; >> } >> pte_unmap(pvmw->pte); >> pvmw->pte = NULL; >> pvmw->flags |= PVMW_PGTABLE_CROSSED; >> goto restart; >> } >> pvmw->pte++; >> } while (pte_none(ptep_get(pvmw->pte))); >> >> The difference now is that swap entries cannot be skipped. >> >> If we're trying to find `page_vma_mapped_walk_batch()`, I suppose >> it could be like this? >> >> bool page_vma_mapped_walk_batch(struct page_vma_mapped_walk *pvmw, >> unsigned long nr) >> { >> ... >> } >> >> static inline bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) >> { >> return page_vma_mapped_walk_batch(pvmw, 1); >> } > > Another approach might be to introduce a flag so that > page_vma_mapped_walk() knows we are doing batched unmaps > and can skip nr - 1 swap entries. > > diff --git a/include/linux/rmap.h b/include/linux/rmap.h > index 8dc0871e5f00..bf03ae006366 100644 > --- a/include/linux/rmap.h > +++ b/include/linux/rmap.h > @@ -856,6 +856,9 @@ struct page *make_device_exclusive(struct > mm_struct *mm, unsigned long addr, > /* Look for migration entries rather than present PTEs */ > #define PVMW_MIGRATION (1 << 1) > > +/* Batched unmap: skip swap entries. */ > +#define PVMW_BATCH_UNMAP (1 << 2) Exactly, I just also came up with the same solution and saw your reply :) We can just name this PVMW_BATCH_PRESENT, the comment saying "Look for present entries", and fix the comment above PVMW_MIGRATION to drop the "rather than present PTEs" because that is wrong, we are currently also looking for softleaf entries by default. > + > /* Result flags */ > > /* The page is mapped across page table boundary */ > > > Thanks > Barry