Message ID | 20181103040041.7085-1-joelaf@google.com (mailing list archive) |
---|---|
Headers | show |
Series | Add support for fast mremap | expand |
On 03/11/2018 09:15, Richard Weinberger wrote: > Joel, > > Am Samstag, 3. November 2018, 05:00:38 CET schrieb Joel Fernandes: >> Hi, >> Here is the latest "fast mremap" series. This just a repost with Kirill's >> Acked-bys added. I would like this to be considered for linux -next. I also >> dropped the CONFIG enablement patch for arm64 since I am yet to test it with >> the new TLB flushing code that is in very recent kernel releases. (None of my >> arm64 devices run mainline right now.) so I will post the arm64 enablement once >> I get to that. The performance numbers in the series are for x86. >> >> List of patches in series: >> >> (1) mm: select HAVE_MOVE_PMD in x86 for faster mremap >> >> (2) mm: speed up mremap by 20x on large regions (v4) >> v1->v2: Added support for per-arch enablement (Kirill Shutemov) >> v2->v3: Updated commit message to state the optimization may also >> run for non-thp type of systems (Daniel Col). >> v3->v4: Remove useless pmd_lock check (Kirill Shutemov) >> Rebased ontop of Linus's master, updated perf results based >> on x86 testing. Added Kirill's Acks. >> >> (3) mm: treewide: remove unused address argument from pte_alloc functions (v2) >> v1->v2: fix arch/um/ prototype which was missed in v1 (Anton Ivanov) >> update changelog with manual fixups for m68k and microblaze. >> >> not included - (4) mm: select HAVE_MOVE_PMD in arm64 for faster mremap >> This patch is dropped since last posting pending further performance >> testing on arm64 with new TLB gather updates. See notes in patch >> titled "mm: speed up mremap by 500x on large regions" for more >> details. >> > This breaks UML build: > CC mm/mremap.o > mm/mremap.c: In function ‘move_normal_pmd’: > mm/mremap.c:229:2: error: implicit declaration of function ‘set_pmd_at’; did you mean ‘set_pte_at’? [-Werror=implicit-function-declaration] > set_pmd_at(mm, new_addr, new_pmd, pmd); > ^~~~~~~~~~ > set_pte_at > CC crypto/rng.o > CC fs/direct-io.o > cc1: some warnings being treated as errors > > To test yourself, just run on a x86 box: > $ make defconfig ARCH=um > $ make linux ARCH=um > > Thanks, > //richard > > > UM somehow managed to miss one of the 3-level functions, I sent a patch at some point to add to the mmremap series, but it looks like it did not get included in the final version. You need these two incremental on top of Joel's patch. Richard - feel free to relocate the actual implementation of the set_pgd_at elsewhere - I put it at the end of tlb.c diff --git a/arch/um/include/asm/pgtable.h b/arch/um/include/asm/pgtable.h index 7485398d0737..1692da55e63a 100644 --- a/arch/um/include/asm/pgtable.h +++ b/arch/um/include/asm/pgtable.h @@ -359,4 +359,7 @@ do { \ __flush_tlb_one((vaddr)); \ } while (0) +extern void set_pmd_at(struct mm_struct *mm, unsigned long addr, + pmd_t *pmdp, pmd_t pmd); + #endif diff --git a/arch/um/kernel/tlb.c b/arch/um/kernel/tlb.c index 763d35bdda01..d17b74184ba0 100644 --- a/arch/um/kernel/tlb.c +++ b/arch/um/kernel/tlb.c @@ -647,3 +647,9 @@ void force_flush_all(void) vma = vma->vm_next; } } +void set_pmd_at(struct mm_struct *mm, unsigned long addr, + pmd_t *pmdp, pmd_t pmd) +{ + *pmdp = pmd; +} +
On Sat, Nov 03, 2018 at 09:24:05AM +0000, Anton Ivanov wrote: > On 03/11/2018 09:15, Richard Weinberger wrote: > > Joel, > > > > Am Samstag, 3. November 2018, 05:00:38 CET schrieb Joel Fernandes: > > > Hi, > > > Here is the latest "fast mremap" series. This just a repost with Kirill's > > > Acked-bys added. I would like this to be considered for linux -next. I also > > > dropped the CONFIG enablement patch for arm64 since I am yet to test it with > > > the new TLB flushing code that is in very recent kernel releases. (None of my > > > arm64 devices run mainline right now.) so I will post the arm64 enablement once > > > I get to that. The performance numbers in the series are for x86. > > > > > > List of patches in series: > > > > > > (1) mm: select HAVE_MOVE_PMD in x86 for faster mremap > > > > > > (2) mm: speed up mremap by 20x on large regions (v4) > > > v1->v2: Added support for per-arch enablement (Kirill Shutemov) > > > v2->v3: Updated commit message to state the optimization may also > > > run for non-thp type of systems (Daniel Col). > > > v3->v4: Remove useless pmd_lock check (Kirill Shutemov) > > > Rebased ontop of Linus's master, updated perf results based > > > on x86 testing. Added Kirill's Acks. > > > > > > (3) mm: treewide: remove unused address argument from pte_alloc functions (v2) > > > v1->v2: fix arch/um/ prototype which was missed in v1 (Anton Ivanov) > > > update changelog with manual fixups for m68k and microblaze. > > > > > > not included - (4) mm: select HAVE_MOVE_PMD in arm64 for faster mremap > > > This patch is dropped since last posting pending further performance > > > testing on arm64 with new TLB gather updates. See notes in patch > > > titled "mm: speed up mremap by 500x on large regions" for more > > > details. > > > > > This breaks UML build: > > CC mm/mremap.o > > mm/mremap.c: In function ‘move_normal_pmd’: > > mm/mremap.c:229:2: error: implicit declaration of function ‘set_pmd_at’; did you mean ‘set_pte_at’? [-Werror=implicit-function-declaration] > > set_pmd_at(mm, new_addr, new_pmd, pmd); > > ^~~~~~~~~~ > > set_pte_at > > CC crypto/rng.o > > CC fs/direct-io.o > > cc1: some warnings being treated as errors > > > > To test yourself, just run on a x86 box: > > $ make defconfig ARCH=um > > $ make linux ARCH=um > > > > Thanks, > > //richard > > > > > > > > UM somehow managed to miss one of the 3-level functions, I sent a patch at > some point to add to the mmremap series, but it looks like it did not get > included in the final version. > > You need these two incremental on top of Joel's patch. Richard - feel free > to relocate the actual implementation of the set_pgd_at elsewhere - I put it > at the end of tlb.c > > diff --git a/arch/um/include/asm/pgtable.h b/arch/um/include/asm/pgtable.h > index 7485398d0737..1692da55e63a 100644 > --- a/arch/um/include/asm/pgtable.h > +++ b/arch/um/include/asm/pgtable.h > @@ -359,4 +359,7 @@ do { \ > __flush_tlb_one((vaddr)); \ > } while (0) > > +extern void set_pmd_at(struct mm_struct *mm, unsigned long addr, > + pmd_t *pmdp, pmd_t pmd); > + > #endif > diff --git a/arch/um/kernel/tlb.c b/arch/um/kernel/tlb.c > index 763d35bdda01..d17b74184ba0 100644 > --- a/arch/um/kernel/tlb.c > +++ b/arch/um/kernel/tlb.c > @@ -647,3 +647,9 @@ void force_flush_all(void) > vma = vma->vm_next; > } > } > +void set_pmd_at(struct mm_struct *mm, unsigned long addr, > + pmd_t *pmdp, pmd_t pmd) > +{ > + *pmdp = pmd; > +} > + > I see it now: https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg140005.html Sorry about that. Actually the reason the suggestion got missed is it did not belong in the patch removing the pte_alloc address argument. The pte_alloc parts of the patch you proposed are infact already included. This set_pmd_at for UM should go into a separate patch and should not be rolled into any existing one. Could you send a proper patch adding this function with a commit message and everything? I can then include it as a separate patch of my series. thanks! - Joel
On Sat, Nov 03, 2018 at 09:24:05AM +0000, Anton Ivanov wrote: > On 03/11/2018 09:15, Richard Weinberger wrote: > > Joel, > > > > Am Samstag, 3. November 2018, 05:00:38 CET schrieb Joel Fernandes: > > > Hi, > > > Here is the latest "fast mremap" series. This just a repost with Kirill's > > > Acked-bys added. I would like this to be considered for linux -next. I also > > > dropped the CONFIG enablement patch for arm64 since I am yet to test it with > > > the new TLB flushing code that is in very recent kernel releases. (None of my > > > arm64 devices run mainline right now.) so I will post the arm64 enablement once > > > I get to that. The performance numbers in the series are for x86. > > > > > > List of patches in series: > > > > > > (1) mm: select HAVE_MOVE_PMD in x86 for faster mremap > > > > > > (2) mm: speed up mremap by 20x on large regions (v4) > > > v1->v2: Added support for per-arch enablement (Kirill Shutemov) > > > v2->v3: Updated commit message to state the optimization may also > > > run for non-thp type of systems (Daniel Col). > > > v3->v4: Remove useless pmd_lock check (Kirill Shutemov) > > > Rebased ontop of Linus's master, updated perf results based > > > on x86 testing. Added Kirill's Acks. > > > > > > (3) mm: treewide: remove unused address argument from pte_alloc functions (v2) > > > v1->v2: fix arch/um/ prototype which was missed in v1 (Anton Ivanov) > > > update changelog with manual fixups for m68k and microblaze. > > > > > > not included - (4) mm: select HAVE_MOVE_PMD in arm64 for faster mremap > > > This patch is dropped since last posting pending further performance > > > testing on arm64 with new TLB gather updates. See notes in patch > > > titled "mm: speed up mremap by 500x on large regions" for more > > > details. > > > > > This breaks UML build: > > CC mm/mremap.o > > mm/mremap.c: In function ‘move_normal_pmd’: > > mm/mremap.c:229:2: error: implicit declaration of function ‘set_pmd_at’; did you mean ‘set_pte_at’? [-Werror=implicit-function-declaration] > > set_pmd_at(mm, new_addr, new_pmd, pmd); > > ^~~~~~~~~~ > > set_pte_at > > CC crypto/rng.o > > CC fs/direct-io.o > > cc1: some warnings being treated as errors > > > > To test yourself, just run on a x86 box: > > $ make defconfig ARCH=um > > $ make linux ARCH=um > > > > Thanks, > > //richard > > > > > > > > UM somehow managed to miss one of the 3-level functions, I sent a patch at > some point to add to the mmremap series, but it looks like it did not get > included in the final version. > > You need these two incremental on top of Joel's patch. Richard - feel free > to relocate the actual implementation of the set_pgd_at elsewhere - I put it > at the end of tlb.c > > diff --git a/arch/um/include/asm/pgtable.h b/arch/um/include/asm/pgtable.h > index 7485398d0737..1692da55e63a 100644 > --- a/arch/um/include/asm/pgtable.h > +++ b/arch/um/include/asm/pgtable.h > @@ -359,4 +359,7 @@ do { \ > __flush_tlb_one((vaddr)); \ > } while (0) > > +extern void set_pmd_at(struct mm_struct *mm, unsigned long addr, > + pmd_t *pmdp, pmd_t pmd); > + > #endif > diff --git a/arch/um/kernel/tlb.c b/arch/um/kernel/tlb.c > index 763d35bdda01..d17b74184ba0 100644 > --- a/arch/um/kernel/tlb.c > +++ b/arch/um/kernel/tlb.c > @@ -647,3 +647,9 @@ void force_flush_all(void) > vma = vma->vm_next; > } > } > +void set_pmd_at(struct mm_struct *mm, unsigned long addr, > + pmd_t *pmdp, pmd_t pmd) > +{ > + *pmdp = pmd; > +} > + > Looks like more architectures don't define set_pmd_at. I am thinking the easiest way forward is to just do the following, instead of defining set_pmd_at for every architecture that doesn't care about it. Thoughts? diff --git a/mm/mremap.c b/mm/mremap.c index 7cf6b0943090..31ad64dcdae6 100644 --- a/mm/mremap.c +++ b/mm/mremap.c @@ -281,7 +281,8 @@ unsigned long move_page_tables(struct vm_area_struct *vma, split_huge_pmd(vma, old_pmd, old_addr); if (pmd_trans_unstable(old_pmd)) continue; - } else if (extent == PMD_SIZE && IS_ENABLED(CONFIG_HAVE_MOVE_PMD)) { + } else if (extent == PMD_SIZE) { +#ifdef CONFIG_HAVE_MOVE_PMD /* * If the extent is PMD-sized, try to speed the move by * moving at the PMD level if possible. @@ -296,6 +297,7 @@ unsigned long move_page_tables(struct vm_area_struct *vma, drop_rmap_locks(vma); if (moved) continue; +#endif } if (pte_alloc(new_vma->vm_mm, new_pmd))
> On Nov 3, 2018, at 12:32 PM, Joel Fernandes <joel@joelfernandes.org> wrote: > > Looks like more architectures don't define set_pmd_at. I am thinking the > easiest way forward is to just do the following, instead of defining > set_pmd_at for every architecture that doesn't care about it. Thoughts? > > diff --git a/mm/mremap.c b/mm/mremap.c > index 7cf6b0943090..31ad64dcdae6 100644 > --- a/mm/mremap.c > +++ b/mm/mremap.c > @@ -281,7 +281,8 @@ unsigned long move_page_tables(struct vm_area_struct *vma, > split_huge_pmd(vma, old_pmd, old_addr); > if (pmd_trans_unstable(old_pmd)) > continue; > - } else if (extent == PMD_SIZE && IS_ENABLED(CONFIG_HAVE_MOVE_PMD)) { > + } else if (extent == PMD_SIZE) { > +#ifdef CONFIG_HAVE_MOVE_PMD > /* > * If the extent is PMD-sized, try to speed the move by > * moving at the PMD level if possible. > @@ -296,6 +297,7 @@ unsigned long move_page_tables(struct vm_area_struct *vma, > drop_rmap_locks(vma); > if (moved) > continue; > +#endif > } > > if (pte_alloc(new_vma->vm_mm, new_pmd)) > That seems reasonable as there are going to be a lot of architectures that never have mappings at the PMD level. Have you thought about what might be needed to extend this paradigm to be able to perform remaps at the PUD level, given many architectures already support PUD-mapped pages? William Kucharski
On Sun, Nov 04, 2018 at 12:56:48AM -0600, William Kucharski wrote: > > > > On Nov 3, 2018, at 12:32 PM, Joel Fernandes <joel@joelfernandes.org> wrote: > > > > Looks like more architectures don't define set_pmd_at. I am thinking the > > easiest way forward is to just do the following, instead of defining > > set_pmd_at for every architecture that doesn't care about it. Thoughts? > > > > diff --git a/mm/mremap.c b/mm/mremap.c > > index 7cf6b0943090..31ad64dcdae6 100644 > > --- a/mm/mremap.c > > +++ b/mm/mremap.c > > @@ -281,7 +281,8 @@ unsigned long move_page_tables(struct vm_area_struct *vma, > > split_huge_pmd(vma, old_pmd, old_addr); > > if (pmd_trans_unstable(old_pmd)) > > continue; > > - } else if (extent == PMD_SIZE && IS_ENABLED(CONFIG_HAVE_MOVE_PMD)) { > > + } else if (extent == PMD_SIZE) { > > +#ifdef CONFIG_HAVE_MOVE_PMD > > /* > > * If the extent is PMD-sized, try to speed the move by > > * moving at the PMD level if possible. > > @@ -296,6 +297,7 @@ unsigned long move_page_tables(struct vm_area_struct *vma, > > drop_rmap_locks(vma); > > if (moved) > > continue; > > +#endif > > } > > > > if (pte_alloc(new_vma->vm_mm, new_pmd)) > > > > That seems reasonable as there are going to be a lot of architectures that never have > mappings at the PMD level. Ok, I will do it like this and resend. > Have you thought about what might be needed to extend this paradigm to be able to > perform remaps at the PUD level, given many architectures already support PUD-mapped > pages? > I have thought about this. I believe it is doable in the future. Off the top I don't see an issue doing it, and it will also reduce the number of flushes. thanks, - Joel