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

[Dev Jain <dev.jain@arm.com>]

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.


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

Yes.


> 
>>
>>
>>>
>>>> -       pte_t expected_pte = pte_next_swp_offset(pte);
>>>> +       pte_t expected_pte = pte_next_softleaf_offset(pte);
>>>
> 
> Best Regards
> Barry