linux-kernel - Re: [PATCH -next v3 03/25] md: add new helpers to suspend/resume array

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPhsuW4tQ9A+ddpxGhfza3WSdkL+FXmS-4NLUUN7MNk0TOY0WA@mail.gmail.com>
Date:   Thu, 28 Sep 2023 11:45:48 -0700
From:   Song Liu <song@...nel.org>
To:     Yu Kuai <yukuai1@...weicloud.com>
Cc:     xni@...hat.com, agk@...hat.com, snitzer@...nel.org,
        dm-devel@...hat.com, linux-kernel@...r.kernel.org,
        linux-raid@...r.kernel.org, yukuai3@...wei.com,
        yi.zhang@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH -next v3 03/25] md: add new helpers to suspend/resume array

On Wed, Sep 27, 2023 at 11:22 PM Yu Kuai <yukuai1@...weicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@...wei.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 changed;
>  - 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@...wei.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 == 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

[...]