Re: [PATCH v2 5/5] x86/mm: Drop usage of __flush_tlb_all() in kernel_physical_mapping_init()

[Dan Williams]

On Sat, Dec 1, 2018 at 10:43 PM Sasha Levin <sashal@xxxxxxxxxx> wrote:
>
> On Fri, Nov 30, 2018 at 04:35:32PM -0800, Dan Williams wrote:
> >Commit f77084d96355 "x86/mm/pat: Disable preemption around
> >__flush_tlb_all()" addressed a case where __flush_tlb_all() is called
> >without preemption being disabled. It also left a warning to catch other
> >cases where preemption is not disabled. That warning triggers for the
> >memory hotplug path which is also used for persistent memory enabling:
> >
> > WARNING: CPU: 35 PID: 911 at ./arch/x86/include/asm/tlbflush.h:460
> > RIP: 0010:__flush_tlb_all+0x1b/0x3a
> > [..]
> > Call Trace:
> >  phys_pud_init+0x29c/0x2bb
> >  kernel_physical_mapping_init+0xfc/0x219
> >  init_memory_mapping+0x1a5/0x3b0
> >  arch_add_memory+0x2c/0x50
> >  devm_memremap_pages+0x3aa/0x610
> >  pmem_attach_disk+0x585/0x700 [nd_pmem]
> >
> >Andy wondered why a path that can sleep was using __flush_tlb_all() [1]
> >and Dave confirmed the expectation for TLB flush is for modifying /
> >invalidating existing pte entries, but not initial population [2]. Drop
> >the usage of __flush_tlb_all() in phys_{p4d,pud,pmd}_init() on the
> >expectation that this path is only ever populating empty entries for the
> >linear map. Note, at linear map teardown time there is a call to the
> >all-cpu flush_tlb_all() to invalidate the removed mappings.
> >
> >[1]: https://lore.kernel.org/lkml/9DFD717D-857D-493D-A606-B635D72BAC21@xxxxxxxxxxxxxx
> >[2]: https://lore.kernel.org/lkml/749919a4-cdb1-48a3-adb4-adb81a5fa0b5@xxxxxxxxx
> >
> >Fixes: f77084d96355 ("x86/mm/pat: Disable preemption around __flush_tlb_all()")
> >Cc: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> >Cc: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> >Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> >Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> >Cc: Borislav Petkov <bp@xxxxxxxxx>
> >Cc: <stable@xxxxxxxxxxxxxxx>
> >Reported-by: Andy Lutomirski <luto@xxxxxxxxxx>
> >Suggested-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> >Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
>
> Hi Dan,
>
> This patch on it's own doesn't apply to any of the stable trees, does it
> maybe depend on some of the previous patches in this series?

It does not strictly depend on them, but it does need to be rebased
without them. The minimum patch for -stable are these
__flush_tlb_all() removals, but without the
set_{pte,pmd,pud,p4d,pgd}_safe() conversion. It's also an option to
backport those helpers, and conversion but I'd defer to x86/mm folks
to make that call.