[PATCH v5] arm64: mm: Populate vmemmap/linear at the page level for hotplugged sections

[PATCH v5] arm64: mm: Populate vmemmap/linear at the page level for hotplugged sections

[Zhenhua Huang]

On the arm64 platform with 4K base page config, SECTION_SIZE_BITS is set
to 27, making one section 128M. The related page struct which vmemmap
points to is 2M then.
Commit c1cc1552616d ("arm64: MMU initialisation") optimizes the
vmemmap to populate at the PMD section level which was suitable
initially since hot plug granule is always one section(128M). However,
commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug")
introduced a 2M(SUBSECTION_SIZE) hot plug granule, which disrupted the
existing arm64 assumptions.

Considering the vmemmap_free -> unmap_hotplug_pmd_range path, when
pmd_sect() is true, the entire PMD section is cleared, even if there is
other effective subsection. For example page_struct_map1 and
page_strcut_map2 are part of a single PMD entry and they are hot-added
sequentially. Then page_struct_map1 is removed, vmemmap_free() will clear
the entire PMD entry freeing the struct page map for the whole section,
even though page_struct_map2 is still active. Similar problem exists
with linear mapping as well, for 16K base page(PMD size = 32M) or 64K
base page(PMD = 512M), their block mappings exceed SUBSECTION_SIZE.
Tearing down the entire PMD mapping too will leave other subsections
unmapped in the linear mapping.

To address the issue, we need to prevent PMD/PUD/CONT mappings for both
linear and vmemmap for non-boot sections if corresponding size on the
given base page exceeds SUBSECTION_SIZE(2MB now).

Cc: stable@vger.kernel.org # v5.4+
Fixes: ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug")
Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
---
Hi Catalin and Anshuman,
I have addressed comments so far, please help review.
One outstanding point which not finalized is in vmemmap_populate(): how to judge hotplug
section. Currently I am using system_state, discussion:
https://lore.kernel.org/linux-mm/1515dae4-cb53-4645-8c72-d33b27ede7eb@quicinc.com/
 arch/arm64/mm/mmu.c | 46 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index e2739b69e11b..8718d6e454c5 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -42,9 +42,13 @@
 #include <asm/pgalloc.h>
 #include <asm/kfence.h>
 
-#define NO_BLOCK_MAPPINGS	BIT(0)
-#define NO_CONT_MAPPINGS	BIT(1)
-#define NO_EXEC_MAPPINGS	BIT(2)	/* assumes FEAT_HPDS is not used */
+#define NO_PMD_BLOCK_MAPPINGS	BIT(0)
+#define NO_PUD_BLOCK_MAPPINGS	BIT(1)  /* Hotplug case: do not want block mapping for PUD */
+#define NO_BLOCK_MAPPINGS	(NO_PMD_BLOCK_MAPPINGS | NO_PUD_BLOCK_MAPPINGS)
+#define NO_PTE_CONT_MAPPINGS	BIT(2)
+#define NO_PMD_CONT_MAPPINGS	BIT(3)  /* Hotplug case: do not want cont mapping for PMD */
+#define NO_CONT_MAPPINGS	(NO_PTE_CONT_MAPPINGS | NO_PMD_CONT_MAPPINGS)
+#define NO_EXEC_MAPPINGS	BIT(4)	/* assumes FEAT_HPDS is not used */
 
 u64 kimage_voffset __ro_after_init;
 EXPORT_SYMBOL(kimage_voffset);
@@ -224,7 +228,7 @@ static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
 
 		/* use a contiguous mapping if the range is suitably aligned */
 		if ((((addr | next | phys) & ~CONT_PTE_MASK) == 0) &&
-		    (flags & NO_CONT_MAPPINGS) == 0)
+		    (flags & NO_PTE_CONT_MAPPINGS) == 0)
 			__prot = __pgprot(pgprot_val(prot) | PTE_CONT);
 
 		init_pte(ptep, addr, next, phys, __prot);
