Re: [PATCH v2 5/8] mm/internal: rename swap offset helpers to softleaf offset

["Garg, Shivank" <shivankg@amd.com>]

On Tue, 2026-09-08 at 11:08 +0530, Dev Jain wrote:
> 
> On 08/09/26 3:03 am, Barry Song wrote:
> > On Mon, Sep 7, 2026 at 1:38 PM Dev Jain <dev.jain@arm.com> wrote:
> > > 
> > > 
> > > 
> > > On 05/09/26 4:12 pm, Barry Song wrote:
> > > > On Tue, Sep 1, 2026 at 1:44 PM Dev Jain <dev.jain@arm.com> wrote:
> > > > > 
> > > > > In preparation for adding a helper to set softleaf ptes in one go,
> > > > > generalize the swap entry helpers shifting the swap offset by delta,
> > > > > for softleaves.
> > > > > 
> > > > > Note that the soft-dirty bit, exclusive bit and uffd bit preservation
> > > > > will still work for non-swap softleaves, since a softleaf entry is
> > > > > constructed out of a type and offset, and those bits are ahead of
> > > > > the soft-dirty, exclusive and uffd bits.
> > > > > 
> > > > > For example, for a migration entry, pte_swp_exclusive() will return
> > > > > false, as the exclusivity is encoded in the type itself
> > > > > (SOFTLEAF_MIGRATION_READ_EXCLUSIVE).
> > > > 
> > > > I don't quite understand why you mention this. Is anyone calling
> > > > `pte_swp_exclusive()` on a migration entry? Shouldn't it only be called
> > > > when `softleaf_is_swap()` is true?
> > > 
> > > You are right. I just wanted to emphasize the second paragraph - that the
> > > pte_move_swp_offset will also work for softleaf entries. But I think
> > > the third para confuses more, I'll drop it.
> > 
> > I would rather interpret this as meaning that
> > `pte_move_softleaf_offset` will also work for swap softleafs,
> > since `pte_move_softleaf_offset` will call some pure-swap
> > functions？
> 
> Yes, so if you see remove_migration_pte, there pte_swp_uffd and
> pte_swp_soft_dirty are being used. Same with restore_exclusive_pte
> (for device-exclusive stuff).
> 
> Which means that pte_swp_exclusive is the only one being used
> *only* for swap entries.
> 
> 
> > 
> > Would it be possible to use `if (softleaf_is_swap())` for
> > these cases to make the intent clearer? or we can
> > keep both pte_move_softleaf_offset and pte_move_swap_offset?
> 
> So in my patch I can do
> 
> if (softleaf_is_swap(entry) && pte_swp_exclusive(pte))
> 
> That would make the intent clear.
> 
> 
> > 
> > As `pte_move_softleaf_offset()` is getting a bit weird now,
> > a generic softleaf function has a lot of swap-specific code in it:
> > 
> > static inline pte_t pte_move_softleaf_offset(pte_t pte, long delta)
> > {
> >         const softleaf_t entry = softleaf_from_pte(pte);
> >         pte_t new = __swp_entry_to_pte(__swp_entry(swp_type(entry),
> >                                                    (swp_offset(entry)
> > + delta)));
> > 
> >         if (pte_swp_soft_dirty(pte))
> >                 new = pte_swp_mksoft_dirty(new);
> >         if (pte_swp_exclusive(pte))
> >                 new = pte_swp_mkexclusive(new);
> >         if (pte_swp_uffd(pte))
> >                 new = pte_swp_mkuffd(new);
> > 
> >         return new;
> > }
> > 
> > Am I missing something here?
> > 
> > > 
> > > > 
> > > > > 
> > > > > Signed-off-by: Dev Jain <dev.jain@arm.com>
> > > > 
> > > > Reviewed-by: Barry Song <baohua@kernel.org>
> > > 
> > > Thanks.
> > > 
> > > 
> > > > 
> > > > > ---
> > > > >  mm/internal.h | 29 +++++++++++++++--------------
> > > > >  mm/memory.c   |  4 ++--
> > > > >  2 files changed, 17 insertions(+), 16 deletions(-)
> > > > > 
> > > > [...]
> > > > > 
> > > > >  /**
> > > > > @@ -523,7 +524,7 @@ static inline pte_t pte_next_swp_offset(pte_t pte)
> > > > >   */
> > > > >  static inline int swap_pte_batch(pte_t *start_ptep, int max_nr, pte_t pte)
> > > > >  {
> > > > 
> > > > We might find a user for this in the future, in which case we might
> > > > want to rename `swap_pte_batch()` to `swap_softleaf_batch()`?
> > > 
> > > That is what is being done here:
> > > https://lore.kernel.org/all/20260813-migrate-rmap-batch-v2-1-3c5424c555c7@amd.com/
> > > 
> > > I don't have a strong opinion, I can also generalize this right now.
> > 
> > I don't know how you and Shivank are collaborating,
> > since you're both changing the same thing at the same time :-)
> 
> We are not collaborating really : ) he should have ideally waited
> for my patchset to go in first since I was doing big changes in
> rmap.c.
> 

Just to clarify, my series is already based on your V1 series, as noted
in its cover letter and b4 prerequisite-message-id.

The intention was always for your series to go in first. I posted mine
meanwhile to collect review rather than wait for yours to land. 

> > 
> > Hopefully, we can get one of yours into mm-new first,
> > and the other one can find a way to resolve the conflicts.
> 
> Yes.
> 

I agree there is some overlap in rmap.c, and I will rebase my series on your
latest version to avoid conflicts.

Regarding softleaf_pte_batch(), my patch factors the generic logic out of
swap_pte_batch() because the migration path (migration_pte_batch) needs it.
I suggest keeping it in my series, but I can drop that patch if you prefer
to carry this revised patch[1] in yours.

[1] https://lore.kernel.org/all/89d8c2d6f35f7fdc3444ea3a62ccd244945d9644.camel@amd.com

Thanks,
Shivank