Re: [PATCH RFC 01/18] x86/mm/pat: Don't gate cpa_lock on debug_pagealloc_enabled()

On Tue, Jul 21, 2026 at 07:23:24PM +0300, Mike Rapoport (Microsoft) wrote:
> The splitting and merging of kernel page table mappings between small and
> large is protected by cpa_lock. The merging is relatively new but the
> splitting is ancient.
>
> The splitting has a locking optimization: since DEBUG_PAGEALLOC forces all
> mappings to 4k, there are no large pages to split. So the code that *might*
> cause a split can just skip the locking (and a few other things).
>
> This is entertaining, but it adds complexity and makes for weird locking
> rules. Plus it's all for a debugging feature which makes the kernel super
> slow in the first place. Optimizing something which is already super slow
> and not used in production is not the best way to spend our complexity
> budget.
>
> Stop gating cpa_lock on debug_pagealloc_enabled() to simplify the code
> and the locking rules.
>
> [ dhansen: flesh out changelog ]
>
> Suggested-by: Dave Hansen <[email protected]>
> Signed-off-by: Mike Rapoport (Microsoft) <[email protected]>
> Signed-off-by: Dave Hansen <[email protected]>
> Link: https://patch.msgid.link/[email protected]
> Link: https://lore.kernel.org/all/[email protected]/

Hmm this patch is already taken separately though? ([0]) (obv. commented there
already with review feedback).

Intended to be with this series as some kind of background or? Probably better
to separate out given it's a live patch

Thanks, Lorenzo

[0]:https://lore.kernel.org/all/[email protected]/


> ---
>  arch/x86/mm/pat/set_memory.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index d023a40a1e03..e8316f5ffa8a 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -62,10 +62,9 @@ enum cpa_warn {
>  static const int cpa_warn_level = CPA_PROTECT;
>
>  /*
> - * Serialize cpa() (for !DEBUG_PAGEALLOC which uses large identity mappings)
> - * using cpa_lock. So that we don't allow any other cpu, with stale large tlb
> - * entries change the page attribute in parallel to some other cpu
> - * splitting a large page entry along with changing the attribute.
> + * Serialize cpa() using cpa_lock so that we don't allow any other cpu, with
> + * stale large tlb entries, to change the page attribute in parallel to some
> + * other cpu splitting a large page entry along with changing the attribute.
>   */
>  static DEFINE_SPINLOCK(cpa_lock);
>
> @@ -1235,11 +1234,9 @@ static int split_large_page(struct cpa_data *cpa, pte_t *kpte,
>  {
>  	struct ptdesc *ptdesc;
>
> -	if (!debug_pagealloc_enabled())
> -		spin_unlock(&cpa_lock);
> +	spin_unlock(&cpa_lock);
>  	ptdesc = pagetable_alloc(GFP_KERNEL, 0);
> -	if (!debug_pagealloc_enabled())
> -		spin_lock(&cpa_lock);
> +	spin_lock(&cpa_lock);
>  	if (!ptdesc)
>  		return -ENOMEM;
>
> @@ -2023,11 +2020,9 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int primary)
>  		if (cpa->flags & (CPA_ARRAY | CPA_PAGES_ARRAY))
>  			cpa->numpages = 1;
>
> -		if (!debug_pagealloc_enabled())
> -			spin_lock(&cpa_lock);
> +		spin_lock(&cpa_lock);
>  		ret = __change_page_attr(cpa, primary);
> -		if (!debug_pagealloc_enabled())
> -			spin_unlock(&cpa_lock);
> +		spin_unlock(&cpa_lock);
>  		if (ret)
>  			goto out;
>
>
> --
> 2.53.0
>