| Message ID | 20200908071931.47767-2-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] arm64/mm: Remove CONT_RANGE_OFFSET | expand |
On 09/08/2020 12:49 PM, Gavin Shan wrote: > The macro CONT_PTE_SHIFT actually depends on CONT_SHIFT, which has > been defined in page-def.h, based on CONFIG_ARM64_CONT_SHIFT. Lets > reflect the dependency. > > Signed-off-by: Gavin Shan <gshan@redhat.com> > --- > arch/arm64/include/asm/pgtable-hwdef.h | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h > index 8a399e666837..0bd9469f4323 100644 > --- a/arch/arm64/include/asm/pgtable-hwdef.h > +++ b/arch/arm64/include/asm/pgtable-hwdef.h > @@ -81,14 +81,12 @@ > /* > * Contiguous page definitions. > */ > +#define CONT_PTE_SHIFT (CONT_SHIFT + PAGE_SHIFT) > #ifdef CONFIG_ARM64_64K_PAGES > -#define CONT_PTE_SHIFT (5 + PAGE_SHIFT) > #define CONT_PMD_SHIFT (5 + PMD_SHIFT) > #elif defined(CONFIG_ARM64_16K_PAGES) > -#define CONT_PTE_SHIFT (7 + PAGE_SHIFT) > #define CONT_PMD_SHIFT (5 + PMD_SHIFT) > #else > -#define CONT_PTE_SHIFT (4 + PAGE_SHIFT) > #define CONT_PMD_SHIFT (4 + PMD_SHIFT) > #endif Could not a similar CONT_PMD be created from a new CONFIG_ARM64_CONT_PMD config option, which would help unify CONT_PMD_SHIFT here as well ?
Hi Anshuman, On 9/10/20 4:17 PM, Anshuman Khandual wrote: > On 09/08/2020 12:49 PM, Gavin Shan wrote: >> The macro CONT_PTE_SHIFT actually depends on CONT_SHIFT, which has >> been defined in page-def.h, based on CONFIG_ARM64_CONT_SHIFT. Lets >> reflect the dependency. >> >> Signed-off-by: Gavin Shan <gshan@redhat.com> >> --- >> arch/arm64/include/asm/pgtable-hwdef.h | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h >> index 8a399e666837..0bd9469f4323 100644 >> --- a/arch/arm64/include/asm/pgtable-hwdef.h >> +++ b/arch/arm64/include/asm/pgtable-hwdef.h >> @@ -81,14 +81,12 @@ >> /* >> * Contiguous page definitions. >> */ >> +#define CONT_PTE_SHIFT (CONT_SHIFT + PAGE_SHIFT) >> #ifdef CONFIG_ARM64_64K_PAGES >> -#define CONT_PTE_SHIFT (5 + PAGE_SHIFT) >> #define CONT_PMD_SHIFT (5 + PMD_SHIFT) >> #elif defined(CONFIG_ARM64_16K_PAGES) >> -#define CONT_PTE_SHIFT (7 + PAGE_SHIFT) >> #define CONT_PMD_SHIFT (5 + PMD_SHIFT) >> #else >> -#define CONT_PTE_SHIFT (4 + PAGE_SHIFT) >> #define CONT_PMD_SHIFT (4 + PMD_SHIFT) >> #endif > Could not a similar CONT_PMD be created from a new CONFIG_ARM64_CONT_PMD > config option, which would help unify CONT_PMD_SHIFT here as well ? > I was thinking of it, to have CONFIG_ARM64_CONT_PMD and defined the following macros in arch/arm64/include/asm/page-def.h: #define CONT_PMD_SHIFT CONFIG_ARM64_CONT_PMD_SHIFT #define CONT_PMD_SIZE (_AC(1, UL) << (CONT_PMD_SHIFT + PMD_SHIFT) #define CONT_PMD_MASK (~(CONT_PMD_SIZE - 1)) PMD_SHIFT is variable because PMD could be folded into PUD or PGD, depending on the kernel configuration. PMD_SHIFT is declared in arch/arm64/include/asm/pgtable-types.h, which isn't supposed to be included in "page-def.h". So the peroper way to handle this might be drop the continuous page macros in page-def.h and introduce the following ones into pgtable-hwdef.h. I will post v2 to do this if it sounds good to you. #define CONT_PTE_SHIFT (CONFIG_ARM64_CONT_PTE_SHIFT + PAGE_SHIFT) #define CONT_PMD_SHIFT (CONFIG_ARM64_CONT_PMD_SHIFT + PMD_SHIFT) Thanks, Gavin
On 09/10/2020 02:01 PM, Gavin Shan wrote: > Hi Anshuman, > > On 9/10/20 4:17 PM, Anshuman Khandual wrote: >> On 09/08/2020 12:49 PM, Gavin Shan wrote: >>> The macro CONT_PTE_SHIFT actually depends on CONT_SHIFT, which has >>> been defined in page-def.h, based on CONFIG_ARM64_CONT_SHIFT. Lets >>> reflect the dependency. >>> >>> Signed-off-by: Gavin Shan <gshan@redhat.com> >>> --- >>> arch/arm64/include/asm/pgtable-hwdef.h | 4 +--- >>> 1 file changed, 1 insertion(+), 3 deletions(-) >>> >>> diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h >>> index 8a399e666837..0bd9469f4323 100644 >>> --- a/arch/arm64/include/asm/pgtable-hwdef.h >>> +++ b/arch/arm64/include/asm/pgtable-hwdef.h >>> @@ -81,14 +81,12 @@ >>> /* >>> * Contiguous page definitions. >>> */ >>> +#define CONT_PTE_SHIFT (CONT_SHIFT + PAGE_SHIFT) >>> #ifdef CONFIG_ARM64_64K_PAGES >>> -#define CONT_PTE_SHIFT (5 + PAGE_SHIFT) >>> #define CONT_PMD_SHIFT (5 + PMD_SHIFT) >>> #elif defined(CONFIG_ARM64_16K_PAGES) >>> -#define CONT_PTE_SHIFT (7 + PAGE_SHIFT) >>> #define CONT_PMD_SHIFT (5 + PMD_SHIFT) >>> #else >>> -#define CONT_PTE_SHIFT (4 + PAGE_SHIFT) >>> #define CONT_PMD_SHIFT (4 + PMD_SHIFT) >>> #endif >> Could not a similar CONT_PMD be created from a new CONFIG_ARM64_CONT_PMD >> config option, which would help unify CONT_PMD_SHIFT here as well ? >> > > I was thinking of it, to have CONFIG_ARM64_CONT_PMD and defined the > following macros in arch/arm64/include/asm/page-def.h: > > #define CONT_PMD_SHIFT CONFIG_ARM64_CONT_PMD_SHIFT > #define CONT_PMD_SIZE (_AC(1, UL) << (CONT_PMD_SHIFT + PMD_SHIFT) > #define CONT_PMD_MASK (~(CONT_PMD_SIZE - 1)) > > PMD_SHIFT is variable because PMD could be folded into PUD or PGD, > depending on the kernel configuration. PMD_SHIFT is declared Even CONT_PMD_SHIFT via the new CONFIG_ARM64_CONT_PMD_SHIFT will be a variable as well depending on page size. > in arch/arm64/include/asm/pgtable-types.h, which isn't supposed > to be included in "page-def.h". Are there build failures if <pgtable-types.h> is included from <page-def.h> ? > > So the peroper way to handle this might be drop the continuous page > macros in page-def.h and introduce the following ones into pgtable-hwdef.h. > I will post v2 to do this if it sounds good to you. Sure, go ahead if that builds. But unifying both these macros seems cleaner. > > #define CONT_PTE_SHIFT (CONFIG_ARM64_CONT_PTE_SHIFT + PAGE_SHIFT) > #define CONT_PMD_SHIFT (CONFIG_ARM64_CONT_PMD_SHIFT + PMD_SHIFT) > > Thanks, > Gavin > >
Hi Anshuman, On 9/10/20 7:28 PM, Anshuman Khandual wrote: > On 09/10/2020 02:01 PM, Gavin Shan wrote: >> On 9/10/20 4:17 PM, Anshuman Khandual wrote: >>> On 09/08/2020 12:49 PM, Gavin Shan wrote: >>>> The macro CONT_PTE_SHIFT actually depends on CONT_SHIFT, which has >>>> been defined in page-def.h, based on CONFIG_ARM64_CONT_SHIFT. Lets >>>> reflect the dependency. >>>> >>>> Signed-off-by: Gavin Shan <gshan@redhat.com> >>>> --- >>>> arch/arm64/include/asm/pgtable-hwdef.h | 4 +--- >>>> 1 file changed, 1 insertion(+), 3 deletions(-) >>>> >>>> diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h >>>> index 8a399e666837..0bd9469f4323 100644 >>>> --- a/arch/arm64/include/asm/pgtable-hwdef.h >>>> +++ b/arch/arm64/include/asm/pgtable-hwdef.h >>>> @@ -81,14 +81,12 @@ >>>> /* >>>> * Contiguous page definitions. >>>> */ >>>> +#define CONT_PTE_SHIFT (CONT_SHIFT + PAGE_SHIFT) >>>> #ifdef CONFIG_ARM64_64K_PAGES >>>> -#define CONT_PTE_SHIFT (5 + PAGE_SHIFT) >>>> #define CONT_PMD_SHIFT (5 + PMD_SHIFT) >>>> #elif defined(CONFIG_ARM64_16K_PAGES) >>>> -#define CONT_PTE_SHIFT (7 + PAGE_SHIFT) >>>> #define CONT_PMD_SHIFT (5 + PMD_SHIFT) >>>> #else >>>> -#define CONT_PTE_SHIFT (4 + PAGE_SHIFT) >>>> #define CONT_PMD_SHIFT (4 + PMD_SHIFT) >>>> #endif >>> Could not a similar CONT_PMD be created from a new CONFIG_ARM64_CONT_PMD >>> config option, which would help unify CONT_PMD_SHIFT here as well ? >>> >> >> I was thinking of it, to have CONFIG_ARM64_CONT_PMD and defined the >> following macros in arch/arm64/include/asm/page-def.h: >> >> #define CONT_PMD_SHIFT CONFIG_ARM64_CONT_PMD_SHIFT >> #define CONT_PMD_SIZE (_AC(1, UL) << (CONT_PMD_SHIFT + PMD_SHIFT) >> #define CONT_PMD_MASK (~(CONT_PMD_SIZE - 1)) >> >> PMD_SHIFT is variable because PMD could be folded into PUD or PGD, >> depending on the kernel configuration. PMD_SHIFT is declared > > Even CONT_PMD_SHIFT via the new CONFIG_ARM64_CONT_PMD_SHIFT will > be a variable as well depending on page size. > Yes, it depends on the variable PMD_SHIFT. >> in arch/arm64/include/asm/pgtable-types.h, which isn't supposed >> to be included in "page-def.h". > > Are there build failures if <pgtable-types.h> is included from <page-def.h> ? > Yes, something like this: AS arch/arm64/kernel/head.o In file included from ./arch/arm64/include/asm/pgtable-hwdef.h:8, from ./arch/arm64/include/asm/page-def.h:12, from ./arch/arm64/include/asm/page.h:11, from ./arch/arm64/include/asm/proc-fns.h:14, from ./arch/arm64/include/asm/pgtable.h:9, from ./include/linux/pgtable.h:6, from ./arch/arm64/include/asm/io.h:12, from ./include/linux/io.h:13, from drivers/bus/vexpress-config.c:9: ./arch/arm64/include/asm/memory.h:91:55: warning: "PAGE_SHIFT" is not defined, evaluates to 0 [-Wundef] #if defined(CONFIG_VMAP_STACK) && (MIN_THREAD_SHIFT < PAGE_SHIFT) ^~~~~~~~~~ ./arch/arm64/include/asm/memory.h:97:21: warning: "PAGE_SHIFT" is not defined, evaluates to 0 [-Wundef] #if THREAD_SHIFT >= PAGE_SHIFT ^~~~~~~~~~ >> >> So the peroper way to handle this might be drop the continuous page >> macros in page-def.h and introduce the following ones into pgtable-hwdef.h. >> I will post v2 to do this if it sounds good to you. > > Sure, go ahead if that builds. But unifying both these macros seems cleaner. > Thanks for your confirmation. v2 is on the way :) >> >> #define CONT_PTE_SHIFT (CONFIG_ARM64_CONT_PTE_SHIFT + PAGE_SHIFT) >> #define CONT_PMD_SHIFT (CONFIG_ARM64_CONT_PMD_SHIFT + PMD_SHIFT) >> Thanks, Gavin
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h index 8a399e666837..0bd9469f4323 100644 --- a/arch/arm64/include/asm/pgtable-hwdef.h +++ b/arch/arm64/include/asm/pgtable-hwdef.h @@ -81,14 +81,12 @@ /* * Contiguous page definitions. */ +#define CONT_PTE_SHIFT (CONT_SHIFT + PAGE_SHIFT) #ifdef CONFIG_ARM64_64K_PAGES -#define CONT_PTE_SHIFT (5 + PAGE_SHIFT) #define CONT_PMD_SHIFT (5 + PMD_SHIFT) #elif defined(CONFIG_ARM64_16K_PAGES) -#define CONT_PTE_SHIFT (7 + PAGE_SHIFT) #define CONT_PMD_SHIFT (5 + PMD_SHIFT) #else -#define CONT_PTE_SHIFT (4 + PAGE_SHIFT) #define CONT_PMD_SHIFT (4 + PMD_SHIFT) #endif
The macro CONT_PTE_SHIFT actually depends on CONT_SHIFT, which has been defined in page-def.h, based on CONFIG_ARM64_CONT_SHIFT. Lets reflect the dependency. Signed-off-by: Gavin Shan <gshan@redhat.com> --- arch/arm64/include/asm/pgtable-hwdef.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)