On 06/13/2018 04:22 AM, Paolo Bonzini wrote: > On 13/06/2018 00:52, Junaid Shahid wrote: >> sp = page_header(root); >> + >> + /* >> + * Even if another VCPU was marking the SP as unsync-ed >> + * simultaneously, any guest page table changes are not >> + * guaranteed to be visible anyway until this VCPU issues a TLB >> + * flush strictly after those changes are made. We only need to >> + * ensure that the other CPU sets these flags before any actual >> + * changes to the page tables are made. The comments in >> + * mmu_need_write_protect() describe what could go wrong if this >> + * requirement isn't satisfied. >> + */ >> + smp_rmb(); > This is actually an smp_read_barrier_depends(), which is possibly more > self-documenting. I can do the replacement myself. Actually, now that I think about it again, I don't think that any read barrier is needed here. I don't believe that the scenario that I described in the comments in this patch needs a read barrier (though please correct me if that isn't the case). The code path that I actually had in mind when putting the barrier here appears to already include a read barrier that I missed seeing earlier. In any case, it seems that smp_read_barrier_depends() is a no-op on x86. So we can just remove it altogether, unless you think that it would be useful to keep it just as documentation. Thanks, Junaid
