From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A07DE1A239A for ; Tue, 21 Jul 2026 15:49:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784648968; cv=none; b=Hvgd+TwvCPeYbnSPY51JnEjKaA9X2wsJGZFs6Th1NNqPJ9dc7VoYz0ncWl4GiSug3NdJW7v8OoufBUTNoPaxPcTaWOZOgRhCOodCb+CZAdDv++Hl6zDxJmVFI287X6Ey07JGVB4dja0vYPI/ACB6coY/gyYuq5/fm37AuE626Og= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784648968; c=relaxed/simple; bh=hbP4mfRKQY1noI33646BgOLSYOPy02XLX24StkOdBOA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=FaV7fHgIuWHO9CjkTnvOCX6VloMhAfkkHQGeNikKhwjXDbJU/rs99KFyGOHrOa5wVgTiPKrlEhi6XQtO7nvBKnfrdUGUU8ndNQU+GWhVYHxUqnZPtppWcTzotmIf/J1WyGXd5Dph4kyu0sxw0R9fUN+8Kg8YoWIM8SmIJKH0kjQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=b5NvOseq; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="b5NvOseq" Received: by smtp.kernel.org (Postfix) with ESMTPSA id BD6351F000E9; Tue, 21 Jul 2026 15:49:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1784648967; bh=xubpAK0mpTPDcCDKg/2CO+wuSDHly0ApJCl7pbc+HfQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=b5NvOseqDXyuCEeohdTJRRzf1EwPa35JjB339YyutZ9ibg8RjZ8xJ10g3g9Nc34Il LJsGzkm845eS9DslW1ZHygXgkDWZY944l8HQJYpgZEQ7lyeZDRfghgAsvfn9M+jR1m VkfmSXw83xVIHVzzppQx+M4mGT9Ds/cE8fmcL7+7TilK26bA/ai20TNNEsM1XXwWKI CyaLVPLHCffBYsklIOuG4TOO6v+zBDRryd/DGsedz4lhyhH1cTiziULM0qf0YkpxIx QYqD/Pd9Ih7nlx/zT3is9nmBmjaFJCO83TnOPtV+VqYFkcELvAQoSVTm5e31ZzL7TH 0HTgUzIzfoSbQ== Date: Tue, 21 Jul 2026 16:49:13 +0100 From: "Lorenzo Stoakes (ARM)" To: Mike Rapoport Cc: Dave Hansen , Andy Lutomirski , Borislav Petkov , "Denis V . Lunev" , Ingo Molnar , Juergen Gross , Kiryl Shutsemau , "H. Peter Anvin" , Peter Zijlstra , Thomas Gleixner , linux-kernel@vger.kernel.org, x86@kernel.org, Dave Hansen Subject: Re: [PATCH] x86/mm/pat: don't gate cpa_lock on debug_pagealloc_enabled() Message-ID: References: <20260715144519.934289-1-rppt@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260715144519.934289-1-rppt@kernel.org> On Wed, Jul 15, 2026 at 05:45:19PM +0300, Mike Rapoport wrote: > From: "Mike Rapoport (Microsoft)" > > Dave Hansen says: > My only question is *why*!?!? Why add extra locking complexity and rules > to optimize debug_pagealloc, which is already horrendously slow. > > Stop gating cpa_lock on debug_pagealloc_enabled() to simplify the code. > > Suggested-by: Dave Hansen > Signed-off-by: Mike Rapoport (Microsoft) > --- > 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); -> _irqsave() is needed I think :) see below > ptdesc = pagetable_alloc(GFP_KERNEL, 0); > - if (!debug_pagealloc_enabled()) > - spin_lock(&cpa_lock); > + spin_lock(&cpa_lock); -> _irqrestore() as below > 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); __kernel_map_pages() can be called from irq context: < GFP_ATOMIC context > kfree() or whatever -> ... -> __free_pages_prepare() -> debug_pagealloc_unmap_pages() -> __kernel_map_pages() -> __change_page_attr_set_clr() -> cpa_lock spins [irqs off] Sooo you're spin locking in irq context here, which is probably not a good idea. All the cpa_lock spin locks have to be updated to reflect this. So spin_lock_irqsave/restore I think? But note that this turns Denis's cpa_lock patch ([0]) into a deadlock because it's held across an IPI on TLB flush. So that has to be changed too, or possibly dropped. I'll reply about that over there! > if (ret) > goto out; > > > base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa > -- > 2.53.0 > > Thanks, Lorenzo [0]:https://lore.kernel.org/all/20260715183453.2381141-1-den@openvz.org/