mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dev Jain <dev.jain@arm.com>
To: Barry Song <baohua@kernel.org>
Cc: akpm@linux-foundation.org, david@kernel.org, ljs@kernel.org,
	hughd@google.com, chrisl@kernel.org, kasong@tencent.com,
	davem@davemloft.net, andreas@gaisler.com, riel@surriel.com,
	liam@infradead.org, vbabka@kernel.org, harry@kernel.org,
	jannh@google.com, lance.yang@linux.dev,
	baolin.wang@linux.alibaba.com, shikemeng@huaweicloud.com,
	nphamcs@gmail.com, baoquan.he@linux.dev, youngjun.park@lge.com,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	rppt@kernel.org, surenb@google.com, mhocko@suse.com,
	pfalcato@suse.de, jgg@ziepe.ca, thuth@redhat.com,
	sparclinux@vger.kernel.org, ryan.roberts@arm.com,
	anshuman.khandual@arm.com
Subject: Re: [PATCH v3 8/9] mm/rmap: batch unmap anonymous swap-backed large folios
Date: Sat, 26 Sep 2026 18:07:01 +0530	[thread overview]
Message-ID: <7a2445ed-0e56-4a87-b4ba-6cf1dabd07d2@arm.com> (raw)
In-Reply-To: <f61fa70f-882a-4787-918e-4b6959062c9c@arm.com>



