On Fri, Oct 12, 2018 at 05:58:40PM +0100, Anton Ivanov wrote:
[...]
> > > > > > If I read the code right, MIPS completely ignores the address
> > > > > > argument so
> > > > > > set_pmd_at there may not have the effect which this patch is trying to
> > > > > > achieve.
> > > > > Ignoring address is fine. Most architectures do that..
> > > > > The ideas is to move page table to the new pmd slot. It's nothing to do
> > > > > with the address passed to set_pmd_at().
> > > > If that is it's only function, then I am going to appropriate the code
> > > > out of the MIPS tree for further uml testing. It does exactly that -
> > > > just move the pmd the new slot.
> > > >
> > > > A.
> > >
> > > A.
> > >
> > > From ac265d96897a346b05646fce91784ed4922c7f8d Mon Sep 17 00:00:00 2001
> > > From: Anton Ivanov <anton.ivanov at cambridgegreys.com>
> > > Date: Fri, 12 Oct 2018 17:24:10 +0100
> > > Subject: [PATCH] Incremental fixes to the mmremap patch
> > >
> > > Signed-off-by: Anton Ivanov <anton.ivanov at cambridgegreys.com>
> > > ---
> > > arch/um/include/asm/pgalloc.h | 4 ++--
> > > arch/um/include/asm/pgtable.h | 3 +++
> > > arch/um/kernel/tlb.c | 6 ++++++
> > > 3 files changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/um/include/asm/pgalloc.h b/arch/um/include/asm/pgalloc.h
> > > index bf90b2aa2002..99eb5682792a 100644
> > > --- a/arch/um/include/asm/pgalloc.h
> > > +++ b/arch/um/include/asm/pgalloc.h
> > > @@ -25,8 +25,8 @@
> > > extern pgd_t *pgd_alloc(struct mm_struct *);
> > > extern void pgd_free(struct mm_struct *mm, pgd_t *pgd);
> > > -extern pte_t *pte_alloc_one_kernel(struct mm_struct *, unsigned long);
> > > -extern pgtable_t pte_alloc_one(struct mm_struct *, unsigned long);
> > > +extern pte_t *pte_alloc_one_kernel(struct mm_struct *);
> > > +extern pgtable_t pte_alloc_one(struct mm_struct *);
> > If its Ok, let me handle this bit since otherwise it complicates things for
> > me.
> >
> > > static inline void pte_free_kernel(struct mm_struct *mm, pte_t *pte)
> > > {
> > > 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 believe this should be included in a separate patch since it is not related
> > specifically to pte_alloc argument removal. If you want, I could split it
> > into a separate patch for my series with you as author.
>
>
> Whichever is more convenient for you.
Ok.
> One thing to note - tlb flush is extremely expensive on uml.
>
> I have lifted the definition of set_pmd_at from the mips tree and removed
> the tlb_flush_all from it for this exact reason.
>
> If I read the original patch correctly, it does its own flush control so
> set_pmd_at does not need to do a force flush every time. It is done further
> up the chain.
That is correct. It is not done during the optimization, but is done later
after the pmds have moved.
thanks,
- Joel