@@ -254,7 +258,7 @@ static void init_pmd(pmd_t *pmdp, unsigned long addr, unsigned long end,
 
 		/* try section mapping first */
 		if (((addr | next | phys) & ~PMD_MASK) == 0 &&
-		    (flags & NO_BLOCK_MAPPINGS) == 0) {
+		    (flags & NO_PMD_BLOCK_MAPPINGS) == 0) {
 			pmd_set_huge(pmdp, phys, prot);
 
 			/*
@@ -311,7 +315,7 @@ static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
 
 		/* use a contiguous mapping if the range is suitably aligned */
 		if ((((addr | next | phys) & ~CONT_PMD_MASK) == 0) &&
-		    (flags & NO_CONT_MAPPINGS) == 0)
+		    (flags & NO_PMD_CONT_MAPPINGS) == 0)
 			__prot = __pgprot(pgprot_val(prot) | PTE_CONT);
 
 		init_pmd(pmdp, addr, next, phys, __prot, pgtable_alloc, flags);
@@ -358,8 +362,8 @@ static void alloc_init_pud(p4d_t *p4dp, unsigned long addr, unsigned long end,
 		 * For 4K granule only, attempt to put down a 1GB block
 		 */
 		if (pud_sect_supported() &&
-		   ((addr | next | phys) & ~PUD_MASK) == 0 &&
-		    (flags & NO_BLOCK_MAPPINGS) == 0) {
+		    ((addr | next | phys) & ~PUD_MASK) == 0 &&
+		    (flags & NO_PUD_BLOCK_MAPPINGS) == 0) {
 			pud_set_huge(pudp, phys, prot);
 
 			/*
@@ -1177,7 +1181,13 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
 {
 	WARN_ON((start < VMEMMAP_START) || (end > VMEMMAP_END));
 
-	if (!IS_ENABLED(CONFIG_ARM64_4K_PAGES))
+	/*
+	 * Hotplugged section does not support hugepages as
+	 * PMD_SIZE (hence PUD_SIZE) section mapping covers
+	 * struct page range that exceeds a SUBSECTION_SIZE
+	 * i.e 2MB - for all available base page sizes.
+	 */
+	if (!IS_ENABLED(CONFIG_ARM64_4K_PAGES) || system_state != SYSTEM_BOOTING)
 		return vmemmap_populate_basepages(start, end, node, altmap);
 	else
 		return vmemmap_populate_hugepages(start, end, node, altmap);
@@ -1339,9 +1349,27 @@ int arch_add_memory(int nid, u64 start, u64 size,
 		    struct mhp_params *params)
 {
 	int ret, flags = NO_EXEC_MAPPINGS;
+	unsigned long start_pfn = PFN_DOWN(start);
+	struct mem_section *ms = __pfn_to_section(start_pfn);
 
 	VM_BUG_ON(!mhp_range_allowed(start, size, true));
 
+	/* should not be invoked by early section */
+	WARN_ON(early_section(ms));
+
+	/*
+	 * Disallow BlOCK/CONT mappings if the corresponding size exceeds
+	 * SUBSECTION_SIZE which now is 2MB.
+	 *
+	 * PUD_BLOCK or PMD_CONT should consistently exceed SUBSECTION_SIZE
+	 * across all variable page size configurations, so add them directly
+	 */
+	flags |= NO_PUD_BLOCK_MAPPINGS | NO_PMD_CONT_MAPPINGS;
+	if (SUBSECTION_SHIFT < PMD_SHIFT)
+		flags |= NO_PMD_BLOCK_MAPPINGS;
+	if (SUBSECTION_SHIFT < CONT_PTE_SHIFT)
+		flags |= NO_PTE_CONT_MAPPINGS;
+
 	if (can_set_direct_map())
 		flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
 
-- 
2.25.1

Re: [PATCH v5] arm64: mm: Populate vmemmap/linear at the page level for hotplugged sections

[Zhenhua Huang]

Gentle reminder if you happened to miss it :)

On 2025/1/9 17:38, Zhenhua Huang wrote:
> On the arm64 platform with 4K base page config, SECTION_SIZE_BITS is set
> to 27, making one section 128M. The related page struct which vmemmap
> points to is 2M then.
> Commit c1cc1552616d ("arm64: MMU initialisation") optimizes the
> vmemmap to populate at the PMD section level which was suitable
> initially since hot plug granule is always one section(128M). However,
> commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug")
> introduced a 2M(SUBSECTION_SIZE) hot plug granule, which disrupted the
> existing arm64 assumptions.
> 
> Considering the vmemmap_free -> unmap_hotplug_pmd_range path, when
> pmd_sect() is true, the entire PMD section is cleared, even if there is
> other effective subsection. For example page_struct_map1 and
> page_strcut_map2 are part of a single PMD entry and they are hot-added
> sequentially. Then page_struct_map1 is removed, vmemmap_free() will clear
> the entire PMD entry freeing the struct page map for the whole section,
> even though page_struct_map2 is still active. Similar problem exists
> with linear mapping as well, for 16K base page(PMD size = 32M) or 64K
> base page(PMD = 512M), their block mappings exceed SUBSECTION_SIZE.
> Tearing down the entire PMD mapping too will leave other subsections
> unmapped in the linear mapping.
> 
> To address the issue, we need to prevent PMD/PUD/CONT mappings for both
> linear and vmemmap for non-boot sections if corresponding size on the
> given base page exceeds SUBSECTION_SIZE(2MB now).
> 
> Cc: stable@vger.kernel.org # v5.4+
> Fixes: ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug")
> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
> ---
> Hi Catalin and Anshuman,
> I have addressed comments so far, please help review.
> One outstanding point which not finalized is in vmemmap_populate(): how to judge hotplug
> section. Currently I am using system_state, discussion:
> https://lore.kernel.org/linux-mm/1515dae4-cb53-4645-8c72-d33b27ede7eb@quicinc.com/
>   arch/arm64/mm/mmu.c | 46 ++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index e2739b69e11b..8718d6e454c5 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -42,9 +42,13 @@
>   #include <asm/pgalloc.h>
>   #include <asm/kfence.h>
>   
> -#define NO_BLOCK_MAPPINGS	BIT(0)
> -#define NO_CONT_MAPPINGS	BIT(1)
> -#define NO_EXEC_MAPPINGS	BIT(2)	/* assumes FEAT_HPDS is not used */
> +#define NO_PMD_BLOCK_MAPPINGS	BIT(0)
> +#define NO_PUD_BLOCK_MAPPINGS	BIT(1)  /* Hotplug case: do not want block mapping for PUD */
> +#define NO_BLOCK_MAPPINGS	(NO_PMD_BLOCK_MAPPINGS | NO_PUD_BLOCK_MAPPINGS)
> +#define NO_PTE_CONT_MAPPINGS	BIT(2)
> +#define NO_PMD_CONT_MAPPINGS	BIT(3)  /* Hotplug case: do not want cont mapping for PMD */
> +#define NO_CONT_MAPPINGS	(NO_PTE_CONT_MAPPINGS | NO_PMD_CONT_MAPPINGS)
> +#define NO_EXEC_MAPPINGS	BIT(4)	/* assumes FEAT_HPDS is not used */
>   
>   u64 kimage_voffset __ro_after_init;
>   EXPORT_SYMBOL(kimage_voffset);
> @@ -224,7 +228,7 @@ static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr,
>   
>   		/* use a contiguous mapping if the range is suitably aligned */
>   		if ((((addr | next | phys) & ~CONT_PTE_MASK) == 0) &&
> -		    (flags & NO_CONT_MAPPINGS) == 0)
> +		    (flags & NO_PTE_CONT_MAPPINGS) == 0)
>   			__prot = __pgprot(pgprot_val(prot) | PTE_CONT);
>   
>   		init_pte(ptep, addr, next, phys, __prot);
> @@ -254,7 +258,7 @@ static void init_pmd(pmd_t *pmdp, unsigned long addr, unsigned long end,
>   
>   		/* try section mapping first */
>   		if (((addr | next | phys) & ~PMD_MASK) == 0 &&
> -		    (flags & NO_BLOCK_MAPPINGS) == 0) {
> +		    (flags & NO_PMD_BLOCK_MAPPINGS) == 0) {
>   			pmd_set_huge(pmdp, phys, prot);
>   
>   			/*
> @@ -311,7 +315,7 @@ static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr,
>   
>   		/* use a contiguous mapping if the range is suitably aligned */
>   		if ((((addr | next | phys) & ~CONT_PMD_MASK) == 0) &&
> -		    (flags & NO_CONT_MAPPINGS) == 0)
> +		    (flags & NO_PMD_CONT_MAPPINGS) == 0)
>   			__prot = __pgprot(pgprot_val(prot) | PTE_CONT);
>   
>   		init_pmd(pmdp, addr, next, phys, __prot, pgtable_alloc, flags);
> @@ -358,8 +362,8 @@ static void alloc_init_pud(p4d_t *p4dp, unsigned long addr, unsigned long end,
>   		 * For 4K granule only, attempt to put down a 1GB block
>   		 */
>   		if (pud_sect_supported() &&
> -		   ((addr | next | phys) & ~PUD_MASK) == 0 &&
> -		    (flags & NO_BLOCK_MAPPINGS) == 0) {
> +		    ((addr | next | phys) & ~PUD_MASK) == 0 &&
> +		    (flags & NO_PUD_BLOCK_MAPPINGS) == 0) {
>   			pud_set_huge(pudp, phys, prot);
>   
>   			/*
> @@ -1177,7 +1181,13 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
>   {
>   	WARN_ON((start < VMEMMAP_START) || (end > VMEMMAP_END));
>   
> -	if (!IS_ENABLED(CONFIG_ARM64_4K_PAGES))
> +	/*
> +	 * Hotplugged section does not support hugepages as
> +	 * PMD_SIZE (hence PUD_SIZE) section mapping covers
> +	 * struct page range that exceeds a SUBSECTION_SIZE
> +	 * i.e 2MB - for all available base page sizes.
> +	 */
> +	if (!IS_ENABLED(CONFIG_ARM64_4K_PAGES) || system_state != SYSTEM_BOOTING)
>   		return vmemmap_populate_basepages(start, end, node, altmap);
>   	else
>   		return vmemmap_populate_hugepages(start, end, node, altmap);
> @@ -1339,9 +1349,27 @@ int arch_add_memory(int nid, u64 start, u64 size,
>   		    struct mhp_params *params)
>   {
>   	int ret, flags = NO_EXEC_MAPPINGS;
> +	unsigned long start_pfn = PFN_DOWN(start);
> +	struct mem_section *ms = __pfn_to_section(start_pfn);
>   
>   	VM_BUG_ON(!mhp_range_allowed(start, size, true));
>   
> +	/* should not be invoked by early section */
> +	WARN_ON(early_section(ms));
> +
> +	/*
> +	 * Disallow BlOCK/CONT mappings if the corresponding size exceeds
> +	 * SUBSECTION_SIZE which now is 2MB.
> +	 *
> +	 * PUD_BLOCK or PMD_CONT should consistently exceed SUBSECTION_SIZE
> +	 * across all variable page size configurations, so add them directly
> +	 */
> +	flags |= NO_PUD_BLOCK_MAPPINGS | NO_PMD_CONT_MAPPINGS;
> +	if (SUBSECTION_SHIFT < PMD_SHIFT)
> +		flags |= NO_PMD_BLOCK_MAPPINGS;
> +	if (SUBSECTION_SHIFT < CONT_PTE_SHIFT)
> +		flags |= NO_PTE_CONT_MAPPINGS;
> +
>   	if (can_set_direct_map())
>   		flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>   

Re: [PATCH v5] arm64: mm: Populate vmemmap/linear at the page level for hotplugged sections

[Zhenhua Huang]

Dear Catalin and Anshuman,

Given that it's a genuine bug affecting stability, we need to address it 
:). Could you please let me know if you have any additional concerns or 
thoughts?

On 2025/1/15 10:13, Zhenhua Huang wrote:
> Gentle reminder if you happened to miss it :)
> 
> On 2025/1/9 17:38, Zhenhua Huang wrote:
>> On the arm64 platform with 4K base page config, SECTION_SIZE_BITS is set
>> to 27, making one section 128M. The related page struct which vmemmap
>> points to is 2M then.
>> Commit c1cc1552616d ("arm64: MMU initialisation") optimizes the
>> vmemmap to populate at the PMD section level which was suitable
>> initially since hot plug granule is always one section(128M). However,
>> commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug")
>> introduced a 2M(SUBSECTION_SIZE) hot plug granule, which disrupted the
>> existing arm64 assumptions.
>>
>> Considering the vmemmap_free -> unmap_hotplug_pmd_range path, when
>> pmd_sect() is true, the entire PMD section is cleared, even if there is
>> other effective subsection. For example page_struct_map1 and
>> page_strcut_map2 are part of a single PMD entry and they are hot-added
>> sequentially. Then page_struct_map1 is removed, vmemmap_free() will clear
>> the entire PMD entry freeing the struct page map for the whole section,
>> even though page_struct_map2 is still active. Similar problem exists
>> with linear mapping as well, for 16K base page(PMD size = 32M) or 64K
>> base page(PMD = 512M), their block mappings exceed SUBSECTION_SIZE.
>> Tearing down the entire PMD mapping too will leave other subsections
>> unmapped in the linear mapping.
>>
>> To address the issue, we need to prevent PMD/PUD/CONT mappings for both
>> linear and vmemmap for non-boot sections if corresponding size on the
>> given base page exceeds SUBSECTION_SIZE(2MB now).
>>
>> Cc: stable@vger.kernel.org # v5.4+
>> Fixes: ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug")
>> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
>> ---
>> Hi Catalin and Anshuman,
>> I have addressed comments so far, please help review.
>> One outstanding point which not finalized is in vmemmap_populate(): 
>> how to judge hotplug
>> section. Currently I am using system_state, discussion:
>> https://lore.kernel.org/linux-mm/1515dae4-cb53-4645-8c72- 
>> d33b27ede7eb@quicinc.com/
>>   arch/arm64/mm/mmu.c | 46 ++++++++++++++++++++++++++++++++++++---------
>>   1 file changed, 37 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index e2739b69e11b..8718d6e454c5 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -42,9 +42,13 @@
>>   #include <asm/pgalloc.h>
>>   #include <asm/kfence.h>
>> -#define NO_BLOCK_MAPPINGS    BIT(0)
>> -#define NO_CONT_MAPPINGS    BIT(1)
>> -#define NO_EXEC_MAPPINGS    BIT(2)    /* assumes FEAT_HPDS is not 
>> used */
>> +#define NO_PMD_BLOCK_MAPPINGS    BIT(0)
>> +#define NO_PUD_BLOCK_MAPPINGS    BIT(1)  /* Hotplug case: do not want 
>> block mapping for PUD */
>> +#define NO_BLOCK_MAPPINGS    (NO_PMD_BLOCK_MAPPINGS | 
>> NO_PUD_BLOCK_MAPPINGS)
>> +#define NO_PTE_CONT_MAPPINGS    BIT(2)
>> +#define NO_PMD_CONT_MAPPINGS    BIT(3)  /* Hotplug case: do not want 
>> cont mapping for PMD */
>> +#define NO_CONT_MAPPINGS    (NO_PTE_CONT_MAPPINGS | 
>> NO_PMD_CONT_MAPPINGS)
>> +#define NO_EXEC_MAPPINGS    BIT(4)    /* assumes FEAT_HPDS is not 
>> used */
>>   u64 kimage_voffset __ro_after_init;
>>   EXPORT_SYMBOL(kimage_voffset);
>> @@ -224,7 +228,7 @@ static void alloc_init_cont_pte(pmd_t *pmdp, 
>> unsigned long addr,
>>           /* use a contiguous mapping if the range is suitably aligned */
>>           if ((((addr | next | phys) & ~CONT_PTE_MASK) == 0) &&
>> -            (flags & NO_CONT_MAPPINGS) == 0)
>> +            (flags & NO_PTE_CONT_MAPPINGS) == 0)
>>               __prot = __pgprot(pgprot_val(prot) | PTE_CONT);
>>           init_pte(ptep, addr, next, phys, __prot);
>> @@ -254,7 +258,7 @@ static void init_pmd(pmd_t *pmdp, unsigned long 
>> addr, unsigned long end,
>>           /* try section mapping first */
>>           if (((addr | next | phys) & ~PMD_MASK) == 0 &&
>> -            (flags & NO_BLOCK_MAPPINGS) == 0) {
>> +            (flags & NO_PMD_BLOCK_MAPPINGS) == 0) {
>>               pmd_set_huge(pmdp, phys, prot);
>>               /*
>> @@ -311,7 +315,7 @@ static void alloc_init_cont_pmd(pud_t *pudp, 
>> unsigned long addr,
>>           /* use a contiguous mapping if the range is suitably aligned */
>>           if ((((addr | next | phys) & ~CONT_PMD_MASK) == 0) &&
>> -            (flags & NO_CONT_MAPPINGS) == 0)
>> +            (flags & NO_PMD_CONT_MAPPINGS) == 0)
>>               __prot = __pgprot(pgprot_val(prot) | PTE_CONT);
>>           init_pmd(pmdp, addr, next, phys, __prot, pgtable_alloc, flags);
>> @@ -358,8 +362,8 @@ static void alloc_init_pud(p4d_t *p4dp, unsigned 
>> long addr, unsigned long end,
>>            * For 4K granule only, attempt to put down a 1GB block
>>            */
>>           if (pud_sect_supported() &&
>> -           ((addr | next | phys) & ~PUD_MASK) == 0 &&
>> -            (flags & NO_BLOCK_MAPPINGS) == 0) {
>> +            ((addr | next | phys) & ~PUD_MASK) == 0 &&
>> +            (flags & NO_PUD_BLOCK_MAPPINGS) == 0) {
>>               pud_set_huge(pudp, phys, prot);
>>               /*
>> @@ -1177,7 +1181,13 @@ int __meminit vmemmap_populate(unsigned long 
>> start, unsigned long end, int node,
>>   {
>>       WARN_ON((start < VMEMMAP_START) || (end > VMEMMAP_END));
>> -    if (!IS_ENABLED(CONFIG_ARM64_4K_PAGES))
>> +    /*
>> +     * Hotplugged section does not support hugepages as
>> +     * PMD_SIZE (hence PUD_SIZE) section mapping covers
>> +     * struct page range that exceeds a SUBSECTION_SIZE
>> +     * i.e 2MB - for all available base page sizes.
>> +     */
>> +    if (!IS_ENABLED(CONFIG_ARM64_4K_PAGES) || system_state != 
>> SYSTEM_BOOTING)
>>           return vmemmap_populate_basepages(start, end, node, altmap);
>>       else
>>           return vmemmap_populate_hugepages(start, end, node, altmap);
>> @@ -1339,9 +1349,27 @@ int arch_add_memory(int nid, u64 start, u64 size,
>>               struct mhp_params *params)
>>   {
>>       int ret, flags = NO_EXEC_MAPPINGS;
>> +    unsigned long start_pfn = PFN_DOWN(start);
>> +    struct mem_section *ms = __pfn_to_section(start_pfn);
>>       VM_BUG_ON(!mhp_range_allowed(start, size, true));
>> +    /* should not be invoked by early section */
>> +    WARN_ON(early_section(ms));
>> +
>> +    /*
>> +     * Disallow BlOCK/CONT mappings if the corresponding size exceeds
>> +     * SUBSECTION_SIZE which now is 2MB.
>> +     *
>> +     * PUD_BLOCK or PMD_CONT should consistently exceed SUBSECTION_SIZE
>> +     * across all variable page size configurations, so add them 
>> directly
>> +     */
>> +    flags |= NO_PUD_BLOCK_MAPPINGS | NO_PMD_CONT_MAPPINGS;
>> +    if (SUBSECTION_SHIFT < PMD_SHIFT)
>> +        flags |= NO_PMD_BLOCK_MAPPINGS;
>> +    if (SUBSECTION_SHIFT < CONT_PTE_SHIFT)
>> +        flags |= NO_PTE_CONT_MAPPINGS;
>> +
>>       if (can_set_direct_map())
>>           flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
> 
> 

Re: [PATCH v5] arm64: mm: Populate vmemmap/linear at the page level for hotplugged sections

[Catalin Marinas]

On Thu, Jan 09, 2025 at 05:38:24PM +0800, Zhenhua Huang wrote:
> On the arm64 platform with 4K base page config, SECTION_SIZE_BITS is set
> to 27, making one section 128M. The related page struct which vmemmap
> points to is 2M then.
> Commit c1cc1552616d ("arm64: MMU initialisation") optimizes the
> vmemmap to populate at the PMD section level which was suitable
> initially since hot plug granule is always one section(128M). However,
> commit ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug")
> introduced a 2M(SUBSECTION_SIZE) hot plug granule, which disrupted the
> existing arm64 assumptions.
> 
> Considering the vmemmap_free -> unmap_hotplug_pmd_range path, when
> pmd_sect() is true, the entire PMD section is cleared, even if there is
> other effective subsection. For example page_struct_map1 and
> page_strcut_map2 are part of a single PMD entry and they are hot-added
> sequentially. Then page_struct_map1 is removed, vmemmap_free() will clear
> the entire PMD entry freeing the struct page map for the whole section,
> even though page_struct_map2 is still active. Similar problem exists
> with linear mapping as well, for 16K base page(PMD size = 32M) or 64K
> base page(PMD = 512M), their block mappings exceed SUBSECTION_SIZE.
> Tearing down the entire PMD mapping too will leave other subsections
> unmapped in the linear mapping.
> 
> To address the issue, we need to prevent PMD/PUD/CONT mappings for both
> linear and vmemmap for non-boot sections if corresponding size on the
> given base page exceeds SUBSECTION_SIZE(2MB now).
> 
> Cc: stable@vger.kernel.org # v5.4+
> Fixes: ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug")
> Signed-off-by: Zhenhua Huang <quic_zhenhuah@quicinc.com>
> ---
> Hi Catalin and Anshuman,
> I have addressed comments so far, please help review.
> One outstanding point which not finalized is in vmemmap_populate(): how to judge hotplug
> section. Currently I am using system_state, discussion:
> https://lore.kernel.org/linux-mm/1515dae4-cb53-4645-8c72-d33b27ede7eb@quicinc.com/

The patch looks fine to me, apart from one nit and a question below:

> @@ -1339,9 +1349,27 @@ int arch_add_memory(int nid, u64 start, u64 size,
>  		    struct mhp_params *params)
>  {
>  	int ret, flags = NO_EXEC_MAPPINGS;
> +	unsigned long start_pfn = PFN_DOWN(start);
> +	struct mem_section *ms = __pfn_to_section(start_pfn);
>  
>  	VM_BUG_ON(!mhp_range_allowed(start, size, true));
>  
> +	/* should not be invoked by early section */
> +	WARN_ON(early_section(ms));

I don't remember the discussion, do we still need this warning here if
the sections are not marked as early? I guess we can keep it if one does
an arch_add_memory() on an early section.

I think I suggested to use a WARN_ON_ONCE(!present_section()) but I
completely forgot the memory hotplug code paths.

> +
> +	/*
> +	 * Disallow BlOCK/CONT mappings if the corresponding size exceeds

Nit: capital L in BlOCK.

Either way,

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v5] arm64: mm: Populate vmemmap/linear at the page level for hotplugged sections

[Zhenhua Huang]

On 2025/2/13 2:28, Catalin Marinas wrote:
>> @@ -1339,9 +1349,27 @@ int arch_add_memory(int nid, u64 start, u64 size,
>>   		    struct mhp_params *params)
>>   {
>>   	int ret, flags = NO_EXEC_MAPPINGS;
>> +	unsigned long start_pfn = PFN_DOWN(start);
>> +	struct mem_section *ms = __pfn_to_section(start_pfn);
>>   
>>   	VM_BUG_ON(!mhp_range_allowed(start, size, true));
>>   
>> +	/* should not be invoked by early section */
>> +	WARN_ON(early_section(ms));
> I don't remember the discussion, do we still need this warning here if
> the sections are not marked as early? I guess we can keep it if one does
> an arch_add_memory() on an early section.
> 
> I think I suggested to use a WARN_ON_ONCE(!present_section()) but I
> completely forgot the memory hotplug code paths.

Dear Catalin,

The previous discussion can be found at 
https://lore.kernel.org/lkml/aedbbc4f-8f6c-46d8-a8d7-53103675a816@quicinc.com/, 
I highlighted the key points from conversation between me and Anshuman 
for your reference:
"
 >>
 >> BTW, shall we remove the check for !early_section since 
arch_add_memory is only called during hotplugging case? Correct me 
please if I'm mistaken :)
 >
 > While this is true, still might be a good idea to keep the 
early_section()
 > check in place just to be extra careful here. Otherwise an WARN_ON() 
might
 > be needed.

Make sense. I would like to add some comments and WARN_ON() if
early_section().
"
Regarding your suggestion, I believed it was intended for the 
vmemmap_populate() function ?(Discussion: 
https://lore.kernel.org/linux-mm/Z3_d59kp4CuHQp97@arm.com/), but as 
workflow below indicates:
Hot plug:
1. section_activate -> vmemmap_populate
2. mark PRESENT

In contrast, the early flow:
1. memblocks_present -> mark PRESENT
2. __populate_section_memmap -> vmemmap_populate

Could this result in a false warning during hotplugging? I replied with 
the doubt in above link before but seems you missed :) Could you please 
share your thoughts if you have a different idea ?

I will include your tags, correct capitalization nit and post one new 
version.