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