[PATCH 0/6] arm64/mm: Enable accounting for page table pages

[PATCH 0/6] arm64/mm: Enable accounting for page table pages

[Anshuman Khandual]

Currently page table page allocation and release has multiple combinations
for user space and kernel mapping page tables. Not only this is vulnerable
for bad page state errors it also prevents accurate accounting for those
pages through NR_PAGETABLE. This patch series attempts to standardize all
(except PGD slab cache 'pdg_cache') page table page allocation and release
through pte_alloc_one()/pte_free() which in turn guarantees that all these
pages will go trough pgtable_page_[ctor|dtor] constructs. It also takes
care of some of the pages who might have been allocated early via memblock.

This enables memcg based accounting for user process page table pages via
__GFP_ACCOUNT. For now this also removes standard ptable page allocators
usage from KVM stage-2 as some of them use multi order allocation which is
not supported in a generic way.

Anshuman Khandual (6):
  KVM: ARM: Remove pgtable standard functions from stage-2 page tables
  arm64/mm: Ensure PGD kmem cache object is actually allocated
  arm64/mm: Make pgd_pgtable_alloc() call pte_alloc_one() always
  arm64/mm: Make all page table pages cycles through standard constructs
  arm64/mm: Call pgtable_page_dtor() for both PMD and PUD page table pages
  arm64/mm: Enable page table page accounting for user space

 arch/arm/include/asm/stage2_pgtable.h   |  4 +--
 arch/arm64/include/asm/pgalloc.h        | 54 +++++++++++++++++++++++++++------
 arch/arm64/include/asm/stage2_pgtable.h |  4 +--
 arch/arm64/include/asm/tlb.h            |  2 ++
 arch/arm64/mm/mmu.c                     | 17 ++++++++---
 arch/arm64/mm/pgd.c                     |  5 +--
 6 files changed, 67 insertions(+), 19 deletions(-)

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/6] KVM: ARM: Remove pgtable standard functions from stage-2 page tables

[Anshuman Khandual]

ARM64 standard pgtable functions are going to use pgtable_page_[ctor|dtor]
constructs. Certain stage-2 page table allocations are multi order which
cannot be allocated through a generic pgtable function as it does not exist
right now. This prevents all pgtable allocations including multi order ones
in stage-2 from moving into new ARM64 (pgtable_page_[ctor|dtor]) pgtable
functions. Hence remove all generic pgtable allocation function dependency
from stage-2 page tables till there is one multi-order allocation function
available.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm/include/asm/stage2_pgtable.h   | 4 ++--
 arch/arm64/include/asm/stage2_pgtable.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/stage2_pgtable.h b/arch/arm/include/asm/stage2_pgtable.h
index c4b1d4f..0895e0b 100644
--- a/arch/arm/include/asm/stage2_pgtable.h
+++ b/arch/arm/include/asm/stage2_pgtable.h
@@ -32,14 +32,14 @@
 #define stage2_pgd_present(kvm, pgd)		pgd_present(pgd)
 #define stage2_pgd_populate(kvm, pgd, pud)	pgd_populate(NULL, pgd, pud)
 #define stage2_pud_offset(kvm, pgd, address)	pud_offset(pgd, address)
-#define stage2_pud_free(kvm, pud)		pud_free(NULL, pud)
+#define stage2_pud_free(kvm, pud)		free_page((unsigned long)pud)
 
 #define stage2_pud_none(kvm, pud)		pud_none(pud)
 #define stage2_pud_clear(kvm, pud)		pud_clear(pud)
 #define stage2_pud_present(kvm, pud)		pud_present(pud)
 #define stage2_pud_populate(kvm, pud, pmd)	pud_populate(NULL, pud, pmd)
 #define stage2_pmd_offset(kvm, pud, address)	pmd_offset(pud, address)
-#define stage2_pmd_free(kvm, pmd)		pmd_free(NULL, pmd)
+#define stage2_pmd_free(kvm, pmd)		free_page((unsigned long)pmd)
 
 #define stage2_pud_huge(kvm, pud)		pud_huge(pud)
 
