| Message ID | 20210202110515.3575274-2-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v12,01/14] ARM: mm: add missing pud_page define to 2-level page tables | expand |
On Tue, Feb 02, 2021 at 09:05:02PM +1000, Nicholas Piggin wrote: > diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h > index c02f24400369..d63a5bb6bd0c 100644 > --- a/arch/arm/include/asm/pgtable.h > +++ b/arch/arm/include/asm/pgtable.h > @@ -166,6 +166,9 @@ extern struct page *empty_zero_page; > > extern pgd_t swapper_pg_dir[PTRS_PER_PGD]; > > +#define pud_page(pud) pmd_page(__pmd(pud_val(pud))) > +#define pud_write(pud) pmd_write(__pmd(pud_val(pud))) As there is no PUD, does it really make sense to return a valid struct page (which will be the PTE page) for pud_page(), which is several tables above?
On 2021/2/2 19:13, Russell King - ARM Linux admin wrote: > On Tue, Feb 02, 2021 at 09:05:02PM +1000, Nicholas Piggin wrote: >> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h >> index c02f24400369..d63a5bb6bd0c 100644 >> --- a/arch/arm/include/asm/pgtable.h >> +++ b/arch/arm/include/asm/pgtable.h >> @@ -166,6 +166,9 @@ extern struct page *empty_zero_page; >> >> extern pgd_t swapper_pg_dir[PTRS_PER_PGD]; >> >> +#define pud_page(pud) pmd_page(__pmd(pud_val(pud))) >> +#define pud_write(pud) pmd_write(__pmd(pud_val(pud))) > > As there is no PUD, does it really make sense to return a valid > struct page (which will be the PTE page) for pud_page(), which is > several tables above? > --- a/arch/arm/include/asm/pgtable-2level.h +++ b/arch/arm/include/asm/pgtable-2level.h +static inline int pud_none(pud_t pud) +{ + return 0; +} I think it could be fix like this. Ding
On 2021/2/2 19:47, Ding Tianhong wrote: > On 2021/2/2 19:13, Russell King - ARM Linux admin wrote: >> On Tue, Feb 02, 2021 at 09:05:02PM +1000, Nicholas Piggin wrote: >>> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h >>> index c02f24400369..d63a5bb6bd0c 100644 >>> --- a/arch/arm/include/asm/pgtable.h >>> +++ b/arch/arm/include/asm/pgtable.h >>> @@ -166,6 +166,9 @@ extern struct page *empty_zero_page; >>> >>> extern pgd_t swapper_pg_dir[PTRS_PER_PGD]; >>> >>> +#define pud_page(pud) pmd_page(__pmd(pud_val(pud))) >>> +#define pud_write(pud) pmd_write(__pmd(pud_val(pud))) >> >> As there is no PUD, does it really make sense to return a valid >> struct page (which will be the PTE page) for pud_page(), which is >> several tables above? >> > --- a/arch/arm/include/asm/pgtable-2level.h > +++ b/arch/arm/include/asm/pgtable-2level.h > > +static inline int pud_none(pud_t pud) > +{ > + return 0; > +} > --- a/arch/arm/include/asm/pgtable-2level.h +++ b/arch/arm/include/asm/pgtable-2level.h> +static inline int pud_page(pud_t pud) +{ + return 0; +} > I think it could be fix like this. > > Ding >
On Tue, Feb 02, 2021 at 07:47:04PM +0800, Ding Tianhong wrote: > On 2021/2/2 19:13, Russell King - ARM Linux admin wrote: > > On Tue, Feb 02, 2021 at 09:05:02PM +1000, Nicholas Piggin wrote: > >> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h > >> index c02f24400369..d63a5bb6bd0c 100644 > >> --- a/arch/arm/include/asm/pgtable.h > >> +++ b/arch/arm/include/asm/pgtable.h > >> @@ -166,6 +166,9 @@ extern struct page *empty_zero_page; > >> > >> extern pgd_t swapper_pg_dir[PTRS_PER_PGD]; > >> > >> +#define pud_page(pud) pmd_page(__pmd(pud_val(pud))) > >> +#define pud_write(pud) pmd_write(__pmd(pud_val(pud))) > > > > As there is no PUD, does it really make sense to return a valid > > struct page (which will be the PTE page) for pud_page(), which is > > several tables above? > > > --- a/arch/arm/include/asm/pgtable-2level.h > +++ b/arch/arm/include/asm/pgtable-2level.h > > +static inline int pud_none(pud_t pud) > +{ > + return 0; > +} > > I think it could be fix like this. We already have that.
Excerpts from Russell King - ARM Linux admin's message of February 2, 2021 9:13 pm: > On Tue, Feb 02, 2021 at 09:05:02PM +1000, Nicholas Piggin wrote: >> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h >> index c02f24400369..d63a5bb6bd0c 100644 >> --- a/arch/arm/include/asm/pgtable.h >> +++ b/arch/arm/include/asm/pgtable.h >> @@ -166,6 +166,9 @@ extern struct page *empty_zero_page; >> >> extern pgd_t swapper_pg_dir[PTRS_PER_PGD]; >> >> +#define pud_page(pud) pmd_page(__pmd(pud_val(pud))) >> +#define pud_write(pud) pmd_write(__pmd(pud_val(pud))) > > As there is no PUD, does it really make sense to return a valid > struct page (which will be the PTE page) for pud_page(), which is > several tables above? There is no PUD on 3-level either, and the pgtable-nopud.h which it uses also passes down p4d_page to pud_page, so by convention... Although in this case at least for my next patch it won't acutally use pud_page unless it's a leaf entry so maybe it shouldn't get called anyway. Thanks, Nick
diff --git a/arch/arm/include/asm/pgtable-3level.h b/arch/arm/include/asm/pgtable-3level.h index 2b85d175e999..d4edab51a77c 100644 --- a/arch/arm/include/asm/pgtable-3level.h +++ b/arch/arm/include/asm/pgtable-3level.h @@ -186,8 +186,6 @@ static inline pte_t pte_mkspecial(pte_t pte) #define pmd_write(pmd) (pmd_isclear((pmd), L_PMD_SECT_RDONLY)) #define pmd_dirty(pmd) (pmd_isset((pmd), L_PMD_SECT_DIRTY)) -#define pud_page(pud) pmd_page(__pmd(pud_val(pud))) -#define pud_write(pud) pmd_write(__pmd(pud_val(pud))) #define pmd_hugewillfault(pmd) (!pmd_young(pmd) || !pmd_write(pmd)) #define pmd_thp_or_huge(pmd) (pmd_huge(pmd) || pmd_trans_huge(pmd)) diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h index c02f24400369..d63a5bb6bd0c 100644 --- a/arch/arm/include/asm/pgtable.h +++ b/arch/arm/include/asm/pgtable.h @@ -166,6 +166,9 @@ extern struct page *empty_zero_page; extern pgd_t swapper_pg_dir[PTRS_PER_PGD]; +#define pud_page(pud) pmd_page(__pmd(pud_val(pud))) +#define pud_write(pud) pmd_write(__pmd(pud_val(pud))) + #define pmd_none(pmd) (!pmd_val(pmd)) static inline pte_t *pmd_page_vaddr(pmd_t pmd)
ARM uses its own PMD folding scheme which is missing pud_page which should just pass through to pmd_page. Move this from the 3-level page table to common header. Cc: Russell King <linux@armlinux.org.uk> Cc: Ding Tianhong <dingtianhong@huawei.com> Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/arm/include/asm/pgtable-3level.h | 2 -- arch/arm/include/asm/pgtable.h | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-)