lkml.org 
[lkml]   [2023]   [Dec]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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 <yukuai1@huaweicloud.com> 于2023年12月22日周五 10:53写道:
>
> 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)
> >>>>
> >>>
> > .
> >
>

\
 
 \ /
  Last update: 2023-12-22 05:57    [W:0.033 / U:1.684 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site