26.05.2021 17:58, Paolo Bonzini wrote:
On 25/05/21 12:25, Vladimir Sementsov-Ogievskiy wrote:
Next, even if we take bitmaps lock in bdrv_dirty_bitmap_next_dirty_area() or
around it, it doesn't bring thread-safety to block_copy_task_create():
The simplest solution here seems to protect bdrv_dirty_bitmap_next_dirty_area
and also bdrv_reset_dirty_bitmap with the tasks lock, so that once it is
released the bitmap is updated accordingly and from my understanding no other
task can get that region.
Yes, we just need to protect larger areas by outer lock, to protect the logic,
not only structures itself.
Locks protects data, not code; code must ensure invariants are respected at the
end of critical sections. Here we have both problems:
Hmm. Anyway, if code doesn't work with data that is potentially shared with
other threads, it doesn't need any protection. So, I agree.. I just wanted to
say that, if we have two data structures A and B, each protected by own lock,
it doesn't mean that our code is thread-safe. In your terminology we can say
that the whole data (which is a combination of A and B) is not protected by
partial locks, we need another lock to protect the combination.
- it's protecting too little data (the bitmap is not protected). This is a
block/dirty-bitmap.c bug.
- it's not respecting the invariant that tasks always reflected a superset of
what is set in the dirty bitmap. This is solved by making the critical section
larger.
--
Best regards,
Vladimir
