Messages in this thread | ![/](/images/icornerl.gif) | | Subject | Re: [PATCH] md/raid5: fix atomicity violation in raid5_cache_count | From | Yu Kuai <> | Date | Fri, 22 Dec 2023 10:53:16 +0800 |
| |
Hi,
在 2023/12/22 10:34, 20 39 写道: > 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.
>>>> @@ -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 >>>> + 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) >>>> >>> > . >
| ![\](/images/icornerr.gif) |