Re: [Qemu-devel] [PATCH 18/41] block-migration: add lock

[Juan Quintela]

Paolo Bonzini <address@hidden> wrote:
> Some state is shared between the block migration code and its AIO
> callbacks.  Once block migration will run outside the iothread,
> the block migration code and the AIO callbacks will be able to
> run concurrently.  Protect the critical sections with a separate
> lock.  Do the same for completed_sectors, which can be used from
> the monitor.
>
> Signed-off-by: Paolo Bonzini <address@hidden>

Reviewed-by: Juan Quintela <address@hidden>


For this idiom:

>      for (sector = bmds->cur_dirty; sector < bmds->total_sectors;) {
> +        blk_mig_lock();
>          if (bmds_aio_inflight(bmds, sector)) {
> +            blk_mig_unlock();
>              bdrv_drain_all();
> +        } else {
> +            blk_mig_unlock();
>          }
>          if (bdrv_get_dirty(bmds->bs, sector)) {
>  

I find easier to understand to do:

     for (sector = bmds->cur_dirty; sector < bmds->total_sectors;) {
        bool drain;
        blk_mig_lock();
        drain = bmds_aio_inflight(bmds, sector)) {
        blk_mig_unlock();
        if (drain)
            bdrv_drain_all();
        }
        if (bdrv_get_dirty(bmds->bs, sector)) {


But it is up-to-you if you have to resend the series.
> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
> index 96a194b..10becb6 100644
> --- a/include/qemu/atomic.h
> +++ b/include/qemu/atomic.h
> @@ -16,6 +16,7 @@
>   */
>  #define smp_wmb()   barrier()
>  #define smp_rmb()   barrier()
> +
>  /*
>   * We use GCC builtin if it's available, as that can use
>   * mfence on 32 bit as well, e.g. if built with -march=pentium-m.

This hunk obviously don't belong to this commit O:-)