On 06/19/2018 10:58 PM, Paolo Bonzini wrote: > On 20/06/2018 00:42, Junaid Shahid wrote: >> 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. > > It's not entirely a no-op, it's (on x86) a compiler optimization barrier. > > In the comment above smp_wmb you should say what is the read barrier > that is "paired" with the smp_wmb. If there is none, that most likely > means that the smp_read_barrier_depends _is_ actually needed. > > Paolo > As best as I can tell, there isn't any explicit read barrier needed corresponding to the smp_wmb() in mmu_need_write_protect(). In the example scenario described there that necessitates the wmb(), the VM exit (that resulted in the call to kvm_mmu_sync_roots) would act as an implicit barrier that would order the sp->unsync read after whatever condition triggered the sync (e.g. a guest TLB flush). Even if smp_read_barrier_depends() acts as a compiler barrier, it wouldn't add anything there. (It is very well possible that I am missing something. Please do let me know if that is the case.) Thanks, Junaid
