Re: [PATCH] mm/vmemmap/devdax: Fix kernel crash when probing devdax devices

[Joao Martins <joao.m.martins@oracle.com>]

On 10/04/2023 22:39, Jane Chu wrote:
> On 4/10/2023 10:47 AM, Joao Martins wrote:
>> On 10/04/2023 18:27, Jane Chu wrote:
>>> On 4/8/2023 3:19 AM, Aneesh Kumar K.V wrote:
>>>> Jane Chu <jane.chu@oracle.com> writes:
>>>>> On 4/7/2023 5:23 AM, Aneesh Kumar K.V wrote:
>>>>>> diff --git a/mm/Kconfig b/mm/Kconfig
>>>>>> index ff7b209dec05..99f87c1be1e8 100644
>>>>>> --- a/mm/Kconfig
>>>>>> +++ b/mm/Kconfig
>>>>>> @@ -461,6 +461,9 @@ config SPARSEMEM_VMEMMAP
>>>>>>           pfn_to_page and page_to_pfn operations.  This is the most
>>>>>>           efficient option when sufficient kernel resources are available.
>>>>>>     +config ARCH_WANT_OPTIMIZE_VMEMMAP
>>>>>> +    bool
>>>>>> +
>>>>>
>>>>> Could this devdax specific config switch be added to drivers/dax/Kconfig
>>>>> ? also, how about adding 'DAX' to the config switch name?
>>>>
>>>> I would say we want to make it more generic. ie, both hugetlb and devdax
>>>> can now derive the feature support via ARCH_WANT_OPTIMIZE_VMEMAP
>>>
>>> The two config switches have different purposes, it's better to keep them
>>> separate.  For example, recent hugetlb high granularity mapping (HGM) project
>>> requires to users to make a choice between HGM and hugetlb vmemmap
>>> optimization(at least for now), while one can keep devdax compound page support
>>> enabled.
>>>
>>
>> Is it done by kconfig? If it helps:
>>
>> * there's a static key hugetlb can use to tell if this is enabled or not like
>> how page_fixed_fake_head() uses it:
>>
>> if (!static_branch_unlikely(&hugetlb_optimize_vmemmap_key))
>>     ...
>>
>> * there's a hugetlb page bit for that vmemmap optimized pages with
>> HpageOptimized(page).
>>
>> * there is a separate hugetlb CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP which is not
>> the same as the ARCH kconfig.
>>
>> But perhaps there's some relevance in the ARCH_HUGETLB specific to HGMv2 that I
>> am unaware.
>>
> 
> Looks like there is no consumer of CONFIG_ARCH_WANT_HUGETLB_PAGE_OPTIMIZE_VMEMMAP.
> There are three ways to enable HVO via
> CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON alone, or a combination of
> CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP and boot param 'vmemmap_optimize_enabled'
> or sysctl variable /proc/sys/vm/hugetlb_optimize_vmemmap.
> 
> It seems that the devdax compound page support 

Let's call it devdax vmemmap deduplication as compound pages can work regardless
of the trick.

> does not cross path with HVO
> which has a lot more moving arms, and so it's better to keep the devdax config
> switch local to devdax and leave HVO alone.

I agree; I would rather not change
CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP_DEFAULT_ON nor
CONFIG_HUGETLB_PAGE_OPTIMIZE_VMEMMAP and its knobs. But to be clear the one I
was talking about is ARCH_WANT_HUGETLB_PAGE_OPTIMIZE_VMEMMAP (which is selected
by architecture) ... not the other ones. Which if it was a generic one would
translate to 'can the architecture support deduplicated vmemmap' regardless of
whether it is active or used by DAX/HVO. HVO would use its hugetlb kconfig knobs
and function helpers pointed above to differentiate in the HVO-specific case as
it does today. Perhaps we can hear also from Muchun on what he thinks is right
for HVO.

	Joao