Re: [PATCH v1 4/7] arm64/pgtable: support __HAVE_ARCH_PTE_SWP_EXCLUSIVE

[Will Deacon <will@kernel.org>]

On Mon, Mar 21, 2022 at 04:07:48PM +0100, David Hildenbrand wrote:
> On 21.03.22 15:38, Will Deacon wrote:
> > On Wed, Mar 16, 2022 at 06:27:01PM +0000, Catalin Marinas wrote:
> >> On Tue, Mar 15, 2022 at 03:18:34PM +0100, David Hildenbrand wrote:
> >>> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
> >>> index b1e1b74d993c..62e0ebeed720 100644
> >>> --- a/arch/arm64/include/asm/pgtable-prot.h
> >>> +++ b/arch/arm64/include/asm/pgtable-prot.h
> >>> @@ -14,6 +14,7 @@
> >>>   * Software defined PTE bits definition.
> >>>   */
> >>>  #define PTE_WRITE		(PTE_DBM)		 /* same as DBM (51) */
> >>> +#define PTE_SWP_EXCLUSIVE	(_AT(pteval_t, 1) << 2)	 /* only for swp ptes */
> >>
> >> I think we can use bit 1 here.
> >>
> >>> @@ -909,12 +925,13 @@ static inline pmd_t pmdp_establish(struct vm_area_struct *vma,
> >>>  /*
> >>>   * Encode and decode a swap entry:
> >>>   *	bits 0-1:	present (must be zero)
> >>> - *	bits 2-7:	swap type
> >>> + *	bits 2:		remember PG_anon_exclusive
> >>> + *	bits 3-7:	swap type
> >>>   *	bits 8-57:	swap offset
> >>>   *	bit  58:	PTE_PROT_NONE (must be zero)
> >>
> >> I don't remember exactly why we reserved bits 0 and 1 when, from the
> >> hardware perspective, it's sufficient for bit 0 to be 0 and the whole
> >> pte becomes invalid. We use bit 1 as the 'table' bit (when 0 at pmd
> >> level, it's a huge page) but we shouldn't check for this on a swap
> >> entry.
> > 
> > I'm a little worried that when we're dealing with huge mappings at the
> > PMD level we might lose the ability to distinguish them from a pte-level
> > mapping with this new flag set if we use bit 1. A similar issue to this
> > was fixed a long time ago by 59911ca4325d ("ARM64: mm: Move PTE_PROT_NONE
> > bit") when we used to use bit 1 for PTE_PROT_NONE.
> > 
> > Is something like:
> > 
> > 	pmd_to_swp_entry(swp_entry_to_pmd(pmd));
> 
> Note that __HAVE_ARCH_PTE_SWP_EXCLUSIVE currently only applies to actual
> swap entries, not non-swap entries (migration, hwpoison, ...). So it
> really only applies to PTEs -- PMDs are not applicable.

Right, thanks for the clarification.

> So the example you gave cannot possibly have that bit set. From what I
> understand, it should be fine. But I have no real preference: I can also
> just stick to the original patch, whatever you prefer.

I think I'd prefer to stay on the safe side and stick with bit 2 as you
originally proposed. If we need to support crazy numbers of swapfiles
in future then we can revisit the idea of allocating bit 1.

Thanks, and sorry for the trouble.

Will