Re: [Qemu-devel] [PATCH v9 03/13] block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap

[Vladimir Sementsov-Ogievskiy]

18.01.2018 11:43, Paolo Bonzini wrote:
> On 29/12/2017 02:31, Fam Zheng wrote:
> > > we have the following comment:
> > >
> > >      /* Writing to the list requires the BQL _and_ the dirty_bitmap_mutex.
> > >       * Reading from the list can be done with either the BQL or the
> > >       * dirty_bitmap_mutex.  Modifying a bitmap only requires
> > >       * dirty_bitmap_mutex. */
> > >      QemuMutex dirty_bitmap_mutex;
> > >
> > > (I don't understand well the logic, why is it so. Paolo introduced the lock,
> > > but didn't update some functions..)
> > >
> > > so, actually, here we need both BQL and mutex.
> > Yes, because of that comment my interpretion has been both "BQL and the mutex"
> > whereever we say "within bdrv_dirty_bitmap_lock..unlock", as to some other
> > places in this file.
> A bit late, but---no, "within bdrv_dirty_bitmap_lock..unlock" means it's
> the "modifying the bitmap" case.
>
> Most functions that looks at the list are "called with BQL taken".
> Functions that write to the list are "called with BQL taken" and call
> bdrv_dirty_bitmaps_lock/bdrv_dirty_bitmaps_unlock themselves.
>
> Paolo

Paolo, could you please explain about bitmap locking in more details? 
Why do we need mutexes? Why do we do not need them
on read from the bitmap, only on write?

I imaging the following cases for locks:

- mutex + BQL(do aio_context_acquire take it?)
- only mutex
- only BQL
- no locks

and following operation on bitmaps:

- r/w bitmaps list (i.e. change .next fields of bitmaps and head of the 
list)
- r/w BdrvDirtyBitmap fields
- r/w HBitmap fields
- r/w HBitmap levels (set/unset/read bits)

what is the relations between locking cases and operations and why?

Sorry if I'm asking stupid questions :(

--
Best regards,
Vladimir