Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 20 May 2021 19:41:03 -0700 (PDT)
From:   Hugh Dickins <hughd@google.com>
To:     Russell King <rmk+kernel@armlinux.org.uk>
cc:     linux-arm-kernel@lists.infradead.org,
        Anshuman Khandual <anshuman.khandual@arm.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Allison Randal <allison@lohutok.net>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ARM: update __swp_entry_to_pte() to use PTE_TYPE_FAULT
In-Reply-To: <E1lh9VH-0002Lz-Pd@rmk-PC.armlinux.org.uk>
Message-ID: <alpine.LSU.2.11.2105201917270.5752@eggly.anvils>
References: <E1lh9VH-0002Lz-Pd@rmk-PC.armlinux.org.uk>
User-Agent: Alpine 2.11 (LSU 23 2013-08-11)
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Precedence: bulk

On Thu, 13 May 2021, Russell King wrote:

> Swap entries use a faulting PTE which have the least two significant
> bits as zero. Due to this, the use of PTE_TYPE_FAULT was overlooked,
> but really should have been included in __swp_entry_to_pte().
> 
> Convert this macro to use PTE_TYPE_FAULT to properly document what is
> going on here, and use __pte() to convert the swp_entry_t to a pte_t.
> 
> This results in no change to the resulting kernel text.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Hmm.  Speaking as one who occasionally has to trawl through all
the architectures considering their __swp_entry() implementations,
I would much prefer you to drop this patch: I don't think it helps
anyone to insist on ORing in (something which when one searches
further turns out to be) 0.

But if you really want to keep it, please remember it's not just
__swp_entry_to_pte() that "needs" changing: __pte_to_swp_entry()
ought to mask it off, pte_clear() ought to set it, pte_none()
ought to compare against it, page table initialization ought
to memset with it, and probably more :)

Thanks,
Hugh

> ---
>  arch/arm/include/asm/pgtable.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
> index c02f24400369..c43e07d6046d 100644
> --- a/arch/arm/include/asm/pgtable.h
> +++ b/arch/arm/include/asm/pgtable.h
> @@ -303,7 +303,7 @@ static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
>  #define __swp_entry(type,offset) ((swp_entry_t) { ((type) << __SWP_TYPE_SHIFT) | ((offset) << __SWP_OFFSET_SHIFT) })
>  
>  #define __pte_to_swp_entry(pte)	((swp_entry_t) { pte_val(pte) })
> -#define __swp_entry_to_pte(swp)	((pte_t) { (swp).val })
> +#define __swp_entry_to_pte(swp)	__pte((swp).val | PTE_TYPE_FAULT)
>  
>  /*
>   * It is an error for the kernel to have more swap files than we can
> -- 
> 2.20.1