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