On 25/09/26 4:55 pm, Dev Jain wrote:
> 
> 
> On 25/09/26 11:45 am, Barry Song wrote:
>> On Thu, Sep 24, 2026 at 9:12 PM Dev Jain <dev.jain@arm.com> wrote:
>>>
>>> Enable batch clearing of PTEs and batch swap setting of PTEs for anon
>>> swap-backed folio unmapping.
>>>
>>> Processing all PTEs of a large folio in one go helps us batch across
>>> atomics (add_mm_counter() etc), barriers in __folio_try_share_anon_rmap(),
>>> and repeated calls to page_vma_mapped_walk(). In general, batching helps
>>> execute similar code together, making the path more memory and CPU
>>> friendly.
>>>
>>> On arm64-contpte, batching also helps avoid redundant ptep_get() calls
>>> and TLB flushes while breaking the contpte mapping.
>>>
>>> The handling of anon-exclusivity is very similar to commit cac1db8c3aad
>>> ("mm: optimize mprotect() by PTE batching"). Since
>>> folio_unmap_pte_batch() does not look at the bits of the underlying page,
>>> process sub-batches of PTEs pointing to pages with the same exclusivity
>>> state, and batch set only those PTEs to swap PTEs in one go.
>>>
>>> Disable batching for sparc (because of arch_unmap_one), we will batch this
>>> later.
>>>
>>> Rmap accounting and reference accounting must happen when anon folio unmap
>>> succeeds. If a large folio is only partially batched or a later sub-batch
>>> fails, account only the pages that were actually unmapped. Put that
>>> accounting in __ttu_anon_swapbacked_folio() itself instead of using goto
>>> jumps at the try_to_unmap_one() callsite.
>>>
>>> Similarly, do the finish_folio_unmap_batch() in ttu_anon_folio() itself for the
>>> non-swapbacked lazyfree case.
>>>
>>> If the batch length is less than the number of pages in the folio, skip
>>> over this batch. page_vma_mapped_walk() handles this: check_pte() returns
>>> true only if any of [pvmw->pfn, pvmw->pfn + nr_pages) is mapped by the
>>> PTE. Swap PTEs have no underlying PFN, so check_pte() returns false until
>>> the walk reaches the next present PTE to unmap.
>>>
>>> Remove the finish_unmap label since no goto callers are left now.
>>>
>>> Signed-off-by: Dev Jain <dev.jain@arm.com>
>>> ---
>>>  mm/rmap.c | 109 +++++++++++++++++++++++++++++++++++++++---------------
>>>  1 file changed, 80 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/mm/rmap.c b/mm/rmap.c
>>> index fa9fc8374fc26..cdda9efd80bc3 100644
>>> --- a/mm/rmap.c
>>> +++ b/mm/rmap.c
>>> @@ -1966,12 +1966,13 @@ static inline unsigned int folio_unmap_pte_batch(struct folio *folio,
>>>         end_addr = pmd_addr_end(addr, vma->vm_end);
>>>         max_nr = (end_addr - addr) >> PAGE_SHIFT;
>>>
>>> -       /* We only support lazyfree or file folios batching for now ... */
>>> -       if (folio_test_anon(folio) && folio_test_swapbacked(folio))
>>> +       if (pte_unused(pte))
>>>                 return 1;
>>>
>>> -       if (pte_unused(pte))
>>> +#ifdef __HAVE_ARCH_UNMAP_ONE
>>> +       if (folio_test_anon(folio) && folio_test_swapbacked(folio))
>>>                 return 1;
>>> +#endif
>>>
>>>         /*
>>>          * If unmap fails, we need to restore the ptes. To avoid accidentally
>>> @@ -2141,16 +2142,25 @@ static pte_t swp_pte_prepare(swp_entry_t entry, pte_t old_pte,
>>>         return swp_pte;
>>>  }
>>>
>>> -static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
>>> +static void finish_folio_unmap_batch(struct vm_area_struct *vma,
>>> +               struct folio *folio, struct page *page, unsigned long nr_pages)
>>> +{
>>> +       folio_remove_rmap_ptes(folio, page, nr_pages, vma);
>>> +       if (vma->vm_flags & VM_LOCKED)
>>> +               mlock_drain_local();
>>> +       folio_put_refs(folio, nr_pages);
>>> +}
>>> +
>>> +static bool __ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
>>>                 struct folio *folio, struct page *page, unsigned long address,
>>> -               pte_t *ptep, pte_t pteval)
>>> +               pte_t *ptep, pte_t pteval, unsigned long nr_pages,
>>> +               bool anon_exclusive)
>>>  {
>>> -       const bool anon_exclusive = folio_test_anon(folio) &&
>>> -                                   PageAnonExclusive(page);
>>>         swp_entry_t entry = folio_page_swap_entry(folio, page);
>>>         struct mm_struct *mm = vma->vm_mm;
>>> +       pte_t swp_pte;
>>>
>>> -       if (folio_dup_swap_pages(folio, page, 1) < 0)
>>> +       if (folio_dup_swap_pages(folio, page, nr_pages) < 0)
>>>                 return false;
>>>
>>>         /*
>>> @@ -2159,21 +2169,57 @@ static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
>>>          * so we'll not check/care.
>>>          */
>>>         if (arch_unmap_one(mm, vma, address, pteval) < 0) {
>>> -               folio_put_swap_pages(folio, page, 1);
>>> +               VM_WARN_ON(nr_pages != 1);
>>> +               folio_put_swap_pages(folio, page, nr_pages);
>>>                 return false;
>>>         }
>>>
>>>         /* See folio_try_share_anon_rmap(): clear PTE first. */
>>> -       if (anon_exclusive && folio_try_share_anon_rmap_pte(folio, page)) {
>>> -               folio_put_swap_pages(folio, page, 1);
>>> +       if (anon_exclusive &&
>>> +           folio_try_share_anon_rmap_ptes(folio, page, nr_pages)) {
>>> +               folio_put_swap_pages(folio, page, nr_pages);
>>>                 return false;
>>>         }
>>>
>>>         mm_prepare_for_swap_entries(mm);
>>> -       dec_mm_counter(mm, MM_ANONPAGES);
>>> -       inc_mm_counter(mm, MM_SWAPENTS);
>>> -       set_pte_at(mm, address, ptep,
>>> -                  swp_pte_prepare(entry, pteval, anon_exclusive));
>>> +       add_mm_counter(mm, MM_ANONPAGES, -nr_pages);
>>> +       add_mm_counter(mm, MM_SWAPENTS, nr_pages);
>>> +       swp_pte = swp_pte_prepare(entry, pteval, anon_exclusive);
>>> +       set_softleaf_ptes(mm, address, ptep, swp_pte, nr_pages);
>>> +       finish_folio_unmap_batch(vma, folio, page, nr_pages);
>>> +       return true;
>>> +}
>>> +
>>> +static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
>>> +               struct folio *folio, struct page *first_page,
>>> +               unsigned long address, pte_t *ptep, pte_t pteval,
>>> +               unsigned long nr_pages)
>>> +{
>>> +       unsigned long batch_idx = 0;
>>> +
>>> +       while (nr_pages) {
>>> +               bool anon_exclusive = PageAnonExclusive(first_page + batch_idx);
>>> +               unsigned long len = page_anon_exclusive_batch(batch_idx,
>>> +                               nr_pages, first_page, anon_exclusive);
>>> +
>>> +               if (!__ttu_anon_swapbacked_folio(vma, folio,
>>> +                               first_page + batch_idx, address, ptep, pteval,
>>> +                               len, anon_exclusive)) {
>>> +                       /* Restore the remaining PTEs that were cleared. */
>>> +                       set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
>>> +                       return false;
>>> +               }
>>> +
>>> +               nr_pages -= len;
>>> +               if (!nr_pages)
>>> +                       break;
>>> +
>>> +               pteval = pte_advance_pfn(pteval, len);
>>> +               address += len * PAGE_SIZE;
>>> +               batch_idx += len;
>>> +               ptep += len;
>>> +       }
>>> +
>>>         return true;
>>>  }
>>>
>>> @@ -2186,15 +2232,22 @@ static bool ttu_anon_folio(struct vm_area_struct *vma, struct folio *folio,
>>>          * See handle_pte_fault() ...
>>>          */
>>>         if (WARN_ON_ONCE(folio_test_swapbacked(folio) !=
>>> -                        folio_test_swapcache(folio)))
>>> +                        folio_test_swapcache(folio))) {
>>> +               set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
>>>                 return false;
>>> +       }
>>>
>>> -       if (!folio_test_swapbacked(folio))
>>> -               return ttu_anon_lazyfree_folio(vma, folio, nr_pages);
>>> +       if (!folio_test_swapbacked(folio)) {
>>> +               if (!ttu_anon_lazyfree_folio(vma, folio, nr_pages)) {
>>> +                       set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
>>> +                       return false;
>>> +               }
>>> +               finish_folio_unmap_batch(vma, folio, page, nr_pages);
>>> +               return true;
>>> +       }
>>
>> Hi Dev,
>>
>> if we look at the whole `ttu_anon_folio()` code:
>>
>> static bool ttu_anon_folio(struct vm_area_struct *vma, struct folio *folio,
>>                 struct page *page, unsigned long address, pte_t *ptep,
>>                 pte_t pteval, unsigned long nr_pages)
>> {
>>         /*
>>          * Store the swap location in the pte.
>>          * See handle_pte_fault() ...
>>          */
>>         if (WARN_ON_ONCE(folio_test_swapbacked(folio) !=
>>                          folio_test_swapcache(folio))) {
>>                 set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
>>                 return false;
>>         }
>>
>>         if (!folio_test_swapbacked(folio)) {
>>                 if (!ttu_anon_lazyfree_folio(vma, folio, nr_pages)) {
>>                         set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
>>                         return false;
>>                 }
>>                 finish_folio_unmap_batch(vma, folio, page, nr_pages);
>>                 return true;
>>         }
>>
>>         return ttu_anon_swapbacked_folio(vma, folio, page, address, ptep,
>>                                          pteval, nr_pages);
>> }
>>
>> The comment "Store the swap location in the pte" is really out of
>> place. Since we are already here, can we move it to the right place?
> 
> I am not sure what is the "right place" you are referring to : ) More
> than that, this comments feels very out of date - it is well-know that
> the "swap location" is stored in the pte.
> 
> In fact, this comment starts on top of the wrong block, which is a
> debug assertion for "when we are here, swapbacked if and only if swapcache".
> So I'd rather just remove it.
> 
>>
>>>
>>> -       /* nr_pages > 1 not supported yet */
>>>         return ttu_anon_swapbacked_folio(vma, folio, page, address, ptep,
>>> -                                        pteval);
>>> +                                        pteval, nr_pages);
>>>  }
>>>
>>>  /*
>>> @@ -2374,13 +2427,14 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>>>                          */
>>>                         dec_mm_counter(mm, mm_counter(folio));
>>>                 } else if (folio_test_anon(folio)) {
>>> +                       /* finish_folio_unmap_batch handled internally */
>>
>>
>> let me paste the final code here:
>>
>>                 } else if (folio_test_anon(folio)) {
>>                         /* finish_folio_unmap_batch handled internally */
>>                         if (!ttu_anon_folio(vma, folio, page, address,
>>                                             pvmw.pte, pteval, nr_pages))
>>                                 goto walk_abort;
>>
>>                         if (nr_pages == folio_nr_pages(folio))
>>                                 goto walk_done;
>>                         continue;
>>                 } else {
>>                         ...
>>                 }
>>
>>                 finish_folio_unmap_batch(vma, folio, page, nr_pages);
>>
>>                 /*
>>                  * If we are sure that we batched the entire folio and cleared
>>                  * all PTEs, we can just optimize and stop right here.
>>                  */
>>                 if (nr_pages == folio_nr_pages(folio))
>>                         goto walk_done;
>>                 continue;
>>
>> I am really struggling to read the code here. For anon folios, we
>> handle `finish_folio_unmap_batch()` internally, while for other folios
>> we still do it outside. This feels a bit inconsistent.
>>
>> Also, we are duplicating the code below twice:
>>
>>                 if (nr_pages == folio_nr_pages(folio))
>>                         goto walk_done;
>>
>> Can't we just do `finish_folio_unmap_batch()` like the other cases?
> 
> The core problem is that we may succeed on a partial range.
> 
> ttu_anon_swapbacked_folio can succeed on x < nr_pages. So for x, we
> should call finish_folio_unmap_batch. Either we do it my way, or,
> ttu_anon_folio returns the number of pages successfully unmapped = x,
> and then even in case of "failure" (we should now jump to
> walk_abort since we failed to unmap 'nr_pages - x' pages) we call
> finish_folio_unmap_batch for x pages.
> 
> IMHO that is uglier. I wanted to avoid this label jumping. I would
> be interested in hearing a better alternate : )

