Re: [PATCH RFC v2 12/20] x86: mm: define pudp_set_access_flags() when CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD is enabled only.

From: "David Hildenbrand (Arm)" <david@kernel.org>
To: Yeoreum Yun <yeoreum.yun@arm.com>, Dave Hansen <dave.hansen@intel.com>
Cc: Russell King <linux@armlinux.org.uk>,
	Huacai Chen <chenhuacai@kernel.org>,
	WANG Xuerui <kernel@xen0n.name>,
	Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Kairui Song <kasong@tencent.com>, Qi Zheng <qi.zheng@linux.dev>,
	Shakeel Butt <shakeel.butt@linux.dev>,
	Barry Song <baohua@kernel.org>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Yuanchu Xie <yuanchu@google.com>, Wei Xu <weixugc@google.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@kernel.org>,
	Lorenzo Stoakes <ljs@kernel.org>,
	Tianrui Zhao <zhaotianrui@loongson.cn>,
	Bibo Mao <maobibo@loongson.cn>, Anup Patel <anup@brainfault.org>,
	Atish Patra <atish.patra@linux.dev>,
	Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Alexandre Ghiti <alex@ghiti.fr>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@kernel.org>, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	"Liam R. Howlett" <liam@infradead.org>,
	Vlastimil Babka <vbabka@kernel.org>,
	Mike Rapoport <rppt@kernel.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Michal Hocko <mhocko@suse.com>, Jonas Bonn <jonas@southpole.se>,
	Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>,
	Stafford Horne <shorne@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, loongarch@lists.linux.dev,
	linux-mips@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-mm@kvack.org, kvm@vger.kernel.org,
	kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org,
	linux-openrisc@vger.kernel.org
Subject: Re: [PATCH RFC v2 12/20] x86: mm: define pudp_set_access_flags() when CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD is enabled only.
Date: Mon, 27 Jul 2026 17:02:00 +0200	[thread overview]
Message-ID: <4dbf7e54-a541-45de-b4b9-43ee5add287b@kernel.org> (raw)
In-Reply-To: <amD9iD7rjmU__9b6@e129823.arm.com>

On 7/22/26 19:27, Yeoreum Yun wrote:
> Hi Dave,
> 
>>> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
>>> index f32facdb3035..edad847a2ecd 100644
>>> --- a/arch/x86/mm/pgtable.c
>>> +++ b/arch/x86/mm/pgtable.c
>>> @@ -411,6 +411,7 @@ int pmdp_set_access_flags(struct vm_area_struct *vma,
>>>  	return changed;
>>>  }
>>>  
>>> +#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>>>  int pudp_set_access_flags(struct vm_area_struct *vma, unsigned long address,
>>>  			  pud_t *pudp, pud_t entry, int dirty)
>>>  {
>>> @@ -430,6 +431,7 @@ int pudp_set_access_flags(struct vm_area_struct *vma, unsigned long address,
>>>  
>>>  	return changed;
>>>  }
>>> +#endif /* CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
>>>  #endif
>>>  
>>>  bool ptep_test_and_clear_young(struct vm_area_struct *vma,
>>
>> #ifdefs in .c files are evil.
>>
>> The changelog doesn't make a strong enough case for why this evil should
>> be tolerated.
>>
>> These are also _precisely_ the kind of #ifdefs that cause compilation
>> problems. This one is:
>>
>> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> /// function here
>> #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>> /// another function here
>> #endif
>> #endif
>>
>> So there end up being a couple of dependent config options in play. If
>> there are compile problems, this makes them harder to find.
>>
>> What is the _actual_ goal here? Saving 50 bytes of kernel text?
> 
> TBH, this came from for v1's change of behavior set_pud() where
> triggered compiliation problem with v2 this change wouldn't require.

If the patch is not required right now, let's drop it.

I agree that it's the right thing to do: just look at pudp_invalidate() in the
very same file, but if we can reduce the churn and leave the cleanups to x86
folks, that seems to be preferred.

-- 
Cheers,

David