[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 2 Feb 2024 15:11:17 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Song Liu <song@...nel.org>, Gui-Dong Han <2045gemini@...il.com>
Cc: linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org,
baijiaju1990@...look.com, stable@...r.kernel.org,
"yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH v4] md/raid5: fix atomicity violation in raid5_cache_count
Hi,
在 2024/01/30 15:37, Song Liu 写道:
> On Thu, Jan 11, 2024 at 11:10 PM Gui-Dong Han <2045gemini@...il.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.
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
> .
>
Powered by blists - more mailing lists