I did try this and it looks nicer actually. Will shift to this in v4,
thanks!


>>
>> That would also avoid the duplication.
>>
>> Best Regards
>> Barry
> 


  reply	other threads:[~2026-09-26 12:37 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-24 13:09 [PATCH v3 0/9] Optimize anonymous swapbacked large folio unmapping Dev Jain
2026-09-24 13:09 ` [PATCH v3 1/9] mm/swapfile: add batched version of folio_dup_swap Dev Jain
2026-09-24 13:09 ` [PATCH v3 2/9] mm/swapfile: add batched version of folio_put_swap Dev Jain
2026-09-24 13:09 ` [PATCH v3 3/9] mm: move anon-exclusive batch helper to rmap.h Dev Jain
2026-09-24 21:02   ` Barry Song
2026-09-25 10:18     ` Dev Jain
2026-09-24 13:09 ` [PATCH v3 4/9] mm/rmap: Add batched version of folio_try_share_anon_rmap_pte Dev Jain
2026-09-25  5:30   ` Barry Song
2026-09-25 11:09     ` Dev Jain
2026-09-25 12:03       ` Barry Song
2026-09-24 13:09 ` [PATCH v3 5/9] mm/internal: rename swap offset helpers to softleaf offset Dev Jain
2026-09-24 13:09 ` [PATCH v3 6/9] mm/internal: add set_softleaf_ptes Dev Jain
2026-09-24 13:09 ` [PATCH v3 7/9] mm/memory: use set_softleaf_ptes for uffd-wp markers Dev Jain
2026-09-24 13:09 ` [PATCH v3 8/9] mm/rmap: batch unmap anonymous swap-backed large folios Dev Jain
2026-09-25  6:15   ` Barry Song
2026-09-25 11:25     ` Dev Jain
2026-09-26 12:37       ` Dev Jain [this message]
2026-09-24 13:09 ` [PATCH v3 9/9] mm, sparc: batch arch_unmap_one() Dev Jain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7a2445ed-0e56-4a87-b4ba-6cf1dabd07d2@arm.com \
    --to=dev.jain@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreas@gaisler.com \
    --cc=anshuman.khandual@arm.com \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=baoquan.he@linux.dev \
    --cc=chrisl@kernel.org \
    --cc=davem@davemloft.net \
    --cc=david@kernel.org \
    --cc=harry@kernel.org \
    --cc=hughd@google.com \
    --cc=jannh@google.com \
    --cc=jgg@ziepe.ca \
    --cc=kasong@tencent.com \
    --cc=lance.yang@linux.dev \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=nphamcs@gmail.com \
    --cc=pfalcato@suse.de \
    --cc=riel@surriel.com \
    --cc=rppt@kernel.org \
    --cc=ryan.roberts@arm.com \
    --cc=shikemeng@huaweicloud.com \
    --cc=sparclinux@vger.kernel.org \
    --cc=surenb@google.com \
    --cc=thuth@redhat.com \
    --cc=vbabka@kernel.org \
    --cc=youngjun.park@lge.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®