mbox series

[-next,v2,0/7] md: make rdev addition and removal independent from daemon thread

Message ID 20230815030957.509535-1-yukuai1@huaweicloud.com
Headers
Series md: make rdev addition and removal independent from daemon thread |

Message

Yu Kuai Aug. 15, 2023, 3:09 a.m. UTC 
  From: Yu Kuai <yukuai3@huawei.com>

Changes in v2:
 - remove patch 1 from v1 and some related patches, those patches will
 be sent later when rcu protection for rdev is removed.
 - add patch 2.

This is the third patchset to do some preparatory work to synchronize
io with array reconfiguration.

1) The first patchset refactor 'active_io', make sure that mddev_suspend()
will wait for io to be done. [1]

2) The second patchset remove 'quiesce' callback from mddev_suspend(), so
that mddev_suspend() doesn't rely on 'quiesce' callback is registered,
and can be used for all personalites; [2]

3) This patchset make array reconfiguration independent from daemon thread,
and synchronize it with io will be much easier because io may rely on
daemon thread to be done.

More patchset on the way!

Yu Kuai (7):
  md: use separate work_struct for md_start_sync()
  md: factor out a helper to choose sync direction from
    md_check_recovery()
  md: delay choosing sync direction to md_start_sync()
  md: factor out a helper rdev_removeable() from remove_and_add_spares()
  md: factor out a helper rdev_is_spare() from remove_and_add_spares()
  md: factor out a helper rdev_addable() from remove_and_add_spares()
  md: delay remove_and_add_spares() for read only array to
    md_start_sync()

 drivers/md/md.c | 257 +++++++++++++++++++++++++++++-------------------
 drivers/md/md.h |   5 +-
 2 files changed, 160 insertions(+), 102 deletions(-)

Comments

Song Liu Aug. 15, 2023, 3:54 p.m. UTC | #1 
On Tue, Aug 15, 2023 at 2:00 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
[...]
> > +
> > +not_running:
> > +     clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
> > +     clear_bit(MD_RECOVERY_RESHAPE, &mddev->recovery);
> > +     clear_bit(MD_RECOVERY_REQUESTED, &mddev->recovery);
> > +     clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
> > +     clear_bit(MD_RECOVERY_RUNNING, &mddev->recovery);
> > +     mddev_unlock(mddev);
> > +
> > +     wake_up(&resync_wait);
> > +     if (test_and_clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery) &&
> > +         mddev->sysfs_action)
> > +             sysfs_notify_dirent_safe(mddev->sysfs_action);
> >   }
> >
> >   /*
> > @@ -9379,7 +9402,6 @@ void md_check_recovery(struct mddev *mddev)
> >               return;
> >
> >       if (mddev_trylock(mddev)) {
> > -             int spares = 0;
> >               bool try_set_sync = mddev->safemode != 0;
> >
> >               if (!mddev->external && mddev->safemode == 1)
> > @@ -9467,29 +9489,11 @@ void md_check_recovery(struct mddev *mddev)
> >               clear_bit(MD_RECOVERY_DONE, &mddev->recovery);
> >
> >               if (!test_and_clear_bit(MD_RECOVERY_NEEDED, &mddev->recovery) ||
> > -                 test_bit(MD_RECOVERY_FROZEN, &mddev->recovery))
> > -                     goto not_running;
> > -             if (!md_choose_sync_direction(mddev, &spares))
> > -                     goto not_running;
> > -             if (mddev->pers->sync_request) {
> > -                     if (spares) {
> > -                             /* We are adding a device or devices to an array
> > -                              * which has the bitmap stored on all devices.
> > -                              * So make sure all bitmap pages get written
> > -                              */
> > -                             md_bitmap_write_all(mddev->bitmap);
> > -                     }
> > +                 test_bit(MD_RECOVERY_FROZEN, &mddev->recovery)) {
>
> Sorry that I made a mistake here while rebasing v2, here should be
>
> !test_bit(MD_RECOVERY_FROZEN, &mddev->recovery)
>
> With this fixed, there are no new regression for mdadm tests using loop
> devicein my VM.

                if (!test_and_clear_bit(MD_RECOVERY_NEEDED, &mddev->recovery) ||
                    !test_bit(MD_RECOVERY_FROZEN, &mddev->recovery)) {
                        queue_work(md_misc_wq, &mddev->sync_work);
                } else {

This doesn't look right. Should we do

                if (test_and_clear_bit(MD_RECOVERY_NEEDED, &mddev->recovery) &&
                    !test_bit(MD_RECOVERY_FROZEN, &mddev->recovery)) {
                        queue_work(md_misc_wq, &mddev->sync_work);
                } else {

instead?

Thanks,
Song