Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
MIME-Version: 1.0
References: <20230928061543.1845742-1-yukuai1@huaweicloud.com> <20230928061543.1845742-4-yukuai1@huaweicloud.com>
In-Reply-To: <20230928061543.1845742-4-yukuai1@huaweicloud.com>
From:   Song Liu <song@kernel.org>
Date:   Thu, 28 Sep 2023 11:45:48 -0700
Message-ID: <CAPhsuW4tQ9A+ddpxGhfza3WSdkL+FXmS-4NLUUN7MNk0TOY0WA@mail.gmail.com>
Subject: Re: [PATCH -next v3 03/25] md: add new helpers to suspend/resume array
To:     Yu Kuai <yukuai1@huaweicloud.com>
Cc:     xni@redhat.com, agk@redhat.com, snitzer@kernel.org,
        dm-devel@redhat.com, linux-kernel@vger.kernel.org,
        linux-raid@vger.kernel.org, yukuai3@huawei.com,
        yi.zhang@huawei.com, yangerkun@huawei.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Sep 27, 2023 at 11:22=E2=80=AFPM Yu Kuai <yukuai1@huaweicloud.com> =
wrote:
>
> From: Yu Kuai <yukuai3@huawei.com>
>
> Advantages for new apis:
>  - reconfig_mutex is not required;
>  - the weird logical that suspend array hold 'reconfig_mutex' for
>    mddev_check_recovery() to update superblock is not needed;
>  - the specail handling, 'pers->prepare_suspend', for raid456 is not
>    needed;
>  - It's safe to be called at any time once mddev is allocated, and it's
>    designed to be used from slow path where array configuration is change=
d;
>  - the new helpers is designed to be called before mddev_lock(), hence
>    it support to be interrupted by user as well.
>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  drivers/md/md.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/md/md.h |   3 ++
>  2 files changed, 103 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index e460b380143d..a075d03d03d3 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -443,12 +443,22 @@ void mddev_suspend(struct mddev *mddev)
>                         lockdep_is_held(&mddev->reconfig_mutex));
>
>         WARN_ON_ONCE(thread && current =3D=3D thread->tsk);
> -       if (mddev->suspended++)
> +
> +       /* can't concurrent with __mddev_suspend() and __mddev_resume() *=
/
> +       mutex_lock(&mddev->suspend_mutex);
> +       if (mddev->suspended++) {
> +               mutex_unlock(&mddev->suspend_mutex);
>                 return;

Can we make mddev->suspended atomic_t, and use atomic_inc_return()
here?

Thanks,
Song

[...]