Re: xen: oops at atomic64_read_cx8+0x4

[Andrea Arcangeli <aarcange@redhat.com>]

On Thu, Jun 07, 2012 at 02:33:33AM -0500, Jonathan Nieder wrote:
> Sergio Gelato wrote[1]:
> 
> >                          That 3.4.1-1~experimental.1 build
> > (3.4-trunk-686-pae #1 SMP Wed Jun 6 15:11:31 UTC 2012 i686 GNU/Linux)
> > is even less well-behaved under Xen: I'm getting a kernel OOPS at
> > EIP: [<c1168e54>] atomic64_read_cx8+0x4/0xc SS:ESP e021:ca853c6c
> > The top of the trace message unfortunately scrolled off the console before I
> > could see it, and the message doesn't have time to make it to syslog (either
> > local or remote).
> [...]
> > Non-Xen boots proceed normally.
> 
> Yeah, apparently[2] that's caused by
> 
> 	commit 26c191788f18
> 	Author: Andrea Arcangeli <aarcange@redhat.com>
> 	Date:   Tue May 29 15:06:49 2012 -0700
> 
> 	    mm: pmd_read_atomic: fix 32bit PAE pmd walk vs pmd_populate SMP race condition
> 
> which was also included in Debian kernel 3.2.19-1.
> 
> [1] http://bugs.debian.org/676360
> [2] https://bugzilla.redhat.com/show_bug.cgi?id=829016#c4

Oops, sorry I didn't imagine atomic64_read on a pmd would trip.

Unfortunately to support pagetable walking with mmap_sem hold for
reading, we need an atomic read on 32bit PAE if
CONFIG_TRANSPARENT_HUGEPAGE=y.

The only case requiring this is 32bit PAE with
CONFIG_TRANSPARENT_HUGEPAGE=y at build time. If you set
CONFIG_TRANSPARENT_HUGEPAGE=n temporarily you should be able to work
around this as I optimized the code in a way to avoid an expensive
cmpxchg8b.

I guess if Xen can't be updated to handle an atomic64_read on a pmd in
the guest, we can add a pmd_read paravirt op? Or if we don't want to
break the paravirt interface a loop like gup_fast with irq disabled
should also work but looping + local_irq_disable()/enable() sounded
worse and more complex than a atomic64_read (gup fast already disables
irqs because it doesn't hold the mmap_sem so it's a different cost
looping there). AFIK Xen disables THP during boot, so a check on THP
being enabled and falling back in the THP=n version of
pmd_read_atomic, would also be safe, but it's not so nice to do it
with a runtime check.

Thanks,
Andrea