Skip to contentSkip to conversation

ARC: Use max_high_pfn as a HIGHMEM zone border

PATCH Superseded Vladimir Isaev 3 participants 5 messages Apr 26, 2021

Subsystems (3)
Memory management Generic include/asm header files Synopsys ARC architecture

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Vladimir Isaev

author
#0 lore raw

PATCH 1/1 ARC: Use max_high_pfn as a HIGHMEM zone border

Commit 4af22ded0ecf ("arc: fix memory initialization for systems with two memory banks") fixed highmem, but not for PAE case when highmem is actually bigger than lowmem.

Signed-off-by Vladimir Isaev <[email protected]>
Cc Mike Rapoport <[email protected]>
---
 arch/arc/mm/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arc/mm/init.c +1 -1
index ce07e697916c..59bad6f94105 100644
@@ -157,7 +157,7 @@ void __init setup_arch_memory(void)
 	min_high_pfn = PFN_DOWN(high_mem_start);
 	max_high_pfn = PFN_DOWN(high_mem_start + high_mem_sz);
 
-	max_zone_pfn[ZONE_HIGHMEM] = min_low_pfn;
+	max_zone_pfn[ZONE_HIGHMEM] = max_high_pfn;
 
 	high_memory = (void *)(min_high_pfn << PAGE_SHIFT);
Signature · 1 line
2.16.2
#1 lore raw

Hi,

Quoted 15 lines · Vladimir Isaev

Commit 4af22ded0ecf ("arc: fix memory initialization for systems with two memory banks") fixed highmem, but not for PAE case when highmem is actually bigger than lowmem.

Signed-off-by Vladimir Isaev <[email protected]>
Cc Mike Rapoport <[email protected]>
---
 arch/arc/mm/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arc/mm/init.c +0 -0
index ce07e697916c..59bad6f94105 100644

arch/arc/mm/init.c void __init setup_arch_memory(void) · line 160

Quoted 5 lines
arch/arc/mm/init.c +1 -1
@@ -157,7 +157,7 @@ void __init setup_arch_memory(void)
 	min_high_pfn = PFN_DOWN(high_mem_start);
 	max_high_pfn = PFN_DOWN(high_mem_start + high_mem_sz);
 
-	max_zone_pfn[ZONE_HIGHMEM] = min_low_pfn;
+	max_zone_pfn[ZONE_HIGHMEM] = max_high_pfn;

This is correct with PAE40, but it will break !PAE40 when "highmem" has lower addresses than lowmem.

It rather should be something like:

        if (IS_ENABLED(CONFIG_ARC_HAS_PAE40))
                max_zone_pfn[ZONE_HIGHMEM] = max_high_pfn;
        else
            	max_zone_pfn[ZONE_HIGHMEM] = min_low_pfn;
Quoted 6 lines
arch/arc/mm/init.c +0 -0
 	high_memory = (void *)(min_high_pfn << PAGE_SHIFT);
Signature · 1 line
2.16.2
Signature · 2 lines
Sincerely yours,
Mike.
author
#2 lore raw

Hi Mike,

Quoted 7 lines · Mike Rapoport

arch/arc/mm/init.c void __init setup_arch_memory(void) · line 160

Quoted 2 lines · Vladimir Isaev
arch/arc/mm/init.c +1 -1
-	max_zone_pfn[ZONE_HIGHMEM] = min_low_pfn;
+	max_zone_pfn[ZONE_HIGHMEM] = max_high_pfn;

This is correct with PAE40, but it will break !PAE40 when "highmem" has lower addresses than lowmem.

Quoted 7 lines

It rather should be something like:

        if (IS_ENABLED(CONFIG_ARC_HAS_PAE40))
                max_zone_pfn[ZONE_HIGHMEM] = max_high_pfn;
        else
            	max_zone_pfn[ZONE_HIGHMEM] = min_low_pfn;

Not sure if I understand why we should have min_low_pfn here. In !PAE40 case max_high_pfn just will be smaller than min_low_pfn.

Thank you,
Vladimir Isaev

#3 lore raw
Quoted 12 lines · Vladimir Isaev

Hi Mike,

Quoted 9 lines · Mike Rapoport

arch/arc/mm/init.c void __init setup_arch_memory(void) · line 160

Quoted 2 lines · Vladimir Isaev
arch/arc/mm/init.c +1 -1
-	max_zone_pfn[ZONE_HIGHMEM] = min_low_pfn;
+	max_zone_pfn[ZONE_HIGHMEM] = max_high_pfn;

This is correct with PAE40, but it will break !PAE40 when "highmem" has lower addresses than lowmem.

It rather should be something like:

Quoted 8 lines
Quoted 5 lines
        if (IS_ENABLED(CONFIG_ARC_HAS_PAE40))
                max_zone_pfn[ZONE_HIGHMEM] = max_high_pfn;
        else
            	max_zone_pfn[ZONE_HIGHMEM] = min_low_pfn;

Not sure if I understand why we should have min_low_pfn here. In !PAE40 case max_high_pfn just will be smaller than min_low_pfn.

Hmm, actually, you are right. This should be fine.

Signature · 2 lines
Sincerely yours,
Mike.
#4 lore raw
Quoted 12 lines · Mike Rapoport
Quoted 11 lines · Vladimir Isaev

Hi Mike,

Quoted 8 lines · Mike Rapoport

arch/arc/mm/init.c void __init setup_arch_memory(void) · line 160

Quoted 2 lines · Vladimir Isaev
arch/arc/mm/init.c +1 -1
-	max_zone_pfn[ZONE_HIGHMEM] = min_low_pfn;
+	max_zone_pfn[ZONE_HIGHMEM] = max_high_pfn;

This is correct with PAE40, but it will break !PAE40 when "highmem" has lower addresses than lowmem.

It rather should be something like:

Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
         if (IS_ENABLED(CONFIG_ARC_HAS_PAE40))
                 max_zone_pfn[ZONE_HIGHMEM] = max_high_pfn;
         else
             	max_zone_pfn[ZONE_HIGHMEM] = min_low_pfn;

Not sure if I understand why we should have min_low_pfn here. In !PAE40 case max_high_pfn just will be smaller than min_low_pfn.

Hmm, actually, you are right. This should be fine.

But still worth adding a comment. If this could trip the person who did the massive cross-arch rework then I don't know what mere mortals would run into ;-)

-Vineet