On Mon, Dec 16, 2019 at 04:47:36AM -0500, Michael S. Tsirkin wrote: > On Sun, Dec 15, 2019 at 12:33:02PM -0500, Peter Xu wrote: > > On Thu, Dec 12, 2019 at 01:08:14AM +0100, Paolo Bonzini wrote: > > > >>> What depends on what here? Looks suspicious ... > > > >> > > > >> Hmm, I think maybe it can be removed because the entry pointer > > > >> reference below should be an ordering constraint already? > > > > > > entry->xxx depends on ring->reset_index. > > > > Yes that's true, but... > > > > entry = &ring->dirty_gfns[ring->reset_index & (ring->size - 1)]; > > /* barrier? */ > > next_slot = READ_ONCE(entry->slot); > > next_offset = READ_ONCE(entry->offset); > > > > ... I think entry->xxx depends on entry first, then entry depends on > > reset_index. So it seems fine because all things have a dependency? > > Is reset_index changed from another thread then? > If yes then you want to read reset_index with READ_ONCE. > That includes a dependency barrier. There're a few readers, but only this function will change it (kvm_dirty_ring_reset). Thanks, -- Peter Xu