diff --git a/arch/arm64/include/asm/stage2_pgtable.h b/arch/arm64/include/asm/stage2_pgtable.h
index 5412fa4..915809e 100644
--- a/arch/arm64/include/asm/stage2_pgtable.h
+++ b/arch/arm64/include/asm/stage2_pgtable.h
@@ -119,7 +119,7 @@ static inline pud_t *stage2_pud_offset(struct kvm *kvm,
 static inline void stage2_pud_free(struct kvm *kvm, pud_t *pud)
 {
 	if (kvm_stage2_has_pud(kvm))
-		pud_free(NULL, pud);
+		free_page((unsigned long)pud);
 }
 
 static inline bool stage2_pud_table_empty(struct kvm *kvm, pud_t *pudp)
@@ -192,7 +192,7 @@ static inline pmd_t *stage2_pmd_offset(struct kvm *kvm,
 static inline void stage2_pmd_free(struct kvm *kvm, pmd_t *pmd)
 {
 	if (kvm_stage2_has_pmd(kvm))
-		pmd_free(NULL, pmd);
+		free_page((unsigned long)pmd);
 }
 
 static inline bool stage2_pud_huge(struct kvm *kvm, pud_t pud)
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/6] arm64/mm: Ensure PGD kmem cache object is actually allocated

[Anshuman Khandual]

'pgd_cache' is created for allocating non PAGE_SIZE based PGD pointers. In
such cases ensure that the kmem cache object is allocated before proceeding
further with boot.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/mm/pgd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/mm/pgd.c b/arch/arm64/mm/pgd.c
index 289f911..8cb4e39 100644
--- a/arch/arm64/mm/pgd.c
+++ b/arch/arm64/mm/pgd.c
@@ -62,4 +62,5 @@ void __init pgd_cache_init(void)
 	 */
 	pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, PGD_SIZE,
 				      SLAB_PANIC, NULL);
+	BUG_ON(!pgd_cache);
 }
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/6] arm64/mm: Make pgd_pgtable_alloc() call pte_alloc_one() always

[Anshuman Khandual]

