Hi folks,
Enabling SMP for sparc32 uncovered some issues in the SRMMU page-table
allocation code. One of these was introduced by me, but the other two
seem to have been there a while and are probably just exposed more
easily by my recent changes.
Tested on QEMU. I'm assuming these will go via David's tree.
Cheers,
Will
--->8
Cc: Peter Zijlstra <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: [email protected]
Will Deacon (3):
sparc32: mm: Don't try to free page-table pages if ctor() fails
sparc32: mm: Disable SPLIT_PTLOCK_CPUS
sparc32: mm: Only call ctor()/dtor() functions for first and last user
arch/sparc/mm/srmmu.c | 15 +++++++++++----
mm/Kconfig | 4 ++++
2 files changed, 15 insertions(+), 4 deletions(-)
--
2.27.0.rc0.183.gde8f92d652-goog
The pages backing page-table allocations for SRMMU are allocated via
memblock as part of the "nocache" region initialisation during
srmmu_paging_init() and should not be freed even if a later call to
pgtable_pte_page_ctor() fails.
Remove the broken call to __free_page().
Cc: David S. Miller <[email protected]>
Cc: Kirill A. Shutemov <[email protected]>
Fixes: 1ae9ae5f7df7 ("sparc: handle pgtable_page_ctor() fail")
Signed-off-by: Will Deacon <[email protected]>
---
arch/sparc/mm/srmmu.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/sparc/mm/srmmu.c b/arch/sparc/mm/srmmu.c
index c861c0f0df73..589370a21b12 100644
--- a/arch/sparc/mm/srmmu.c
+++ b/arch/sparc/mm/srmmu.c
@@ -365,7 +365,6 @@ pgtable_t pte_alloc_one(struct mm_struct *mm)
return NULL;
page = pfn_to_page(__nocache_pa((unsigned long)ptep) >> PAGE_SHIFT);
if (!pgtable_pte_page_ctor(page)) {
- __free_page(page);
return NULL;
}
return ptep;
--
2.27.0.rc0.183.gde8f92d652-goog
The SRMMU page-table allocator is not compatible with SPLIT_PTLOCK_CPUS
for two major reasons:
1. Pages are allocated via memblock, and therefore the ptl is not
cleared by prep_new_page(), which is expected by ptlock_init()
2. Multiple PTE tables can exist in a single page, causing them to
share the same ptl and deadlock when attempting to take the same
lock twice (e.g. as part of copy_page_range()).
Ensure that SPLIT_PTLOCK_CPUS is not selected for SPARC32.
Cc: David S. Miller <[email protected]>
Signed-off-by: Will Deacon <[email protected]>
---
mm/Kconfig | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/Kconfig b/mm/Kconfig
index c1acc34c1c35..97458119cce8 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -192,6 +192,9 @@ config MEMORY_HOTREMOVE
# Default to 4 for wider testing, though 8 might be more appropriate.
# ARM's adjust_pte (unused if VIPT) depends on mm-wide page_table_lock.
# PA-RISC 7xxx's spinlock_t would enlarge struct page from 32 to 44 bytes.
+# SPARC32 allocates multiple pte tables within a single page, and therefore
+# a per-page lock leads to problems when multiple tables need to be locked
+# at the same time (e.g. copy_page_range()).
# DEBUG_SPINLOCK and DEBUG_LOCK_ALLOC spinlock_t also enlarge struct page.
#
config SPLIT_PTLOCK_CPUS
@@ -199,6 +202,7 @@ config SPLIT_PTLOCK_CPUS
default "999999" if !MMU
default "999999" if ARM && !CPU_CACHE_VIPT
default "999999" if PARISC && !PA20
+ default "999999" if SPARC32
default "4"
config ARCH_ENABLE_SPLIT_PMD_PTLOCK
--
2.27.0.rc0.183.gde8f92d652-goog
The SRMMU page-table allocator allocates multiple PTE tables per page,
since they are only 1K in size. However, this means that calls to
pgtable_pte_page_{ctor,dtor}() must be serialised and performed only by
the first and last page-table allocation for the page respectively.
Use the page reference count to track how many PTE tables we have
allocated for a given page returned by the SRMMU allocator and only
call the ctor()/dtor() functions for the first and last user respectively.
Cc: David S. Miller <[email protected]>
Fixes: 8c8f3156dd40 ("sparc32: mm: Reduce allocation size for PMD and PTE tables")
Signed-off-by: Will Deacon <[email protected]>
---
arch/sparc/mm/srmmu.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/arch/sparc/mm/srmmu.c b/arch/sparc/mm/srmmu.c
index 589370a21b12..116d19a390f2 100644
--- a/arch/sparc/mm/srmmu.c
+++ b/arch/sparc/mm/srmmu.c
@@ -364,9 +364,13 @@ pgtable_t pte_alloc_one(struct mm_struct *mm)
if ((ptep = pte_alloc_one_kernel(mm)) == 0)
return NULL;
page = pfn_to_page(__nocache_pa((unsigned long)ptep) >> PAGE_SHIFT);
- if (!pgtable_pte_page_ctor(page)) {
- return NULL;
+ spin_lock(&mm->page_table_lock);
+ if (page_ref_inc_return(page) == 2 && !pgtable_pte_page_ctor(page)) {
+ page_ref_dec(page);
+ ptep = NULL;
}
+ spin_unlock(&mm->page_table_lock);
+
return ptep;
}
@@ -375,7 +379,11 @@ void pte_free(struct mm_struct *mm, pgtable_t ptep)
struct page *page;
page = pfn_to_page(__nocache_pa((unsigned long)ptep) >> PAGE_SHIFT);
- pgtable_pte_page_dtor(page);
+ spin_lock(&mm->page_table_lock);
+ if (page_ref_dec_return(page) == 1)
+ pgtable_pte_page_dtor(page);
+ spin_unlock(&mm->page_table_lock);
+
srmmu_free_nocache(ptep, SRMMU_PTE_TABLE_SIZE);
}
--
2.27.0.rc0.183.gde8f92d652-goog
From: Will Deacon <[email protected]>
Date: Tue, 26 May 2020 18:32:59 +0100
> Hi folks,
>
> Enabling SMP for sparc32 uncovered some issues in the SRMMU page-table
> allocation code. One of these was introduced by me, but the other two
> seem to have been there a while and are probably just exposed more
> easily by my recent changes.
>
> Tested on QEMU. I'm assuming these will go via David's tree.
Series applied, thanks Will.