On Fri, 2019-09-20 at 17:48 -0700, John Hubbard wrote: > [...] > So it seems that full memory barriers (not just compiler barriers) are required. > If the irq enable/disable somehow provides that, then your new code just goes > along for the ride and Just Works. (You don't have any memory barriers in > start_lockless_pgtbl_walk() / end_lockless_pgtbl_walk(), just the compiler > barriers provided by the atomic inc/dec.) > > So it's really a pre-existing question about the correctness of the gup_fast() > irq disabling approach. I am not experienced in other archs, and I am still pretty new to Power, but by what I could understand, this behavior is better explained in serialize_against_pte_lookup. What happens here is that, before doing a THP split/collapse, the function does a update of the pmd and a serialize_against_pte_lookup, in order do avoid a invalid output on a lockless pagetable walk. Serialize basically runs a do_nothing in every cpu related to the process, and wait for it to return. This running depends on interrupt being enabled, so disabling it before gup_pgd_range() and re-enabling after the end, makes the THP split/collapse wait for gup_pgd_range() completion in every cpu before continuing. (here happens the lock) (As told before, every gup_pgd_range() that occurs after it uses a updated pmd, so no problem.) I am sure other archs may have a similar mechanism using local_irq_{disable,enable}. Did it answer your questions? Best regards, Leonardo Bras
Attachment:
signature.asc
Description: This is a digitally signed message part