pgd_pgtable_alloc() provides page allocation function while creating all
levels of page table (e.g PGD, PUD, CONT PMD) for various kernel mappings.
It calls __get_free_page() and initializes page with pagetable_page_ctor().
pte_alloc_one() already provides a standard interface for allocating a page
table page and initializes it with pagetable_page_ctor(). This removes the
redundancy and instead make it call pte_alloc_one() directly.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/mm/mmu.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index b6f5aa5..2dbd723 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -372,13 +372,22 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
 
 static phys_addr_t pgd_pgtable_alloc(void)
 {
-	void *ptr = (void *)__get_free_page(PGALLOC_GFP);
-	if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
-		BUG();
+
+	pgtable_t ptr;
+
+	/*
+	 * pgd_pgtable_alloc() is called while creating kernel mappings
+	 * through __create_pgd_mapping() might not install it through
+	 * swapper_pg_dir (&init_mm). Even then init_mm is passed here
+	 * just to indicate that the allocation is kernel specific not
+	 * for the user space page tables.
+	 */
+	ptr = pte_alloc_one(&init_mm);
+	BUG_ON(!ptr);
 
 	/* Ensure the zeroed page is visible to the page table walker */
 	dsb(ishst);
-	return __pa(ptr);
+	return page_to_phys(ptr);
 }
 
 /*
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/6] arm64/mm: Make all page table pages cycles through standard constructs

[Anshuman Khandual]

Currently there are inconsistencies during page table page's allocation,
initialization and destruction. Not all those pages go through standard
constructors pgtable_page_ctor/pgtable_page_dtor which is very much
essential for proper initialization and accounting. Not going through
proper pgtable_page_ctor/pgtable_page_dtor cycle makes them vulnerable
for bad page state errors during page table tear down process.

This makes all levels of page table pages either for the user space or
for the kernel mappings go through these standard constructs. While
here pte_free() has been modified to accommodate pages which might not
have been allocated through buddy system but instead came from memblock
[1] directly during early boot process. Those pages must not go through
the pgtable_page_dtor() and instead are freed with free_reserved_page().

PGD based page table page which is allocated from 'pgd_cache' kmem cache
in certain situations (PGD_SIZE != PAGE_SIZE ) is excluded from being
cycled through pgtable_page_[ctor/dtor]. This is not a problem because
this slab never gets freed back to buddy allocator.

All stage-2 page table pages must cycle through pgtable_page_[ctor/dtor]
constructs for proper initialization and accounting. Use the standard
pte_alloc_one() and pte_free() for their allocation and release instead
of calling into the buddy allocator directly.

[1] Page table page allocation from memblock

(a) early_pgtable_alloc (passed with __create_pgd_mapping)
(b) vmemmap_pgd|pud_populate (vmemmap_populate)

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/include/asm/pgalloc.h | 42 ++++++++++++++++++++++++++++++++--------
 arch/arm64/mm/pgd.c              |  4 ++--
 2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
index 52fa47c..076764f 100644
--- a/arch/arm64/include/asm/pgalloc.h
+++ b/arch/arm64/include/asm/pgalloc.h
@@ -29,17 +29,21 @@
 #define PGALLOC_GFP	(GFP_KERNEL | __GFP_ZERO)
 #define PGD_SIZE	(PTRS_PER_PGD * sizeof(pgd_t))
 
+static inline pgtable_t pte_alloc_one(struct mm_struct *mm);
+static inline unsigned long pte_alloc_one_virt(struct mm_struct *mm);
+static inline void pte_free(struct mm_struct *mm, pgtable_t pte);
+
 #if CONFIG_PGTABLE_LEVELS > 2
 
 static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
 {
-	return (pmd_t *)__get_free_page(PGALLOC_GFP);
+	return (pmd_t *)pte_alloc_one_virt(mm);
 }
 
 static inline void pmd_free(struct mm_struct *mm, pmd_t *pmdp)
 {
 	BUG_ON((unsigned long)pmdp & (PAGE_SIZE-1));
-	free_page((unsigned long)pmdp);
+	pte_free(mm, virt_to_page(pmdp));
 }
 
 static inline void __pud_populate(pud_t *pudp, phys_addr_t pmdp, pudval_t prot)
@@ -62,13 +66,13 @@ static inline void __pud_populate(pud_t *pudp, phys_addr_t pmdp, pudval_t prot)
 
 static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
 {
-	return (pud_t *)__get_free_page(PGALLOC_GFP);
+	return (pud_t *)pte_alloc_one_virt(mm);
 }
 
 static inline void pud_free(struct mm_struct *mm, pud_t *pudp)
 {
 	BUG_ON((unsigned long)pudp & (PAGE_SIZE-1));
-	free_page((unsigned long)pudp);
+	pte_free(mm, virt_to_page(pudp));
 }
 
 static inline void __pgd_populate(pgd_t *pgdp, phys_addr_t pudp, pgdval_t prot)
@@ -93,7 +97,7 @@ extern void pgd_free(struct mm_struct *mm, pgd_t *pgdp);
 static inline pte_t *
 pte_alloc_one_kernel(struct mm_struct *mm)
 {
-	return (pte_t *)__get_free_page(PGALLOC_GFP);
+	return (pte_t *)pte_alloc_one_virt(mm);
 }
 
 static inline pgtable_t
@@ -111,19 +115,41 @@ pte_alloc_one(struct mm_struct *mm)
 	return pte;
 }
 
+static inline unsigned long
+pte_alloc_one_virt(struct mm_struct *mm)
+{
+	pgtable_t ptr;
+
+	ptr = pte_alloc_one(mm);
+	if (!ptr)
+		return 0;
+
+	return (unsigned long) page_to_virt(ptr);
+}
+
 /*
  * Free a PTE table.
  */
 static inline void pte_free_kernel(struct mm_struct *mm, pte_t *ptep)
 {
 	if (ptep)
-		free_page((unsigned long)ptep);
+		pte_free(mm, virt_to_page(ptep));
 }
 
 static inline void pte_free(struct mm_struct *mm, pgtable_t pte)
 {
-	pgtable_page_dtor(pte);
-	__free_page(pte);
+	/*
+	 * Some page table pages might have come from memblock either
+	 * with vmemmap_alloc_block (during vmemmap_populate) or with
+	 * early_pgtable_alloc (during __create_pgd_mapping). These
+	 * pages should be freed with free_reserved_page() instead.
+	 */
+	if (PageReserved(pte)) {
+		free_reserved_page(pte);
+	} else {
+		pgtable_page_dtor(pte);
+		__free_page(pte);
+	}
 }
 
 static inline void __pmd_populate(pmd_t *pmdp, phys_addr_t ptep,
diff --git a/arch/arm64/mm/pgd.c b/arch/arm64/mm/pgd.c
index 8cb4e39..e344075 100644
--- a/arch/arm64/mm/pgd.c
+++ b/arch/arm64/mm/pgd.c
@@ -31,7 +31,7 @@ static struct kmem_cache *pgd_cache __ro_after_init;
 pgd_t *pgd_alloc(struct mm_struct *mm)
 {
 	if (PGD_SIZE == PAGE_SIZE)
-		return (pgd_t *)__get_free_page(PGALLOC_GFP);
+		return (pgd_t *)pte_alloc_one_virt(mm);
 	else
 		return kmem_cache_alloc(pgd_cache, PGALLOC_GFP);
 }
@@ -39,7 +39,7 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
 void pgd_free(struct mm_struct *mm, pgd_t *pgd)
 {
 	if (PGD_SIZE == PAGE_SIZE)
-		free_page((unsigned long)pgd);
+		pte_free(mm, virt_to_page(pgd));
 	else
 		kmem_cache_free(pgd_cache, pgd);
 }
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/6] arm64/mm: Call pgtable_page_dtor() for both PMD and PUD page table pages

