Received-SPF: pass (google.com: domain of linux-kernel+bounces-168420-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240503005023.174597-1-21cnbao@gmail.com> <20240503005023.174597-4-21cnbao@gmail.com>
 <7548e30c-d56a-4a57-ab87-86c9c8e523b1@arm.com>
In-Reply-To: <7548e30c-d56a-4a57-ab87-86c9c8e523b1@arm.com>
From: Barry Song <21cnbao@gmail.com>
Date: Sat, 4 May 2024 07:40:16 +0800
Message-ID: <CAGsJ_4wx60GoB1erTQ7v3GTXLb_140bOJ_+z=kqY39eOd3P23g@mail.gmail.com>
Subject: Re: [PATCH v3 3/6] mm: introduce pte_move_swp_offset() helper which
 can move offset bidirectionally
To: Ryan Roberts <ryan.roberts@arm.com>
Cc: akpm@linux-foundation.org, linux-mm@kvack.org, 
	baolin.wang@linux.alibaba.com, chrisl@kernel.org, david@redhat.com, 
	hanchuanhua@oppo.com, hannes@cmpxchg.org, hughd@google.com, 
	kasong@tencent.com, linux-kernel@vger.kernel.org, surenb@google.com, 
	v-songbaohua@oppo.com, willy@infradead.org, xiang@kernel.org, 
	ying.huang@intel.com, yosryahmed@google.com, yuzhao@google.com, 
	ziy@nvidia.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, May 3, 2024 at 5:41=E2=80=AFPM Ryan Roberts <ryan.roberts@arm.com> =
wrote:
>
> On 03/05/2024 01:50, Barry Song wrote:
> > From: Barry Song <v-songbaohua@oppo.com>
> >
> > There could arise a necessity to obtain the first pte_t from a swap
> > pte_t located in the middle. For instance, this may occur within the
> > context of do_swap_page(), where a page fault can potentially occur in
> > any PTE of a large folio. To address this, the following patch introduc=
es
> > pte_move_swp_offset(), a function capable of bidirectional movement by
> > a specified delta argument. Consequently, pte_increment_swp_offset()
>
> You mean pte_next_swp_offset()?

yes.

>
> > will directly invoke it with delta =3D 1.
> >
> > Suggested-by: "Huang, Ying" <ying.huang@intel.com>
> > Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> > ---
> >  mm/internal.h | 25 +++++++++++++++++++++----
> >  1 file changed, 21 insertions(+), 4 deletions(-)
> >
> > diff --git a/mm/internal.h b/mm/internal.h
> > index c5552d35d995..cfe4aed66a5c 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -211,18 +211,21 @@ static inline int folio_pte_batch(struct folio *f=
olio, unsigned long addr,
> >  }
> >
> >  /**
> > - * pte_next_swp_offset - Increment the swap entry offset field of a sw=
ap pte.
> > + * pte_move_swp_offset - Move the swap entry offset field of a swap pt=
e
> > + *    forward or backward by delta
> >   * @pte: The initial pte state; is_swap_pte(pte) must be true and
> >   *    non_swap_entry() must be false.
> > + * @delta: The direction and the offset we are moving; forward if delt=
a
> > + *    is positive; backward if delta is negative
> >   *
> > - * Increments the swap offset, while maintaining all other fields, inc=
luding
> > + * Moves the swap offset, while maintaining all other fields, includin=
g
> >   * swap type, and any swp pte bits. The resulting pte is returned.
> >   */
> > -static inline pte_t pte_next_swp_offset(pte_t pte)
> > +static inline pte_t pte_move_swp_offset(pte_t pte, long delta)
>
> We have equivalent functions for pfn:
>
>   pte_next_pfn()
>   pte_advance_pfn()
>
> Although the latter takes an unsigned long and only moves forward current=
ly. I
> wonder if it makes sense to have their naming and semantics match? i.e. c=
hange
> pte_advance_pfn() to pte_move_pfn() and let it move backwards too.
>
> I guess we don't have a need for that and it adds more churn.

we might have a need in the below case.
A forks B, then A and B share large folios. B unmap/exit, then large
folios of process
A become single-mapped.
Right now, while writing A's folios, we are CoWing A's large folios
into many small
folios. I believe we can reuse the entire large folios instead of doing nr_=
pages
CoW and page faults.
In this case, we might want to get the first PTE from vmf->pte.

Another case, might be
A forks B, and we write either A or B, we might CoW an entire large
folios instead
CoWing nr_pages small folios.

case 1 seems more useful, I might have a go after some days. then we might
see pte_move_pfn().

>
> Anyway:
>
> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>

thanks!

>
>
> >  {
> >       swp_entry_t entry =3D pte_to_swp_entry(pte);
> >       pte_t new =3D __swp_entry_to_pte(__swp_entry(swp_type(entry),
> > -                                                (swp_offset(entry) + 1=
)));
> > +                                                (swp_offset(entry) + d=
elta)));
> >
> >       if (pte_swp_soft_dirty(pte))
> >               new =3D pte_swp_mksoft_dirty(new);
> > @@ -234,6 +237,20 @@ static inline pte_t pte_next_swp_offset(pte_t pte)
> >       return new;
> >  }
> >
> > +
> > +/**
> > + * pte_next_swp_offset - Increment the swap entry offset field of a sw=
ap pte.
> > + * @pte: The initial pte state; is_swap_pte(pte) must be true and
> > + *    non_swap_entry() must be false.
> > + *
> > + * Increments the swap offset, while maintaining all other fields, inc=
luding
> > + * swap type, and any swp pte bits. The resulting pte is returned.
> > + */
> > +static inline pte_t pte_next_swp_offset(pte_t pte)
> > +{
> > +     return pte_move_swp_offset(pte, 1);
> > +}
> > +
> >  /**
> >   * swap_pte_batch - detect a PTE batch for a set of contiguous swap en=
tries
> >   * @start_ptep: Page table pointer for the first entry.
>

Barry