Received-SPF: pass (google.com: domain of linux-kernel+bounces-44054-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240112071017.16313-1-2045gemini@gmail.com>
In-Reply-To: <20240112071017.16313-1-2045gemini@gmail.com>
From: Song Liu <song@kernel.org>
Date: Mon, 29 Jan 2024 23:37:11 -0800
Message-ID: <CAPhsuW64FjJEqTFKX9WVzrXvC4rpcjojBw5A3StUq0C20F_vyA@mail.gmail.com>
Subject: Re: [PATCH v4] md/raid5: fix atomicity violation in raid5_cache_count
To: Gui-Dong Han <2045gemini@gmail.com>
Cc: yukuai3@huawei.com, linux-raid@vger.kernel.org, 
	linux-kernel@vger.kernel.org, baijiaju1990@outlook.com, 
	stable@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Jan 11, 2024 at 11:10=E2=80=AFPM Gui-Dong Han <2045gemini@gmail.com=
> wrote:
>
[...]
>
>         raid5_release_stripe(sh);
> -       conf->max_nr_stripes++;
> +       WRITE_ONCE(conf->max_nr_stripes, conf->max_nr_stripes + 1);

This is weird. We are reading max_nr_stripes without READ_ONCE.

>         return 1;
>  }
>
> @@ -2707,7 +2707,7 @@ static int drop_one_stripe(struct r5conf *conf)
>         shrink_buffers(sh);
>         free_stripe(conf->slab_cache, sh);
>         atomic_dec(&conf->active_stripes);
> -       conf->max_nr_stripes--;
> +       WRITE_ONCE(conf->max_nr_stripes, conf->max_nr_stripes - 1);

Same here.

>         return 1;
>  }
>
> @@ -6820,7 +6820,7 @@ raid5_set_cache_size(struct mddev *mddev, int size)
>         if (size <=3D 16 || size > 32768)
>                 return -EINVAL;
>
> -       conf->min_nr_stripes =3D size;
> +       WRITE_ONCE(conf->min_nr_stripes, size);
>         mutex_lock(&conf->cache_size_mutex);
>         while (size < conf->max_nr_stripes &&
>                drop_one_stripe(conf))
> @@ -6832,7 +6832,7 @@ raid5_set_cache_size(struct mddev *mddev, int size)
>         mutex_lock(&conf->cache_size_mutex);
>         while (size > conf->max_nr_stripes)
>                 if (!grow_one_stripe(conf, GFP_KERNEL)) {
> -                       conf->min_nr_stripes =3D conf->max_nr_stripes;
> +                       WRITE_ONCE(conf->min_nr_stripes, conf->max_nr_str=
ipes);

And here.

>                         result =3D -ENOMEM;
>                         break;
>                 }

Thanks,
Song