* Peter Xu ([email protected]) wrote:
> On Tue, Oct 04, 2022 at 05:59:36PM +0100, Dr. David Alan Gilbert wrote:
> > * Peter Xu ([email protected]) wrote:
> > > To prepare for thread-safety on page accountings, at least below counters
> > > need to be accessed only atomically, they are:
> > > 
> > >         ram_counters.transferred
> > >         ram_counters.duplicate
> > >         ram_counters.normal
> > >         ram_counters.postcopy_bytes
> > > 
> > > There are a lot of other counters but they won't be accessed outside
> > > migration thread, then they're still safe to be accessed without atomic
> > > ops.
> > > 
> > > Signed-off-by: Peter Xu <[email protected]>
> > 
> > I think this is OK; I'm not sure whether the memset 0's of ram_counters
> > technically need changing.
> 
> IMHO they're fine - what we need there should be thing like WRITE_ONCE()
> just to make sure no register caches (actually atomic_write() is normally
> implemented with WRITE_ONCE afaik).  But I think that's already guaranteed
> by memset() as the function call does, so we should be 100% safe.

I agree you're probably OK.

> > I'd love to put a comment somewhere saying these fields need to be
> > atomically read, but their qapi defined so I don't think we can.
> 
> How about I add a comment above ram_counters declarations in ram.c?

Yeh.

> > 
> > Finally, we probably need to check these are happy on 32 bit builds,
> > sometimes it's a bit funny with atomic adds.
> 
> Yeah.. I hope using qatomic_*() APIs can help me avoid any issues.  Or
> anything concerning?  I'd be happy to test on specific things if there are.

I just remember hitting problems in the past; especially if we end up
with trying to do a 64 bit atomic on a platofmr that can only do 32???

Dave

> > 
> > 
> > Reviewed-by: Dr. David Alan Gilbert <[email protected]>
> 
> Thanks!
> 
> -- 
> Peter Xu
> 
-- 
Dr. David Alan Gilbert / [email protected] / Manchester, UK


Reply via email to