18.01.2018 13:09, Paolo Bonzini wrote:
On 18/01/2018 10:55, Vladimir Sementsov-Ogievskiy wrote:
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, could you please explain about bitmap locking in more details?
Why do we need mutexes?
We have three cases:
1) monitor creates and destroy bitmaps.
2) monitor also has to read the list. We know it operates with BQL.
3) users such as mirror.c create a dirty bitmap in the monitor command
(under BQL), but they can operate without BQL in a separate iothread so
we create a separate lock (bitmap->mutex).
While in the second and third case, bitmaps cannot disappear. So in the
first case you operate with BQL+dirty bitmap mutex. The result is that
you lock out both the second and the third case while creating and
destroying bitmaps.
Why do we do not need them
on read from the bitmap, only on write?
Indeed, reading the bitmap also requires taking the lock. So
s/Modifying/Accessing/ in that comment.
Paolo
so,
/* 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. Accessing a bitmap only requires
* dirty_bitmap_mutex. */
QemuMutex dirty_bitmap_mutex;
so, accessing the bitmap needs mutex lock?
Then what do you mean under accessing the bitmap? Any touch of
BdrvDirtyBitmap fields? Then "reading the list" will require bitmap
mutex too.
Or accessing the bitmap is accessing any field except
BdrvDirtyBitmap.list? Then in (2), what do you mean? For example
query-block will go through
the list, but it touches other fields too, so it should lock mutex.