Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230708091727.1417894-1-yukuai1@huaweicloud.com>
In-Reply-To: <20230708091727.1417894-1-yukuai1@huaweicloud.com>
From:   Song Liu <song@kernel.org>
Date:   Sat, 29 Jul 2023 18:44:48 +0800
Message-ID: <CAPhsuW4P9t6fhAg5EFvTVkf3WLQy0=NTpZ-+1fcYBn3uyn9bng@mail.gmail.com>
Subject: Re: [PATCH -next v3] md/raid5-cache: fix a deadlock in r5l_exit_log()
To:     Yu Kuai <yukuai1@huaweicloud.com>
Cc:     logang@deltatee.com, axboe@kernel.dk, linux-raid@vger.kernel.org,
        linux-kernel@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 Sat, Jul 8, 2023 at 5:19=E2=80=AFPM Yu Kuai <yukuai1@huaweicloud.com> wr=
ote:
>
> From: Yu Kuai <yukuai3@huawei.com>
>
> Commit b13015af94cf ("md/raid5-cache: Clear conf->log after finishing
> work") introduce a new problem:
>
> // caller hold reconfig_mutex
> r5l_exit_log
>  flush_work(&log->disable_writeback_work)
>                         r5c_disable_writeback_async
>                          wait_event
>                           /*
>                            * conf->log is not NULL, and mddev_trylock()
>                            * will fail, wait_event() can never pass.
>                            */
>  conf->log =3D NULL
>
> Fix this problem by setting 'config->log' to NULL before wake_up() as it
> used to be, so that wait_event() from r5c_disable_writeback_async() can
> exist. In the meantime, move forward md_unregister_thread() so that
> null-ptr-deref this commit fixed can still be fixed.
>
> Fixes: b13015af94cf ("md/raid5-cache: Clear conf->log after finishing wor=
k")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>

Applied to md-next. Thanks!

Song

> ---
>
> Changes in v3:
>  - Use a different solution.
>
>  drivers/md/raid5-cache.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
> index 47ba7d9e81e1..2eac4a50d99b 100644
> --- a/drivers/md/raid5-cache.c
> +++ b/drivers/md/raid5-cache.c
> @@ -3168,12 +3168,15 @@ void r5l_exit_log(struct r5conf *conf)
>  {
>         struct r5l_log *log =3D conf->log;
>
> -       /* Ensure disable_writeback_work wakes up and exits */
> -       wake_up(&conf->mddev->sb_wait);
> -       flush_work(&log->disable_writeback_work);
>         md_unregister_thread(&log->reclaim_thread);
>
> +       /*
> +        * 'reconfig_mutex' is held by caller, set 'confg->log' to NULL t=
o
> +        * ensure disable_writeback_work wakes up and exits.
> +        */
>         conf->log =3D NULL;
> +       wake_up(&conf->mddev->sb_wait);
> +       flush_work(&log->disable_writeback_work);
>
>         mempool_exit(&log->meta_pool);
>         bioset_exit(&log->bs);
> --
> 2.39.2
>