| Message ID | db6981c69f96a8c9c6dcf688b7f485e15993ddef.1618828806.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Convert powerpc to GENERIC_PTDUMP | expand |
| Related | show |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (e4361a664ee985c1f54ddecf80b93159750d93d3) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 88 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On 19/04/2021 11:47, Christophe Leroy wrote: > Pagewalk ignores hugepd entries and walk down the tables > as if it was traditionnal entries, leading to crazy result. > > Add walk_hugepd_range() and use it to walk hugepage tables. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> Looks correct to me, sadly I don't have a suitable system to test it. Reviewed-by: Steven Price <steven.price@arm.com> > --- > v2: > - Add a guard for NULL ops->pte_entry > - Take mm->page_table_lock when walking hugepage table, as suggested by follow_huge_pd() > --- > mm/pagewalk.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 53 insertions(+), 5 deletions(-) > > diff --git a/mm/pagewalk.c b/mm/pagewalk.c > index e81640d9f177..9b3db11a4d1d 100644 > --- a/mm/pagewalk.c > +++ b/mm/pagewalk.c > @@ -58,6 +58,45 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end, > return err; > } > > +#ifdef CONFIG_ARCH_HAS_HUGEPD > +static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr, > + unsigned long end, struct mm_walk *walk, int pdshift) > +{ > + int err = 0; > + const struct mm_walk_ops *ops = walk->ops; > + int shift = hugepd_shift(*phpd); > + int page_size = 1 << shift; > + > + if (!ops->pte_entry) > + return 0; > + > + if (addr & (page_size - 1)) > + return 0; > + > + for (;;) { > + pte_t *pte; > + > + spin_lock(&walk->mm->page_table_lock); > + pte = hugepte_offset(*phpd, addr, pdshift); > + err = ops->pte_entry(pte, addr, addr + page_size, walk); > + spin_unlock(&walk->mm->page_table_lock); > + > + if (err) > + break; > + if (addr >= end - page_size) > + break; > + addr += page_size; > + } > + return err; > +} > +#else > +static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr, > + unsigned long end, struct mm_walk *walk, int pdshift) > +{ > + return 0; > +} > +#endif > + > static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end, > struct mm_walk *walk) > { > @@ -108,7 +147,10 @@ static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end, > goto again; > } > > - err = walk_pte_range(pmd, addr, next, walk); > + if (is_hugepd(__hugepd(pmd_val(*pmd)))) > + err = walk_hugepd_range((hugepd_t *)pmd, addr, next, walk, PMD_SHIFT); > + else > + err = walk_pte_range(pmd, addr, next, walk); > if (err) > break; > } while (pmd++, addr = next, addr != end); > @@ -157,7 +199,10 @@ static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end, > if (pud_none(*pud)) > goto again; > > - err = walk_pmd_range(pud, addr, next, walk); > + if (is_hugepd(__hugepd(pud_val(*pud)))) > + err = walk_hugepd_range((hugepd_t *)pud, addr, next, walk, PUD_SHIFT); > + else > + err = walk_pmd_range(pud, addr, next, walk); > if (err) > break; > } while (pud++, addr = next, addr != end); > @@ -189,7 +234,9 @@ static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end, > if (err) > break; > } > - if (ops->pud_entry || ops->pmd_entry || ops->pte_entry) > + if (is_hugepd(__hugepd(p4d_val(*p4d)))) > + err = walk_hugepd_range((hugepd_t *)p4d, addr, next, walk, P4D_SHIFT); > + else if (ops->pud_entry || ops->pmd_entry || ops->pte_entry) > err = walk_pud_range(p4d, addr, next, walk); > if (err) > break; > @@ -224,8 +271,9 @@ static int walk_pgd_range(unsigned long addr, unsigned long end, > if (err) > break; > } > - if (ops->p4d_entry || ops->pud_entry || ops->pmd_entry || > - ops->pte_entry) > + if (is_hugepd(__hugepd(pgd_val(*pgd)))) > + err = walk_hugepd_range((hugepd_t *)pgd, addr, next, walk, PGDIR_SHIFT); > + else if (ops->p4d_entry || ops->pud_entry || ops->pmd_entry || ops->pte_entry) > err = walk_p4d_range(pgd, addr, next, walk); > if (err) > break; >
Hi Michael, Le 19/04/2021 à 12:47, Christophe Leroy a écrit : > Pagewalk ignores hugepd entries and walk down the tables > as if it was traditionnal entries, leading to crazy result. > > Add walk_hugepd_range() and use it to walk hugepage tables. I see you took patch 2 and 3 of the series. Do you expect Andrew to take patch 1 via mm tree, and then you'll take patch 4 once mm tree is merged ? Christophe > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > v2: > - Add a guard for NULL ops->pte_entry > - Take mm->page_table_lock when walking hugepage table, as suggested by follow_huge_pd() > --- > mm/pagewalk.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 53 insertions(+), 5 deletions(-) > > diff --git a/mm/pagewalk.c b/mm/pagewalk.c > index e81640d9f177..9b3db11a4d1d 100644 > --- a/mm/pagewalk.c > +++ b/mm/pagewalk.c > @@ -58,6 +58,45 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end, > return err; > } > > +#ifdef CONFIG_ARCH_HAS_HUGEPD > +static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr, > + unsigned long end, struct mm_walk *walk, int pdshift) > +{ > + int err = 0; > + const struct mm_walk_ops *ops = walk->ops; > + int shift = hugepd_shift(*phpd); > + int page_size = 1 << shift; > + > + if (!ops->pte_entry) > + return 0; > + > + if (addr & (page_size - 1)) > + return 0; > + > + for (;;) { > + pte_t *pte; > + > + spin_lock(&walk->mm->page_table_lock); > + pte = hugepte_offset(*phpd, addr, pdshift); > + err = ops->pte_entry(pte, addr, addr + page_size, walk); > + spin_unlock(&walk->mm->page_table_lock); > + > + if (err) > + break; > + if (addr >= end - page_size) > + break; > + addr += page_size; > + } > + return err; > +} > +#else > +static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr, > + unsigned long end, struct mm_walk *walk, int pdshift) > +{ > + return 0; > +} > +#endif > + > static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end, > struct mm_walk *walk) > { > @@ -108,7 +147,10 @@ static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end, > goto again; > } > > - err = walk_pte_range(pmd, addr, next, walk); > + if (is_hugepd(__hugepd(pmd_val(*pmd)))) > + err = walk_hugepd_range((hugepd_t *)pmd, addr, next, walk, PMD_SHIFT); > + else > + err = walk_pte_range(pmd, addr, next, walk); > if (err) > break; > } while (pmd++, addr = next, addr != end); > @@ -157,7 +199,10 @@ static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end, > if (pud_none(*pud)) > goto again; > > - err = walk_pmd_range(pud, addr, next, walk); > + if (is_hugepd(__hugepd(pud_val(*pud)))) > + err = walk_hugepd_range((hugepd_t *)pud, addr, next, walk, PUD_SHIFT); > + else > + err = walk_pmd_range(pud, addr, next, walk); > if (err) > break; > } while (pud++, addr = next, addr != end); > @@ -189,7 +234,9 @@ static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end, > if (err) > break; > } > - if (ops->pud_entry || ops->pmd_entry || ops->pte_entry) > + if (is_hugepd(__hugepd(p4d_val(*p4d)))) > + err = walk_hugepd_range((hugepd_t *)p4d, addr, next, walk, P4D_SHIFT); > + else if (ops->pud_entry || ops->pmd_entry || ops->pte_entry) > err = walk_pud_range(p4d, addr, next, walk); > if (err) > break; > @@ -224,8 +271,9 @@ static int walk_pgd_range(unsigned long addr, unsigned long end, > if (err) > break; > } > - if (ops->p4d_entry || ops->pud_entry || ops->pmd_entry || > - ops->pte_entry) > + if (is_hugepd(__hugepd(pgd_val(*pgd)))) > + err = walk_hugepd_range((hugepd_t *)pgd, addr, next, walk, PGDIR_SHIFT); > + else if (ops->p4d_entry || ops->pud_entry || ops->pmd_entry || ops->pte_entry) > err = walk_p4d_range(pgd, addr, next, walk); > if (err) > break; >
Christophe Leroy <christophe.leroy@csgroup.eu> writes: > Hi Michael, > > Le 19/04/2021 à 12:47, Christophe Leroy a écrit : >> Pagewalk ignores hugepd entries and walk down the tables >> as if it was traditionnal entries, leading to crazy result. >> >> Add walk_hugepd_range() and use it to walk hugepage tables. > > I see you took patch 2 and 3 of the series. Yeah I decided those were bug fixes so could be taken separately. > Do you expect Andrew to take patch 1 via mm tree, and then you'll take > patch 4 once mm tree is merged ? I didn't feel I could take patch 1 via the powerpc tree without risking conflicts. Andrew could take patch 1 and 4 via mm, though he might not want to pick them up this late. I guess step one would be to repost 1 and 4 as a new series. Either they can go via mm, or for 5.15 I could probably take them both as long as I pick them up early enough. cheers
Le 25/06/2021 à 06:45, Michael Ellerman a écrit : > Christophe Leroy <christophe.leroy@csgroup.eu> writes: >> Hi Michael, >> >> Le 19/04/2021 à 12:47, Christophe Leroy a écrit : >>> Pagewalk ignores hugepd entries and walk down the tables >>> as if it was traditionnal entries, leading to crazy result. >>> >>> Add walk_hugepd_range() and use it to walk hugepage tables. >> >> I see you took patch 2 and 3 of the series. > > Yeah I decided those were bug fixes so could be taken separately. > >> Do you expect Andrew to take patch 1 via mm tree, and then you'll take >> patch 4 once mm tree is merged ? > > I didn't feel I could take patch 1 via the powerpc tree without risking > conflicts. > > Andrew could take patch 1 and 4 via mm, though he might not want to pick > them up this late. Patch 4 needs patches 2 and 3 and doesn't apply without them so it is not that easy. Maybe Andrew you can take patch 1 now and then Michael you can take patch 4 at anytime during 5.15 preparation without any conflict risk ? > > I guess step one would be to repost 1 and 4 as a new series. Either they > can go via mm, or for 5.15 I could probably take them both as long as I > pick them up early enough. > I'll first repost patch 1 as standalone and see what happens. Christophe
Christophe Leroy <christophe.leroy@csgroup.eu> writes: > Le 25/06/2021 à 06:45, Michael Ellerman a écrit : >> Christophe Leroy <christophe.leroy@csgroup.eu> writes: >>> Hi Michael, >>> >>> Le 19/04/2021 à 12:47, Christophe Leroy a écrit : >>>> Pagewalk ignores hugepd entries and walk down the tables >>>> as if it was traditionnal entries, leading to crazy result. >>>> >>>> Add walk_hugepd_range() and use it to walk hugepage tables. >>> >>> I see you took patch 2 and 3 of the series. >> >> Yeah I decided those were bug fixes so could be taken separately. >> >>> Do you expect Andrew to take patch 1 via mm tree, and then you'll take >>> patch 4 once mm tree is merged ? >> >> I didn't feel I could take patch 1 via the powerpc tree without risking >> conflicts. >> >> Andrew could take patch 1 and 4 via mm, though he might not want to pick >> them up this late. > > Patch 4 needs patches 2 and 3 and doesn't apply without them so it is not that easy. Ah duh, sorry. > Maybe Andrew you can take patch 1 now and then Michael you can take patch 4 at anytime during 5.15 > preparation without any conflict risk ? Yeah that would work. >> I guess step one would be to repost 1 and 4 as a new series. Either they >> can go via mm, or for 5.15 I could probably take them both as long as I >> pick them up early enough. >> > > I'll first repost patch 1 as standalone and see what happens. Thanks. cheers
diff --git a/mm/pagewalk.c b/mm/pagewalk.c index e81640d9f177..9b3db11a4d1d 100644 --- a/mm/pagewalk.c +++ b/mm/pagewalk.c @@ -58,6 +58,45 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end, return err; } +#ifdef CONFIG_ARCH_HAS_HUGEPD +static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr, + unsigned long end, struct mm_walk *walk, int pdshift) +{ + int err = 0; + const struct mm_walk_ops *ops = walk->ops; + int shift = hugepd_shift(*phpd); + int page_size = 1 << shift; + + if (!ops->pte_entry) + return 0; + + if (addr & (page_size - 1)) + return 0; + + for (;;) { + pte_t *pte; + + spin_lock(&walk->mm->page_table_lock); + pte = hugepte_offset(*phpd, addr, pdshift); + err = ops->pte_entry(pte, addr, addr + page_size, walk); + spin_unlock(&walk->mm->page_table_lock); + + if (err) + break; + if (addr >= end - page_size) + break; + addr += page_size; + } + return err; +} +#else +static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr, + unsigned long end, struct mm_walk *walk, int pdshift) +{ + return 0; +} +#endif + static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end, struct mm_walk *walk) { @@ -108,7 +147,10 @@ static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end, goto again; } - err = walk_pte_range(pmd, addr, next, walk); + if (is_hugepd(__hugepd(pmd_val(*pmd)))) + err = walk_hugepd_range((hugepd_t *)pmd, addr, next, walk, PMD_SHIFT); + else + err = walk_pte_range(pmd, addr, next, walk); if (err) break; } while (pmd++, addr = next, addr != end); @@ -157,7 +199,10 @@ static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end, if (pud_none(*pud)) goto again; - err = walk_pmd_range(pud, addr, next, walk); + if (is_hugepd(__hugepd(pud_val(*pud)))) + err = walk_hugepd_range((hugepd_t *)pud, addr, next, walk, PUD_SHIFT); + else + err = walk_pmd_range(pud, addr, next, walk); if (err) break; } while (pud++, addr = next, addr != end); @@ -189,7 +234,9 @@ static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end, if (err) break; } - if (ops->pud_entry || ops->pmd_entry || ops->pte_entry) + if (is_hugepd(__hugepd(p4d_val(*p4d)))) + err = walk_hugepd_range((hugepd_t *)p4d, addr, next, walk, P4D_SHIFT); + else if (ops->pud_entry || ops->pmd_entry || ops->pte_entry) err = walk_pud_range(p4d, addr, next, walk); if (err) break; @@ -224,8 +271,9 @@ static int walk_pgd_range(unsigned long addr, unsigned long end, if (err) break; } - if (ops->p4d_entry || ops->pud_entry || ops->pmd_entry || - ops->pte_entry) + if (is_hugepd(__hugepd(pgd_val(*pgd)))) + err = walk_hugepd_range((hugepd_t *)pgd, addr, next, walk, PGDIR_SHIFT); + else if (ops->p4d_entry || ops->pud_entry || ops->pmd_entry || ops->pte_entry) err = walk_p4d_range(pgd, addr, next, walk); if (err) break;
Pagewalk ignores hugepd entries and walk down the tables as if it was traditionnal entries, leading to crazy result. Add walk_hugepd_range() and use it to walk hugepage tables. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- v2: - Add a guard for NULL ops->pte_entry - Take mm->page_table_lock when walking hugepage table, as suggested by follow_huge_pd() --- mm/pagewalk.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 5 deletions(-)