Re: [PATCH 2/8] x86/mm: Always "broadcast" PMD setting operations

[Dave Hansen <dave.hansen@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

On 4/15/25 01:25, Kirill A. Shutemov wrote:
>>  #ifdef CONFIG_X86_32
>> -	if (!SHARED_KERNEL_PMD) {
>> +	{
>>  		struct page *page;
>>  
>>  		list_for_each_entry(page, &pgd_list, lru) {
> Removing the condition, but leaving the block looks sloppy.
> 
> Maybe convert #ifdef to IS_ENABLED() while you are there, so it would
> justify the block?

It does, and it's right at the beginning of the function. Simplifying
the code here also made it _less_ self-documenting so it needs a better
comment too.

I'll tack the attached patch on to the end of the series.

[-- Attachment #2: kill-CONFIG_X86_32-ifdef.patch --]
[-- Type: text/x-patch, Size: 2073 bytes --]


This block of code used to be:

	if (SHARED_KERNEL_PMD)

But it was zapped when 32-bit kernels transitioned to private
(non-shared) PMDs. It also made it rather unclear what the block
of code is doing in the first place.

Remove the #ifdef and replace it with IS_ENABLED(). Unindent the
code block and add an actually useful comment about what it is
doing.

Suggested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

---

 b/arch/x86/mm/pat/set_memory.c |   41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff -puN arch/x86/mm/pat/set_memory.c~kill-CONFIG_X86_32-ifdef arch/x86/mm/pat/set_memory.c
--- a/arch/x86/mm/pat/set_memory.c~kill-CONFIG_X86_32-ifdef	2025-04-15 06:45:17.579717047 -0700
+++ b/arch/x86/mm/pat/set_memory.c	2025-04-15 06:53:27.890709422 -0700
@@ -881,31 +881,32 @@ phys_addr_t slow_virt_to_phys(void *__vi
 }
 EXPORT_SYMBOL_GPL(slow_virt_to_phys);
 
-/*
- * Set the new pmd in all the pgds we know about:
- */
 static void __set_pmd_pte(pte_t *kpte, unsigned long address, pte_t pte)
 {
+	struct page *page;
+
 	/* change init_mm */
 	set_pte_atomic(kpte, pte);
-#ifdef CONFIG_X86_32
-	{
-		struct page *page;
-
-		list_for_each_entry(page, &pgd_list, lru) {
-			pgd_t *pgd;
-			p4d_t *p4d;
-			pud_t *pud;
-			pmd_t *pmd;
-
-			pgd = (pgd_t *)page_address(page) + pgd_index(address);
-			p4d = p4d_offset(pgd, address);
-			pud = pud_offset(p4d, address);
-			pmd = pmd_offset(pud, address);
-			set_pte_atomic((pte_t *)pmd, pte);
-		}
+
+	if (IS_ENABLED(CONFIG_X86_64))
+		return;
+
+	/*
+	 * 32-bit mm_structs don't share kernel PMD pages.
+	 * Propagate the change to each relevant PMD entry:
+	 */
+	list_for_each_entry(page, &pgd_list, lru) {
+		pgd_t *pgd;
+		p4d_t *p4d;
+		pud_t *pud;
+		pmd_t *pmd;
+
+		pgd = (pgd_t *)page_address(page) + pgd_index(address);
+		p4d = p4d_offset(pgd, address);
+		pud = pud_offset(p4d, address);
+		pmd = pmd_offset(pud, address);
+		set_pte_atomic((pte_t *)pmd, pte);
 	}
-#endif
 }
 
 static pgprot_t pgprot_clear_protnone_bits(pgprot_t prot)
_