Re: [PATCH 3/3] x86/mm: Fix and document DEBUG_PAGEALLOC

[Mike Rapoport <rppt@kernel.org>]

On Wed, Jul 29, 2026 at 01:08:10PM +0200, Peter Zijlstra wrote:
> It turns out that commit 5fce67641a3e ("x86/mm/pat: Don't gate
> cpa_lock on debug_pagealloc_enabled()") was a little too quick to
> remove the debug_pagealloc exception for cpa_lock.
> 
> Notably __kernel_map_pages() is used by the page-allocator from any
> context the page-allocator itself is used, which violates the cpa_lock
> rules.
> 
> Re-instate the exception, except make it specific to the
> __kernel_map_pages() such that any other cpa() usage is still fully
> serialized by cpa_lock. Also note that since cpa() should not be used
> on memory that isn't allocated, the page-allocator locking and cpa are
> infact mutually exclusive and all cpa usage in fully serialized.
> 
> Add a comment explaining this and other 'funnies' surrounding
> DEBUG_PAGEALLOC, including how pgd_lock is not affected and the TLB
> trickery.
> 
> Fixes: 5fce67641a3e ("x86/mm/pat: Don't gate cpa_lock on debug_pagealloc_enabled()")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> +	/*
> +	 * DEBUG_PAGEALLOC is special; it is called from any context the
> +	 * page-allocator is, which violates the normal cpa_lock locking
> +	 * rules.
> +	 *
> +	 * However, since it is part of the page-allocator, things are still
> +	 * properly serialized by the page-allocator locking and the fact that
> +	 * when a page is owned by the page-allocator, it isn't owned by
> +	 * anybody else. That is, you *SHOULD* not be calling cpa() on memory

                                     *SHOULD NOT* ?

> +	 * that isn't allocated.
> +	 *
> +	 * Additionally, DEBUG_PAGEALLOC ensures (per probe_page_size_mask())
> +	 * that the kernel mapping is 4k pages, therefore there are no large
> +	 * pages to split/collapse.
> +	 *
> +	 * Furthermore, the page-allocator strictly manages pages that
> +	 * *exist*, avoiding pgd_lock.
> +	 *
> +	 * Therefore, it is safe to not take cpa_lock.
> +	 */
> +	if (debug_pagealloc_enabled() && (cpa->flags & CPA_DEBUG_PAGEALLOC))
> +		lock = false;
> +
>  	while (rempages) {
>  		/*
>  		 * Store the remaining nr of pages for the large page
> @@ -2008,9 +2033,12 @@ static int __change_page_attr_set_clr(st
>  		if (cpa->flags & (CPA_ARRAY | CPA_PAGES_ARRAY))
>  			cpa->numpages = 1;
>  
> -		spin_lock(&cpa_lock);
> -		ret = __change_page_attr(cpa, primary);
> -		spin_unlock(&cpa_lock);
> +		if (lock) {
> +			guard(spinlock)(&cpa_lock);
> +			ret = __change_page_attr(cpa, primary);
> +		} else {
> +			ret = __change_page_attr(cpa, primary);
> +		}

This does make DEBUG_PAGEALLOC exception more explicit *here*, but OTOH the
spin_(un)lock(&cpa_lock) in split_large_page() becomes confusing.

I like my version with your comments added there more as it localizes the
DEBUG_PAGEALLOC exception in the lock wrappers.

>  		if (ret)
>  			goto out;
>  
> @@ -2661,15 +2689,23 @@ void __kernel_map_pages(struct page *pag
>  	 * and hence no memory allocations during large page split.
>  	 */

I'd also return early and maybe even WARN if !debug_pagealloc_enabled(). 

>  	if (enable)
> -		__set_pages_p(page, numpages);
> +		__set_pages_p(page, numpages, CPA_DEBUG_PAGEALLOC);
>  	else
> -		__set_pages_np(page, numpages);
> +		__set_pages_np(page, numpages, CPA_DEBUG_PAGEALLOC);

-- 
Sincerely yours,
Mike.