Re: [PATCH v4] md/raid5: fix atomicity violation in raid5_cache_count

[Yu Kuai]

Hi,

在 2024/01/30 15:37, Song Liu 写道:
> On Thu, Jan 11, 2024 at 11:10 PM Gui-Dong Han <2045gemini@xxxxxxxxx> 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.

We don't need READ_ONCE() here because writers are protected by
'cache_size_mutex', there are no concurrent writers, it's safe to
read 'max_nr_stripes' directly.

Thanks,
Kuai

> >          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 <= 16 || size > 32768)
> >                  return -EINVAL;
> >
> > -       conf->min_nr_stripes = 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 = conf->max_nr_stripes;
> > +                       WRITE_ONCE(conf->min_nr_stripes, conf->max_nr_stripes);
>
> And here.
>
> >                          result = -ENOMEM;
> >                          break;
> >                  }
>
> Thanks,
> Song
> .