-----Original Message-----
From: heming.zhao@suse.com [mailto:heming.zhao@suse.com]
Sent: Monday, April 12, 2021 3:59 AM
To: Sudhakar Panneerselvam <redacted>; Song Liu <song@kernel.org>
Cc: linux-raid@vger.kernel.org; lidong.zhong@suse.com; xni@redhat.com; colyli@suse.com; Martin Petersen
[off-list ref]
Subject: Re: [PATCH] md/bitmap: wait for bitmap writes to complete during the tear down sequence

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

quoted

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

quoted

quoted

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.

Hello Heming, Song,

I could not reproduce the issue with the revised fix. I will be submitting the revised patch shortly.

Thanks
Sudhakar