Re: [Qemu-devel] [PATCH v3 10/12] Dump: add qmp command "query-dump"

[Peter Xu]

On Tue, Dec 01, 2015 at 10:54:48AM +0100, Paolo Bonzini wrote:
> 
> 
> On 01/12/2015 04:57, Peter Xu wrote:
> >> > You need a mutex around the reads of ->status and ->written_size.
> > Could I avoid using mutex here? Let me try to explain what I
> > thought.
> > 
> > The concurrency of this should only happen when:
> > 
> > - detached dump thread is working (dump thread)
> > - user queries dump status (main thread)
> > 
> > What the dump thread is doing should be something like:
> > 
> > - [start dumping]
> > - inc written_size
> > - inc written_size
> > - ...
> > - inc written_size
> > - set ->status to COMPLETED|FAILED
> > - [end dumping]
> 
> Yes, it's possible but you need to use atomic_mb_read/atomic_mb_set to
> write ->status.  Otherwise a CPU can see the write to ->status before
> some of the final writes to ->written_size.

Hi, Paolo,

Thanks to point out. However, would it be confusing to use
atomic_mb_{read|set} rather than directly use smp_rmb() and
smp_wmb()? Like:

In dump thread:

- inc written_size
- inc written_size
- ...
- inc written_size
- smp_wmb()
- atomic_set(status, COMPLETED|FAILED)

In main thread:

- atomic_read(status)
- smp_rmb()
- read written_size

What I understand from the doc (seems written by you, thanks :) ) is
that: atomic_mb_{read|set} is the pair of helper functions for _one_
specific variable, to make sure its operations are always in order
as long as we are using atomic_mb_* functions to access it all the
time. However, in the dump thread case, it's related to read/write
order of two variables (status and written_size).

Thanks!
Peter

> 
> Paolo