RE: [PATCH] md/bitmap: wait for bitmap writes to complete during the tear down sequence

[Sudhakar Panneerselvam <sudhakar.panneerselvam@oracle.com>]

> -----Original Message-----
> From: heming.zhao@suse.com [mailto:heming.zhao@suse.com]
> Sent: Monday, April 12, 2021 3:59 AM
> To: Sudhakar Panneerselvam <sudhakar.panneerselvam@oracle.com>; Song Liu <song@kernel.org>
> Cc: linux-raid@vger.kernel.org; lidong.zhong@suse.com; xni@redhat.com; colyli@suse.com; Martin Petersen
> <martin.petersen@oracle.com>
> 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:
> >>> 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.
> 

Hello Heming, Song,

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

Thanks
Sudhakar