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

Hi,
We've updated to use READ_ONCE() in PATCH v2, Thank you for helpful advice.
Best regards,
Han

Yu Kuai [off-list ref] 于2023年12月22日周五 10:53写道：

Hi,

在 2023/12/22 10:34, 20 39 写道:

quoted

Hi Kuai,

Thank you for your patience. This email is essentially the same as my
previous one, only now adjusted to plain text format. I apologize for
any inconvenience caused earlier.

Thanks for your email and the insightful points you've raised. Let me
clarify a few aspects regarding the raid5_cache_count() and
raid5_set_cache_size() functions.

1. Callback Function in setup_conf(): You mentioned that
raid5_cache_count() is called from setup_conf() where reconfig_mutex
is held. While this is true, it's important to note that
raid5_cache_count() is actually initialized as a callback function in
setup_conf(), as described in /include/linux/shrinker.h. This means it
could be invoked later in a context where the reconfig_mutex isn't
necessarily held. The documentation in shrinker.h indicates potential
invocation scenarios beyond the initial setup context.

Yes, you're right. I misread the code. Then this patch looks good to me,
just one nit below.

quoted

quoted

quoted

quoted

@@ -7390,11 +7390,12 @@ static unsigned long raid5_cache_count(struct shrinker *shrink,
                                      struct shrink_control *sc)
   {
       struct r5conf *conf = shrink->private_data;
-
-     if (conf->max_nr_stripes < conf->min_nr_stripes)
+     int max_stripes = conf->max_nr_stripes;
+     int min_stripes = conf->min_nr_stripes;

Since read and write can concurrent, I'll suggest to use READ_ONCE() and
WRITE_ONCE() for max/min_nr_stripes.

Thanks,
Kuai

quoted

quoted

quoted

quoted

+     if (max_stripes < min_stripes)
               /* unlikely, but not impossible */
               return 0;
-     return conf->max_nr_stripes - conf->min_nr_stripes;
+     return max_stripes - min_stripes;
   }

   static struct r5conf *setup_conf(struct mddev *mddev)

.