[Anshuman Khandual]

TLB flush during PMD and PUD level page table page tear down currently does
not call pgtable_page_dtor() like that for PTE page table pages. Now that
all the page table pages which are allocated from the buddy allocator must
have been initialized through pgtable_page_ctor() they must be freed back
only after being called into pgtable_page_dtor() destructor. Else these
pages might run into bad page state errors while getting freed into buddy
allocator.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/include/asm/tlb.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h
index 106fdc9..de46ba0 100644
--- a/arch/arm64/include/asm/tlb.h
+++ b/arch/arm64/include/asm/tlb.h
@@ -62,6 +62,7 @@ static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte,
 static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmdp,
 				  unsigned long addr)
 {
+	pgtable_page_dtor(virt_to_page(pmdp));
 	tlb_remove_table(tlb, virt_to_page(pmdp));
 }
 #endif
@@ -70,6 +71,7 @@ static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmdp,
 static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pudp,
 				  unsigned long addr)
 {
+	pgtable_page_dtor(virt_to_page(pudp));
 	tlb_remove_table(tlb, virt_to_page(pudp));
 }
 #endif
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/6] arm64/mm: Enable page table page accounting for user space

[Anshuman Khandual]

Page table pages created for user space processes must be accounted against
their memory control groups if initialized. This just introduces a helper
function pgtable_gfp_flags() which returns PGALLOC_GFP for all kernel page
table page allocations but adds __GFP_ACCOUNT for user page table pages.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/include/asm/pgalloc.h | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
index 076764f..775cde9 100644
--- a/arch/arm64/include/asm/pgalloc.h
+++ b/arch/arm64/include/asm/pgalloc.h
@@ -100,12 +100,22 @@ pte_alloc_one_kernel(struct mm_struct *mm)
 	return (pte_t *)pte_alloc_one_virt(mm);
 }
 
