Re: [PATCH v3 4/9] mm/rmap: Add batched version of folio_try_share_anon_rmap_pte

[Barry Song <baohua@kernel.org>]

On Thu, Sep 24, 2026 at 9:11 PM Dev Jain <dev.jain@arm.com> wrote:
>
> To enable batched unmapping of anonymous folios, we need to handle the
> sharing of exclusive pages. Hence, a batched version of
> folio_try_share_anon_rmap_pte is required.
>
> Currently, the sole purpose of nr_pages in __folio_try_share_anon_rmap is
> to do some rmap sanity checks. Now, clear the PageAnonExclusive bit on a
> batch of nr_pages. Refactor the function such that the clearing of the bit
> can be done at one place without duplication.
>
> Note that __folio_try_share_anon_rmap can receive nr_pages == HPAGE_PMD_NR
> from the PMD path, but currently we only clear the bit on the head page.
> Retain this behaviour by setting nr_pages = 1 in case the caller is
> folio_try_share_anon_rmap_pmd.
>
> While at it, convert nr_pages to unsigned long to future-proof from
> overflow in case P4D-huge mappings etc get supported down the road.
> I haven't made such a change in each function receiving nr_pages in
> try_to_unmap_one - perhaps this can be done incrementally.
>
> Add two WARN's: check that the batch is entirely exclusive (for PMD
> callers, need to check only head page), and that there are only
> PTE/PMD paths converging into __folio_try_share_anon_rmap.
>
> Signed-off-by: Dev Jain <dev.jain@arm.com>

The patch looks correct to me, but it seems a bit hard to follow
because of the `pinnable` handling.

> ---
>  include/linux/rmap.h | 56 ++++++++++++++++++++++++++++++--------------
>  1 file changed, 39 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/rmap.h b/include/linux/rmap.h
> index 62ef511a6175a..91b5763ef466f 100644
> --- a/include/linux/rmap.h
> +++ b/include/linux/rmap.h
> @@ -723,17 +723,23 @@ static inline int folio_try_dup_anon_rmap_pmd(struct folio *folio,
>  }
>
>  static __always_inline int __folio_try_share_anon_rmap(struct folio *folio,
> -               struct page *page, int nr_pages, enum pgtable_level level)
> +               struct page *page, unsigned long nr_pages, enum pgtable_level level)
>  {
> +       /* device private folios cannot get pinned via GUP. */
> +       const bool pinnable = !folio_is_device_private(folio);
> +

If we make a helper that does:

for (;;) {
     ClearPageAnonExclusive(page);
     if (--nr_pages == 0)
               break;
     page++;
}

wouldn't that allow us to avoid this ugly `pinnable` variant?

Then we can make a much smaller modification, which should
also be much easier to follow? No?

Best Regards
Barry