Re: [Qemu-devel] [PATCH 16/19] block: protect modification of dirty bitmaps with a mutex

[Vladimir Sementsov-Ogievskiy]

27.06.2017 16:58, Paolo Bonzini wrote:
> On 27/06/2017 15:51, Vladimir Sementsov-Ogievskiy wrote:
> > > > bdrv_create_dirty_bitmap(...)
> > > >
> > > > bdrv_dirty_bitmaps_lock(bs)
> > > >
> > > > bitmap  = bdrv_find_dirty_bitmap(bs, name)
> > > >
> > > > <some changes>
> > > >
> > > > bdrv_dirty_bitmaps_unlock(bs)
> > > >
> > > > - because we can't now trust the pointer returned by
> > > > bdrv_create_dirty_bitmap, as it releases bitmap lock before return.
> > > If you have the big QEMU lock (you do if you are in the QEMU monitor),
> > > you are protected from changes to the list of bitmaps.
> > >
> > > Paolo
> > but you wrote "Writing to the list requires the BQL _and_ the
> > dirty_bitmap_mutex".
> >
> > should it be BQL only?
> bdrv_dirty_bitmaps_lock/unlock is (should be) called by the functions
> that write to the list.  I hope I can post the patch today already.
>
> Paolo


I'm likely not right, but for me introducing this mutex looks like dirty 
bitmaps may be accessed from concurrent threads. So for me it looks 
strange that only accessors are protected by the mutex and not the whole 
transactions.

One example I've wrote above, other examples from code: are 
qmp_block_dirty_bitmap** functions:

bdrv_create_dirty_bitmap() {

   bdrv_find_dirty_bitmap()

   ....

bdrv_dirty_bitmaps_lock(bs);
   QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
   bdrv_dirty_bitmaps_unlock(bs);

}

- we protect inserting into list from other threads, but what prevent 
creating bitmap with the same name from other thread after 
bdrv_find_dirty_bitmap() and before bdrv_dirty_bitmaps_lock() ?


-----

qmp_block_dirty_bitmap_clear() {

    bitmap = block_dirty_bitmap_lookup()

....

    bdrv_clear_dirty_bitmap()

}

- bdrv_clear_dirty_bitmap is protected by lock, but what prevent 
deleting this bitmap by other thread between block_dirty_bitmap_lookup() 
and bdrv_clear_dirty_bitmap() ?


--- it is not the whole list ---

--
Best regards,
Vladimir