| Message ID | 20210811004258.138075-10-vgupta@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | ARC mm updates to support 3 or 4 levels of paging | expand |
On Tue, Aug 10, 2021 at 05:42:49PM -0700, Vineet Gupta wrote: > Signed-off-by: Vineet Gupta <vgupta@kernel.org> > --- > arch/arc/include/asm/page.h | 30 ++++++++++++++++-------------- > arch/arc/include/asm/pgalloc.h | 7 ++++++- > 2 files changed, 22 insertions(+), 15 deletions(-) > > diff --git a/arch/arc/include/asm/page.h b/arch/arc/include/asm/page.h > index c4ac827379cd..313e6f543d2d 100644 > --- a/arch/arc/include/asm/page.h > +++ b/arch/arc/include/asm/page.h > @@ -34,6 +34,13 @@ void copy_user_highpage(struct page *to, struct page *from, > unsigned long u_vaddr, struct vm_area_struct *vma); > void clear_user_page(void *to, unsigned long u_vaddr, struct page *page); > > +typedef struct { > + unsigned long pgd; > +} pgd_t; > + > +#define pgd_val(x) ((x).pgd) > +#define __pgd(x) ((pgd_t) { (x) }) > + > typedef struct { > #ifdef CONFIG_ARC_HAS_PAE40 > unsigned long long pte; > @@ -41,22 +48,17 @@ typedef struct { > unsigned long pte; > #endif > } pte_t; > -typedef struct { > - unsigned long pgd; > -} pgd_t; > + > +#define pte_val(x) ((x).pte) > +#define __pte(x) ((pte_t) { (x) }) > + > typedef struct { > unsigned long pgprot; > } pgprot_t; > > -#define pte_val(x) ((x).pte) > -#define pgd_val(x) ((x).pgd) > -#define pgprot_val(x) ((x).pgprot) > - > -#define __pte(x) ((pte_t) { (x) }) > -#define __pgd(x) ((pgd_t) { (x) }) > -#define __pgprot(x) ((pgprot_t) { (x) }) > - > -#define pte_pgprot(x) __pgprot(pte_val(x)) > +#define pgprot_val(x) ((x).pgprot) > +#define __pgprot(x) ((pgprot_t) { (x) }) > +#define pte_pgprot(x) __pgprot(pte_val(x)) > > typedef pte_t * pgtable_t; > > @@ -96,8 +98,8 @@ extern int pfn_valid(unsigned long pfn); > * virt here means link-address/program-address as embedded in object code. > * And for ARC, link-addr = physical address > */ > -#define __pa(vaddr) ((unsigned long)(vaddr)) > -#define __va(paddr) ((void *)((unsigned long)(paddr))) > +#define __pa(vaddr) ((unsigned long)(vaddr)) > +#define __va(paddr) ((void *)((unsigned long)(paddr))) > > #define virt_to_page(kaddr) pfn_to_page(virt_to_pfn(kaddr)) > #define virt_addr_valid(kaddr) pfn_valid(virt_to_pfn(kaddr)) > diff --git a/arch/arc/include/asm/pgalloc.h b/arch/arc/include/asm/pgalloc.h > index 356237b9c537..0cf73431eb89 100644 > --- a/arch/arc/include/asm/pgalloc.h > +++ b/arch/arc/include/asm/pgalloc.h > @@ -29,6 +29,11 @@ > #ifndef _ASM_ARC_PGALLOC_H > #define _ASM_ARC_PGALLOC_H > > +/* > + * For ARC, pgtable_t is not struct page *, but pte_t * (to avoid > + * extraneous page_address() calculations) hence can't use > + * use asm-generic/pgalloc.h which assumes it being struct page * > + */ Another reason to leave ARC without asm-generic/pgalloc.h was __get_order_pte() that other arches don't have. So this and pgtable_t aliased to pte_t * seemed to me too much to bother then, but probably it's worth reconsidering with addition of 3rd and 4th levels. > #include <linux/mm.h> > #include <linux/log2.h> > > @@ -36,7 +41,7 @@ static inline void > pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmdp, pte_t *ptep) > { > /* > - * The cast to long below is OK even when pte is long long (PAE40) > + * The cast to long below is OK in 32-bit PAE40 regime with long long pte > * Despite "wider" pte, the pte table needs to be in non-PAE low memory > * as all higher levels can only hold long pointers. > * > -- > 2.25.1 >
On 8/11/21 5:31 AM, Mike Rapoport wrote: >> +/* >> + * For ARC, pgtable_t is not struct page *, but pte_t * (to avoid >> + * extraneous page_address() calculations) hence can't use >> + * use asm-generic/pgalloc.h which assumes it being struct page * >> + */ > Another reason to leave ARC without asm-generic/pgalloc.h was > __get_order_pte() that other arches don't have. > So this and pgtable_t aliased to pte_t * seemed to me too much to bother > then, but probably it's worth reconsidering with addition of 3rd and 4th > levels. I agree that savings of not havign page_address() might not be huge. However asm-generic/pgalloc.h only has pte allocation routines and all other allocation levels come from arch file Also for ARCv2, given the arbitrary address split and ensuing paging levels (as well as support for different page sizes) we will need to make sure that one page is enough to hold any level's paging using say BUILD_BUG_ON. In fact that should also be done for 3rd and 4th levels for sanity. -Vineet
On Wed, Aug 11, 2021 at 06:37:19PM -0700, Vineet Gupta wrote: > On 8/11/21 5:31 AM, Mike Rapoport wrote: > > > +/* > > > + * For ARC, pgtable_t is not struct page *, but pte_t * (to avoid > > > + * extraneous page_address() calculations) hence can't use > > > + * use asm-generic/pgalloc.h which assumes it being struct page * > > > + */ > > Another reason to leave ARC without asm-generic/pgalloc.h was > > __get_order_pte() that other arches don't have. > > So this and pgtable_t aliased to pte_t * seemed to me too much to bother > > then, but probably it's worth reconsidering with addition of 3rd and 4th > > levels. > > I agree that savings of not havign page_address() might not be huge. However > asm-generic/pgalloc.h only has pte allocation routines and all other > allocation levels come from arch file asm-generic/pgalloc.h has allocation routines up to PUD. There is also pgtable_pmd_page_ctor() and pgtable_pmd_page_dtor() called in the generic versions of PMD allocation, it seems they are not called in ARC implementation. So using asm-generic/pgalloc.h would probably save you some THP debugging ;-) We may even probably accommodate multi-page PTEs in asm-generic/pgalloc.h with something like #ifndef __HAVE_ARCH_PTE_GET_ORDER static inline int __pte_get_order(void) { return 0; } #endif > Also for ARCv2, given the arbitrary address split and ensuing paging levels > (as well as support for different page sizes) we will need to make sure that > one page is enough to hold any level's paging using say BUILD_BUG_ON. In > fact that should also be done for 3rd and 4th levels for sanity. Right, these sanity checks would be useful, but they may live in one of .c files in arch/arc/mm. > -Vineet > >
On 8/11/21 11:18 PM, Mike Rapoport wrote: > On Wed, Aug 11, 2021 at 06:37:19PM -0700, Vineet Gupta wrote: >> On 8/11/21 5:31 AM, Mike Rapoport wrote: >>>> +/* >>>> + * For ARC, pgtable_t is not struct page *, but pte_t * (to avoid >>>> + * extraneous page_address() calculations) hence can't use >>>> + * use asm-generic/pgalloc.h which assumes it being struct page * >>>> + */ >>> Another reason to leave ARC without asm-generic/pgalloc.h was >>> __get_order_pte() that other arches don't have. >>> So this and pgtable_t aliased to pte_t * seemed to me too much to bother >>> then, but probably it's worth reconsidering with addition of 3rd and 4th >>> levels. >> I agree that savings of not havign page_address() might not be huge. However >> asm-generic/pgalloc.h only has pte allocation routines and all other >> allocation levels come from arch file > asm-generic/pgalloc.h has allocation routines up to PUD. > There is also pgtable_pmd_page_ctor() and pgtable_pmd_page_dtor() called in > the generic versions of PMD allocation, it seems they are not called in ARC > implementation. :-( > So using asm-generic/pgalloc.h would probably save you some THP debugging ;-) > > We may even probably accommodate multi-page PTEs in asm-generic/pgalloc.h > with something like > > #ifndef __HAVE_ARCH_PTE_GET_ORDER > static inline int __pte_get_order(void) > { > return 0; > } > #endif Not needed - those cases are unreal, esoteric at best. I'm working on switching back to canonical struct page based pgtable_t, will do that in v2. >> Also for ARCv2, given the arbitrary address split and ensuing paging levels >> (as well as support for different page sizes) we will need to make sure that >> one page is enough to hold any level's paging using say BUILD_BUG_ON. In >> fact that should also be done for 3rd and 4th levels for sanity. > Right, these sanity checks would be useful, but they may live in one of .c > files in arch/arc/mm. Sure ! Thx, -Vineet
diff --git a/arch/arc/include/asm/page.h b/arch/arc/include/asm/page.h index c4ac827379cd..313e6f543d2d 100644 --- a/arch/arc/include/asm/page.h +++ b/arch/arc/include/asm/page.h @@ -34,6 +34,13 @@ void copy_user_highpage(struct page *to, struct page *from, unsigned long u_vaddr, struct vm_area_struct *vma); void clear_user_page(void *to, unsigned long u_vaddr, struct page *page); +typedef struct { + unsigned long pgd; +} pgd_t; + +#define pgd_val(x) ((x).pgd) +#define __pgd(x) ((pgd_t) { (x) }) + typedef struct { #ifdef CONFIG_ARC_HAS_PAE40 unsigned long long pte; @@ -41,22 +48,17 @@ typedef struct { unsigned long pte; #endif } pte_t; -typedef struct { - unsigned long pgd; -} pgd_t; + +#define pte_val(x) ((x).pte) +#define __pte(x) ((pte_t) { (x) }) + typedef struct { unsigned long pgprot; } pgprot_t; -#define pte_val(x) ((x).pte) -#define pgd_val(x) ((x).pgd) -#define pgprot_val(x) ((x).pgprot) - -#define __pte(x) ((pte_t) { (x) }) -#define __pgd(x) ((pgd_t) { (x) }) -#define __pgprot(x) ((pgprot_t) { (x) }) - -#define pte_pgprot(x) __pgprot(pte_val(x)) +#define pgprot_val(x) ((x).pgprot) +#define __pgprot(x) ((pgprot_t) { (x) }) +#define pte_pgprot(x) __pgprot(pte_val(x)) typedef pte_t * pgtable_t; @@ -96,8 +98,8 @@ extern int pfn_valid(unsigned long pfn); * virt here means link-address/program-address as embedded in object code. * And for ARC, link-addr = physical address */ -#define __pa(vaddr) ((unsigned long)(vaddr)) -#define __va(paddr) ((void *)((unsigned long)(paddr))) +#define __pa(vaddr) ((unsigned long)(vaddr)) +#define __va(paddr) ((void *)((unsigned long)(paddr))) #define virt_to_page(kaddr) pfn_to_page(virt_to_pfn(kaddr)) #define virt_addr_valid(kaddr) pfn_valid(virt_to_pfn(kaddr)) diff --git a/arch/arc/include/asm/pgalloc.h b/arch/arc/include/asm/pgalloc.h index 356237b9c537..0cf73431eb89 100644 --- a/arch/arc/include/asm/pgalloc.h +++ b/arch/arc/include/asm/pgalloc.h @@ -29,6 +29,11 @@ #ifndef _ASM_ARC_PGALLOC_H #define _ASM_ARC_PGALLOC_H +/* + * For ARC, pgtable_t is not struct page *, but pte_t * (to avoid + * extraneous page_address() calculations) hence can't use + * use asm-generic/pgalloc.h which assumes it being struct page * + */ #include <linux/mm.h> #include <linux/log2.h> @@ -36,7 +41,7 @@ static inline void pmd_populate_kernel(struct mm_struct *mm, pmd_t *pmdp, pte_t *ptep) { /* - * The cast to long below is OK even when pte is long long (PAE40) + * The cast to long below is OK in 32-bit PAE40 regime with long long pte * Despite "wider" pte, the pte table needs to be in non-PAE low memory * as all higher levels can only hold long pointers. *
Signed-off-by: Vineet Gupta <vgupta@kernel.org> --- arch/arc/include/asm/page.h | 30 ++++++++++++++++-------------- arch/arc/include/asm/pgalloc.h | 7 ++++++- 2 files changed, 22 insertions(+), 15 deletions(-)