+static inline gfp_t pgtable_gfp_flags(struct mm_struct *mm)
+{
+	if (unlikely(mm == &init_mm))
+		return PGALLOC_GFP;
+	else
+		return PGALLOC_GFP | __GFP_ACCOUNT;
+}
+
 static inline pgtable_t
 pte_alloc_one(struct mm_struct *mm)
 {
 	struct page *pte;
+	gfp_t gfp;
 
-	pte = alloc_pages(PGALLOC_GFP, 0);
+	gfp = pgtable_gfp_flags(mm);
+	pte = alloc_pages(gfp, 0);
 	if (!pte)
 		return NULL;
 	if (!pgtable_page_ctor(pte)) {
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] arm64/mm: Ensure PGD kmem cache object is actually allocated

[Catalin Marinas]

On Thu, Jan 24, 2019 at 10:50:16AM +0530, Anshuman Khandual wrote:
> 'pgd_cache' is created for allocating non PAGE_SIZE based PGD pointers. In
> such cases ensure that the kmem cache object is allocated before proceeding
> further with boot.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/arm64/mm/pgd.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/mm/pgd.c b/arch/arm64/mm/pgd.c
> index 289f911..8cb4e39 100644
> --- a/arch/arm64/mm/pgd.c
> +++ b/arch/arm64/mm/pgd.c
> @@ -62,4 +62,5 @@ void __init pgd_cache_init(void)
>  	 */
>  	pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, PGD_SIZE,
>  				      SLAB_PANIC, NULL);
> +	BUG_ON(!pgd_cache);

Isn't SLAB_PANIC sufficient here?

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] arm64/mm: Ensure PGD kmem cache object is actually allocated

[Anshuman Khandual]

On 01/24/2019 08:46 PM, Catalin Marinas wrote:
> On Thu, Jan 24, 2019 at 10:50:16AM +0530, Anshuman Khandual wrote:
>> 'pgd_cache' is created for allocating non PAGE_SIZE based PGD pointers. In
>> such cases ensure that the kmem cache object is allocated before proceeding
>> further with boot.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  arch/arm64/mm/pgd.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/mm/pgd.c b/arch/arm64/mm/pgd.c
>> index 289f911..8cb4e39 100644
>> --- a/arch/arm64/mm/pgd.c
>> +++ b/arch/arm64/mm/pgd.c
>> @@ -62,4 +62,5 @@ void __init pgd_cache_init(void)
>>  	 */
>>  	pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, PGD_SIZE,
>>  				      SLAB_PANIC, NULL);
>> +	BUG_ON(!pgd_cache);
> 
> Isn't SLAB_PANIC sufficient here?

It should be sufficient as it calls panic(). Please drop this patch from consideration.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] arm64/mm: Make pgd_pgtable_alloc() call pte_alloc_one() always

[Will Deacon]

On Thu, Jan 24, 2019 at 10:50:17AM +0530, Anshuman Khandual wrote:
> pgd_pgtable_alloc() provides page allocation function while creating all
> levels of page table (e.g PGD, PUD, CONT PMD) for various kernel mappings.
> It calls __get_free_page() and initializes page with pagetable_page_ctor().
> pte_alloc_one() already provides a standard interface for allocating a page
> table page and initializes it with pagetable_page_ctor(). This removes the
> redundancy and instead make it call pte_alloc_one() directly.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/arm64/mm/mmu.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index b6f5aa5..2dbd723 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -372,13 +372,22 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
>  
>  static phys_addr_t pgd_pgtable_alloc(void)
>  {
> -	void *ptr = (void *)__get_free_page(PGALLOC_GFP);
> -	if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
> -		BUG();
> +
> +	pgtable_t ptr;
> +
> +	/*
> +	 * pgd_pgtable_alloc() is called while creating kernel mappings
> +	 * through __create_pgd_mapping() might not install it through
> +	 * swapper_pg_dir (&init_mm). Even then init_mm is passed here
> +	 * just to indicate that the allocation is kernel specific not
> +	 * for the user space page tables.
> +	 */
> +	ptr = pte_alloc_one(&init_mm);
> +	BUG_ON(!ptr);
>  
>  	/* Ensure the zeroed page is visible to the page table walker */
>  	dsb(ishst);
> -	return __pa(ptr);
> +	return page_to_phys(ptr);

Are you sure this is safe? afaict, this code is used to set up the linear
mapping which happens before the mem_map is initialised. I don't think
page_to_phys() can be used at this point. What am I missing?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/6] arm64/mm: Call pgtable_page_dtor() for both PMD and PUD page table pages

