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

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

> > 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? 

> >
> >          /* release the bitmap file  */
> >          md_bitmap_file_unmap(&bitmap->storage);
> > ```
> 
> I like Heming's version better.
> 
> Hi Sudhakar,
> 
> Are you able to reproduce the issue? If so, could you please verify
> that Heming's
> change fixes the issue?

Hi Song, Heming,

Please review my comments above. I can try to reproduce the issue with the fix once we agree on the final fix.

Thanks
Sudhakar

> 
> Thanks,
> Song