On Tue, Mar 14, 2017 at 10:48:51AM -0700, Linus Torvalds wrote: > On Tue, Mar 14, 2017 at 12:47 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote: > > > > I've also applied the GUP patch, with the assumption that you'll address Linus's > > request to switch x86 over to the generic version. > > Note that switching over to the generic version is somewhat fraught > with subtle issues: > > (a) we need to make sure that x86 actually matches the required > semantics for the generic GUP. > > (b) we need to make sure the atomicity of the page table reads is ok. > > (c) need to verify the maximum VM address properly > > I _think_ (a) is ok. The code (and the config option name) talks about > freeing page tables using RCU, but in fact I don't think it relies on > it, and it's sufficient that it disables interrupts and that that will > block any IPI's. > > In contrast, I think (b) needs real work to make sure it's ok on > 32-bit PAE with 64-bit pte entries. The generic code currently just > does READ_ONCE(), while the x86 code does gup_get_pte(). + Andrea. Looking on gup_get_pte() makes me thinkg, why don't we need the same approach for pmd level (pud is not relevant for PAE)? Looks like a bug to me. We have pmd_read_atomic() to address the issue in other places. The helper doesn't match required for GUP_fast() semantics, but we clearly need to address the issue. pgd deference doesn't look good too on PAE. Or am I missing something? Heck, we don't even have READ_ONCE() on x86 for page table entry dereference. Looks like a bug waiting to explode. And not only on PAE. -- Kirill A. Shutemov