[Will Deacon]

On Thu, Jan 24, 2019 at 10:50:19AM +0530, Anshuman Khandual wrote:
> TLB flush during PMD and PUD level page table page tear down currently does
> not call pgtable_page_dtor() like that for PTE page table pages. Now that
> all the page table pages which are allocated from the buddy allocator must
> have been initialized through pgtable_page_ctor() they must be freed back
> only after being called into pgtable_page_dtor() destructor. Else these
> pages might run into bad page state errors while getting freed into buddy
> allocator.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/arm64/include/asm/tlb.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h
> index 106fdc9..de46ba0 100644
> --- a/arch/arm64/include/asm/tlb.h
> +++ b/arch/arm64/include/asm/tlb.h
> @@ -62,6 +62,7 @@ static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte,
>  static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmdp,
>  				  unsigned long addr)
>  {
> +	pgtable_page_dtor(virt_to_page(pmdp));
>  	tlb_remove_table(tlb, virt_to_page(pmdp));
>  }
>  #endif
> @@ -70,6 +71,7 @@ static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmdp,
>  static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pudp,
>  				  unsigned long addr)
>  {
> +	pgtable_page_dtor(virt_to_page(pudp));
>  	tlb_remove_table(tlb, virt_to_page(pudp));

This would be cleaner if you assigned to a temporary struct page * variable.
With that:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/6] arm64/mm: Enable page table page accounting for user space

[Will Deacon]

On Thu, Jan 24, 2019 at 10:50:20AM +0530, Anshuman Khandual wrote:
> Page table pages created for user space processes must be accounted against
> their memory control groups if initialized. This just introduces a helper
> function pgtable_gfp_flags() which returns PGALLOC_GFP for all kernel page
> table page allocations but adds __GFP_ACCOUNT for user page table pages.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/arm64/include/asm/pgalloc.h | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
> index 076764f..775cde9 100644
> --- a/arch/arm64/include/asm/pgalloc.h
> +++ b/arch/arm64/include/asm/pgalloc.h
> @@ -100,12 +100,22 @@ pte_alloc_one_kernel(struct mm_struct *mm)
>  	return (pte_t *)pte_alloc_one_virt(mm);
>  }
>  
> +static inline gfp_t pgtable_gfp_flags(struct mm_struct *mm)
> +{
> +	if (unlikely(mm == &init_mm))
> +		return PGALLOC_GFP;
> +	else
> +		return PGALLOC_GFP | __GFP_ACCOUNT;
> +}

Hmm, what about things like the efi mm?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/6] arm64/mm: Make pgd_pgtable_alloc() call pte_alloc_one() always

[Anshuman Khandual]

On 02/14/2019 10:41 PM, Will Deacon wrote:
> On Thu, Jan 24, 2019 at 10:50:17AM +0530, Anshuman Khandual wrote:
>> pgd_pgtable_alloc() provides page allocation function while creating all
>> levels of page table (e.g PGD, PUD, CONT PMD) for various kernel mappings.
>> It calls __get_free_page() and initializes page with pagetable_page_ctor().
>> pte_alloc_one() already provides a standard interface for allocating a page
>> table page and initializes it with pagetable_page_ctor(). This removes the
>> redundancy and instead make it call pte_alloc_one() directly.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  arch/arm64/mm/mmu.c | 17 +++++++++++++----
>>  1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index b6f5aa5..2dbd723 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -372,13 +372,22 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
>>  
>>  static phys_addr_t pgd_pgtable_alloc(void)
>>  {
>> -	void *ptr = (void *)__get_free_page(PGALLOC_GFP);
>> -	if (!ptr || !pgtable_page_ctor(virt_to_page(ptr)))
>> -		BUG();
>> +
>> +	pgtable_t ptr;
>> +
>> +	/*
>> +	 * pgd_pgtable_alloc() is called while creating kernel mappings
>> +	 * through __create_pgd_mapping() might not install it through
>> +	 * swapper_pg_dir (&init_mm). Even then init_mm is passed here
>> +	 * just to indicate that the allocation is kernel specific not
>> +	 * for the user space page tables.
>> +	 */
>> +	ptr = pte_alloc_one(&init_mm);
>> +	BUG_ON(!ptr);
>>  
>>  	/* Ensure the zeroed page is visible to the page table walker */
>>  	dsb(ishst);
>> -	return __pa(ptr);
>> +	return page_to_phys(ptr);
> 
> Are you sure this is safe? afaict, this code is used to set up the linear
> mapping which happens before the mem_map is initialised. I don't think
> page_to_phys() can be used at this point. What am I missing?

We are calling into buddy allocator to get a page before coming here. So
mem_map is definitely initialized. There is again early_pgtable_alloc()
used for __create_pgd_mapping() which allocates page from memblock when
buddy has not been initialized.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/6] arm64/mm: Call pgtable_page_dtor() for both PMD and PUD page table pages

