Re: [PATCH] x86/mm: do not BUG_ON() on stall pgd entries

[Jerome Glisse <jglisse@redhat.com>]

On Thu, Jun 01, 2017 at 06:59:04AM -0700, Andy Lutomirski wrote:
> On Wed, May 31, 2017 at 8:03 AM, Jerome Glisse <jglisse@redhat.com> wrote:
> > Since af2cf278ef4f ("Don't remove PGD entries in remove_pagetable()")
> > we no longer cleanup stall pgd entries and thus the BUG_ON() inside
> > sync_global_pgds() is wrong.
> >
> > This patch remove the BUG_ON() and unconditionaly update stall pgd
> > entries.
> >
> > Signed-off-by: Jerome Glisse <jglisse@redhat.com>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > ---
> >  arch/x86/mm/init_64.c | 7 +------
> >  1 file changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> > index ff95fe8..36b9020 100644
> > --- a/arch/x86/mm/init_64.c
> > +++ b/arch/x86/mm/init_64.c
> > @@ -123,12 +123,7 @@ void sync_global_pgds(unsigned long start, unsigned long end)
> >                         pgt_lock = &pgd_page_get_mm(page)->page_table_lock;
> >                         spin_lock(pgt_lock);
> >
> > -                       if (!p4d_none(*p4d_ref) && !p4d_none(*p4d))
> > -                               BUG_ON(p4d_page_vaddr(*p4d)
> > -                                      != p4d_page_vaddr(*p4d_ref));
> > -
> > -                       if (p4d_none(*p4d))
> > -                               set_p4d(p4d, *p4d_ref);
> > +                       set_p4d(p4d, *p4d_ref);
> 
> If we have a mismatch in the vmalloc range, vmalloc_fault is going to
> screw up and we'll end up using incorrect page tables.
> 
> What's causing the mismatch?  If you're hitting this BUG in practice,
> I suspect we have a bug elsewhere.

No bug elsewhere, simply hotplug memory then hotremove same memory you
just hotplugged then hotplug it again and you will trigger this as on
the first hotplug we allocate p4d/pud for the struct pages area, then on
hot remove we free that memory and clear the p4d/pud in the mm_init pgd
but not in any of the other pgds. So at that point the next hotplug
will trigger the BUG because of stall entries from the first hotplug.

Maybe we can add a flag to differentiate between hotplug and vmalloc
(thought looking at virtual address range is a dead give away of vmalloc
versus hotplug) and only avoid BUG_ON and force overwritte for hotplug.

Cheers,
Jerome

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>