| Message ID | 1551071039-20192-3-git-send-email-anshuman.khandual@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64/mm: Enable accounting for page table pages | expand |
On Mon, Feb 25, 2019 at 10:33:55AM +0530, Anshuman Khandual wrote: > pgd_pgtable_alloc() provides page allocation function while creating all > levels of page table (PGD, PUD, CONT PMD etc) 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. I see we also have pte_alloc_one_kernel(), which does not call pgtable_page_ctor(). Was it deliberate that we don't call the ctor on kernel mappings? > > 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 b6f5aa52ac67..2dbd72319152 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. > + */ I'm not sure what this is trying to convey. I guess we also use this for creating the EFI mappings? No other architecture seems to use pte_alloc_one() for kernel tables, and it's not clear to me how the mm argument is intended to be used by the implementation. Thanks, Mark. > + 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.20.1 >
pgtable_page_ctor/pgtable On 02/25/2019 04:38 PM, Mark Rutland wrote: > On Mon, Feb 25, 2019 at 10:33:55AM +0530, Anshuman Khandual wrote: >> pgd_pgtable_alloc() provides page allocation function while creating all >> levels of page table (PGD, PUD, CONT PMD etc) 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. > > I see we also have pte_alloc_one_kernel(), which does not call > pgtable_page_ctor(). We are changing it. > > Was it deliberate that we don't call the ctor on kernel mappings? Right now parts of the kernel mapping call pgtable_page_ctor() like everything with __create_pgd_mapping(pgd_pgtabble_alloc,....) but pgtable_page_ctor() never gets called with pte_alloc_one_kernel() based allocations. This series is trying to move all possible kernel mappings to use pgtable_page_ctor/pgtable _page_dtor() constructs. It makes sense to call pgtable_page_ctor/pgtable_page_dtor for kernel mappings as it helps put the page in right state PageTable and also account towards zone statistics (NR_PAGETABLE). > >> >> 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 b6f5aa52ac67..2dbd72319152 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. >> + */ > > I'm not sure what this is trying to convey. I guess we also use this for > creating the EFI mappings? The argument here will be used to select right GFP flags. With this init_mm would pick the right one whether its used for init_mm or efi_mm mappings. Though we still check for both the options while picking up the GFP flags in one of the later patches. > > No other architecture seems to use pte_alloc_one() for kernel tables, > and it's not clear to me how the mm argument is intended to be used by > the implementation. It selects appropriate GFP flags (with or without __GFP_ACCOUNT). To make things better will probably pass mm_struct in pgtable_alloc() which is used with __create_pgd_mappings. Parts of arm64 kernel mappings already use it. This is just making it uniform across the kernel.
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index b6f5aa52ac67..2dbd72319152 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); } /*
pgd_pgtable_alloc() provides page allocation function while creating all levels of page table (PGD, PUD, CONT PMD etc) 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(-)