[Anshuman Khandual]

On 02/14/2019 10:41 PM, Will Deacon wrote:
> On Thu, Jan 24, 2019 at 10:50:19AM +0530, Anshuman Khandual wrote:
>> TLB flush during PMD and PUD level page table page tear down currently does
>> not call pgtable_page_dtor() like that for PTE page table pages. Now that
>> all the page table pages which are allocated from the buddy allocator must
>> have been initialized through pgtable_page_ctor() they must be freed back
>> only after being called into pgtable_page_dtor() destructor. Else these
>> pages might run into bad page state errors while getting freed into buddy
>> allocator.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  arch/arm64/include/asm/tlb.h | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h
>> index 106fdc9..de46ba0 100644
>> --- a/arch/arm64/include/asm/tlb.h
>> +++ b/arch/arm64/include/asm/tlb.h
>> @@ -62,6 +62,7 @@ static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte,
>>  static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmdp,
>>  				  unsigned long addr)
>>  {
>> +	pgtable_page_dtor(virt_to_page(pmdp));
>>  	tlb_remove_table(tlb, virt_to_page(pmdp));
>>  }
>>  #endif
>> @@ -70,6 +71,7 @@ static inline void __pmd_free_tlb(struct mmu_gather *tlb, pmd_t *pmdp,
>>  static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pudp,
>>  				  unsigned long addr)
>>  {
>> +	pgtable_page_dtor(virt_to_page(pudp));
>>  	tlb_remove_table(tlb, virt_to_page(pudp));
> 
> This would be cleaner if you assigned to a temporary struct page * variable.
> With that:

Sure will do.

> 
> Acked-by: Will Deacon <will.deacon@arm.com>
> 
> Will
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/6] arm64/mm: Enable page table page accounting for user space

[Anshuman Khandual]

On 02/14/2019 10:41 PM, Will Deacon wrote:
> On Thu, Jan 24, 2019 at 10:50:20AM +0530, Anshuman Khandual wrote:
>> Page table pages created for user space processes must be accounted against
>> their memory control groups if initialized. This just introduces a helper
>> function pgtable_gfp_flags() which returns PGALLOC_GFP for all kernel page
>> table page allocations but adds __GFP_ACCOUNT for user page table pages.
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  arch/arm64/include/asm/pgalloc.h | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h
>> index 076764f..775cde9 100644
>> --- a/arch/arm64/include/asm/pgalloc.h
>> +++ b/arch/arm64/include/asm/pgalloc.h
>> @@ -100,12 +100,22 @@ pte_alloc_one_kernel(struct mm_struct *mm)
>>  	return (pte_t *)pte_alloc_one_virt(mm);
>>  }
>>  
>> +static inline gfp_t pgtable_gfp_flags(struct mm_struct *mm)
>> +{
>> +	if (unlikely(mm == &init_mm))
>> +		return PGALLOC_GFP;
>> +	else
>> +		return PGALLOC_GFP | __GFP_ACCOUNT;
>> +}
> 
> Hmm, what about things like the efi mm?


EFI mappings are created with

efi_virtmap_init() -> efi_create_mapping() -> create_pgd_mapping()

which calls into pgd_pgtable_alloc(). Sure will add check for efi_mm along
with init_mm not to use __GFP_ACCOUNT.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel