Received-SPF: pass (google.com: domain of linux-kernel+bounces-168205-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240503005023.174597-1-21cnbao@gmail.com> <20240503005023.174597-4-21cnbao@gmail.com>
In-Reply-To: <20240503005023.174597-4-21cnbao@gmail.com>
From: Chris Li <chrisl@kernel.org>
Date: Fri, 3 May 2024 13:51:11 -0700
Message-ID: <CANeU7Q=RFC7i6q=BjK3cXG+J0U2KeGMDuz0sV1K2sG2ifvpYcw@mail.gmail.com>
Subject: Re: [PATCH v3 3/6] mm: introduce pte_move_swp_offset() helper which
 can move offset bidirectionally
To: Barry Song <21cnbao@gmail.com>
Cc: akpm@linux-foundation.org, linux-mm@kvack.org, 
	baolin.wang@linux.alibaba.com, david@redhat.com, hanchuanhua@oppo.com, 
	hannes@cmpxchg.org, hughd@google.com, kasong@tencent.com, 
	linux-kernel@vger.kernel.org, ryan.roberts@arm.com, 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 Thu, May 2, 2024 at 5:51=E2=80=AFPM Barry Song <21cnbao@gmail.com> 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 introduces
> pte_move_swp_offset(), a function capable of bidirectional movement by
> a specified delta argument. Consequently, pte_increment_swp_offset()
> will directly invoke it with delta =3D 1.

BTW, this patch has conflict with the latest mm-unstable. You might
need to refresh it.

I do not see the delta =3D -1 usage case here. Maybe merge the patch
with the follow up patch that uses the delta =3D -1?
Does delta only make sense as 1 and -1?

This patch doesn't seem straightly necessary to me.

Chris


Chris

>
> 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 *fol=
io, unsigned long addr,
>  }
>
>  /**
> - * pte_next_swp_offset - Increment the swap entry offset field of a swap=
 pte.
> + * pte_move_swp_offset - Move the swap entry offset field of a swap pte
> + *      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 delta
> + *      is positive; backward if delta is negative
>   *
> - * Increments the swap offset, while maintaining all other fields, inclu=
ding
> + * Moves the swap offset, while maintaining all other fields, including
>   * 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)
>  {
>         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 swap=
 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, inclu=
ding
> + * 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 entr=
ies
>   * @start_ptep: Page table pointer for the first entry.
> --
> 2.34.1
>
>