On 4/12/21 5:38 PM, Sudhakar Panneerselvam wrote:
In my opinion, using a special wait is more clear than calling general md_bitmap_wait_writes(). the md_bitmap_wait_writes makes people
feel bitmap module does repetitive clean job.
My idea like:
```
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 200c5d0f08bf..ea6fa5a2cb6b 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -1723,6 +1723,8 @@ void md_bitmap_flush(struct mddev *mddev)
bitmap->daemon_lastrun -= sleep;
md_bitmap_daemon_work(mddev);
md_bitmap_update_sb(bitmap);
+ if (mddev->bitmap_info.external)
+ md_super_wait(mddev);
Agreed. This looks much cleaner.
}
/*
@@ -1746,6 +1748,7 @@ void md_bitmap_free(struct bitmap *bitmap)
/* Shouldn't be needed - but just in case.... */
wait_event(bitmap->write_wait,
atomic_read(&bitmap->pending_writes) == 0);
+ wait_event(mddev->sb_wait, atomic_read(&mddev->pending_writes)==0);
I think this call looks redundant as this wait is already covered by md_update_sb() for non-external bitmaps and the proposed change in md_bitmap_flush() for external bitmaps. So, can we omit this change?
Yes, it's absolute redundant step, to add or remove this line is up to you.
I added this line for following the style of bitmap->pending_writes. The comment in this area also give a explanation.
