On 4/12/21 5:38 PM, Sudhakar Panneerselvam wrote:

quoted

quoted

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.

quoted

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.

quoted

quoted

   }

   /*

@@ -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.