On Sat, Nov 05 2016, Shaohua Li wrote: > On Fri, Nov 04, 2016 at 04:46:03PM +1100, Neil Brown wrote: >> As we don't wait for writes to complete in bitmap_daemon_work, they >> could still be in-flight when bitmap_unplug writes again. Or when >> bitmap_daemon_work tries to write again. >> This can be confusing and could risk the wrong data being written last. > > Applied the first 3 patches, thanks! > > This one seems not completely solving the race condition. It's still possible > bitmap_daemon_work clears BITMAP_PAGE_NEEDWRITE but hasn't dispatch the IO yet, > bitmap_unplug then does nothing and thinks bitmap is updated to disk. Why don't > we add locking here? Thanks for the review! BITMAP_PAGE_NEEDWRITE is set for pages that need to be written out in order to clear bits that don't need to be set any more. There is never any urgency to do this. BITMAP_PAGE_DIRTY is set of pages that need to be written out in order to set bits representing regions that are about to be written to. These have to be flushed by bitmap_unplug(). Pages can have both bits set, in which case bitmap_daemon_work() will leave them for bitmap_unplug() to deal with. So if bitmap_daemon_work() clears BITMAP_PAGE_PENDING on a page, then it is a page that bitmap_unplug() doesn't need to wait for. Does that answer your concerns? Thanks, NeilBrown
Attachment:
signature.asc
Description: PGP signature