Re: [RESEND v7 06/29] s390: mm: add PMD swap-exclusive helpers

[David Hildenbrand (Arm)]

On 9/14/26 14:27, Usama Arif wrote:
> A later patch keeps a PMD-mapped anonymous THP mapped by a PMD across the
> swap round-trip, so PG_anon_exclusive now has to survive in a swap PMD and
> not just in a swap PTE.
> 
> s390 is the one architecture where a swap PMD is not a swap PTE in
> disguise: it is an RSTE with its own layout, converted to a fake PTE swap
> entry for the common code. Give it its own exclusive bit rather than
> borrowing the PTE-format macro. The two happen to have the same value, but
> that is a coincidence. Bit 52 was documented as unused; document what it
> is now.
> 
> Cc: Alexander Gordeev <agordeev@xxxxxxxxxxxxx>
> Cc: Gerald Schaefer <gerald.schaefer@xxxxxxxxxxxxx>
> Cc: Heiko Carstens <hca@xxxxxxxxxxxxx>
> Cc: Vasily Gorbik <gor@xxxxxxxxxxxxx>
> Signed-off-by: Usama Arif <usama.arif@xxxxxxxxx>
> ---
>  arch/s390/include/asm/pgtable.h | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
> index 2d5c2ab06de98..0790a0884cfab 100644
> --- a/arch/s390/include/asm/pgtable.h
> +++ b/arch/s390/include/asm/pgtable.h
> @@ -333,6 +333,7 @@ void setup_protection_map(void);
>  /* Common bits in region and segment table entries, for swap entries */
>  #define _RST_ENTRY_COMM		0x0010	/* Common-Region/Segment, marks swap entry */
>  #define _RST_ENTRY_INVALID	0x0020	/* invalid region/segment table entry */
> +#define _RST_ENTRY_SWP_EXCLUSIVE 0x0800	/* SW exclusive swap bit, see mk_swap_rste() */
>  
>  #define _CRST_ENTRIES	2048	/* number of region/segment table entries */
>  #define _PAGE_ENTRIES	256	/* number of page table entries	*/
> @@ -859,6 +860,28 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
>  	return clear_pte_bit(pte, __pgprot(_PAGE_SWP_EXCLUSIVE));
>  }
>  
> +#ifdef CONFIG_ARCH_HAS_PMD_SOFTLEAVES
> +/*
> + * A PMD swap entry is an RSTE, not a PTE, so it needs its own exclusive bit
> + * rather than the PTE-format _PAGE_SWP_EXCLUSIVE.  The two happen to have the
> + * same value; see the RSTE swap layout above mk_swap_rste().
> + */

I'm not sure the documentation here is warranted. It's all documented above
above __SWP_OFFSET_MASK_RSTE, no? I'd just extend the documentation there and
keep it away from these helpers that just use the bit.

The fact that they use the same bit doesn't really matter.

> +static inline pmd_t pmd_swp_mkexclusive(pmd_t pmd)
> +{
> +	return set_pmd_bit(pmd, __pgprot(_RST_ENTRY_SWP_EXCLUSIVE));
> +}
> +
> +static inline bool pmd_swp_exclusive(pmd_t pmd)
> +{
> +	return pmd_val(pmd) & _RST_ENTRY_SWP_EXCLUSIVE;
> +}
> +
> +static inline pmd_t pmd_swp_clear_exclusive(pmd_t pmd)
> +{
> +	return clear_pmd_bit(pmd, __pgprot(_RST_ENTRY_SWP_EXCLUSIVE));
> +}
> +#endif

I guess we could move it above the pmd_swp_soft_dirty() stuff in the same
CONFIG_ARCH_HAS_PMD_SOFTLEAVES block.

> +
>  static inline int pte_soft_dirty(pte_t pte)
>  {
>  	return pte_val(pte) & _PAGE_SOFT_DIRTY;
> @@ -1900,15 +1923,16 @@ static inline swp_entry_t __swp_entry(unsigned long type, unsigned long offset)
>   * Bits 59 and 63 are used to indicate the swap entry. Bit 58 marks the rste
>   * as invalid.
>   * A swap entry is indicated by bit pattern (rste & 0x011) == 0x010
> - * |			  offset			|Xtype |11TT|S0|
> + * |			  offset			|Etype |11TT|S0|
>   * |0000000000111111111122222222223333333333444444444455|555555|5566|66|
>   * |0123456789012345678901234567890123456789012345678901|234567|8901|23|
>   *
>   * Bits 0-51 store the offset.
> + * Bit 52 (E) is used to remember PG_anon_exclusive
> + * (_RST_ENTRY_SWP_EXCLUSIVE), mirroring bit 52 of a swap pte.
>   * Bits 53-57 store the type.
>   * Bit 62 (S) is used for softdirty tracking.
>   * Bits 60-61 (TT) indicate the table type: 0x01 for REGION3 and 0x00 for SEGMENT.
> - * Bit 52 (X) is unused.
>   */
>  
>  #define __SWP_OFFSET_MASK_RSTE	((1UL << 52) - 1)

IIRC, __pmd_to_swp_entry() and __swp_entry_to_pmd() will lose the flag, which is
the right thing to do.

So conceptually LGTM.

-- 
Cheers,

David