[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ce5d1e69-6a9a-0492-a5b0-57ec860b77c3@huaweicloud.com>
Date: Thu, 19 Oct 2023 09:59:37 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Yu Kuai <yukuai1@...weicloud.com>, Song Liu <song@...nel.org>
Cc: linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org,
yi.zhang@...wei.com, yangerkun@...wei.com,
"yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH -next 0/6] md: remvoe rcu protection to access rdev from
conf
Hi,
在 2023/10/19 9:04, Yu Kuai 写道:
> Hi,
>
> 在 2023/10/19 1:58, Song Liu 写道:
>> On Sun, Oct 15, 2023 at 6:28 PM Yu Kuai <yukuai1@...weicloud.com> wrote:
>>>
>>> From: Yu Kuai <yukuai3@...wei.com>
>>>
>>> Yu Kuai (6):
>>> md: remove useless debug code to print configuration
>>> md: remove flag RemoveSynchronized
>>> md/raid1: remove rcu protection to access rdev from conf
>>> md/raid10: remove rcu protection to access rdev from conf
>>> md/raid5: remove rcu protection to access rdev from conf
>>> md/md-multipath: remove rcu protection to access rdev from conf
>>
>> While a cover letter is highly recommended for a patchset. An empty
>> cover letter like this doesn't really add value. If the description of
>> each
>> patch is clear enough, it is OK to not have a cover-letter.
>
> Yes, I was hoping commit message in each patch is clear enough,I'll try
> to explain everything more detailed.
I'll add following cover letter in the next version:
The lifetime of rdev:
1. md_import_device() generate a rdev based on underlying disk;
mddev_lock()
rdev = kzalloc();
rdev->bdev = blkdev_get_by_dev();
mddev_unlock()
2. bind_rdev_to_array() add this rdev to mddev->disks;
mddev_lock()
kobject_add(&rdev->kobj, &mddev->kobj, ...);
list_add_rcu(&rdev->same_set, &mddev->disks);
mddev_unlock()
3. remove_and_add_spares() add this rdev to conf;
mddev_lock()
rdev_addable();
pers->hot_add_disk();
rcu_assign_pointer(conf->rdev, rdev);
mddev_unlock()
4. Use this array with rdev;
5. remove_and_add_spares() remove rdev from conf;
// triggered by sysfs/ioctl
mddev_lock()
rdev_removeable();
pers->hot_remove_disk();
rcu_assign_pointer(conf->rdev, NULL);
synchronize_rcu();
mddev_unlock()
// triggered by deamon
mddev_lock()
rdev_removeable();
synchronize_rcu(); -> this can't protect accessing rdev from conf
pers->hot_remove_disk();
rcu_assign_pointer(conf->rdev, NULL);
mddev_unlock()
6. md_kick_rdev_from_array() remove rdev from mddev->disks;
mddev_lock()
list_del_rcu(&rdev->same_set);
synchronize_rcu();
list_add(&rdev->same_set, &mddev->deleting)
mddev_unlock()
export_rdev
There are two seperate rcu protection for rdev, and this pathset remove
the protection of conf(step 3 and 5), because it's safe to access rdev
from conf in following cases:
- If 'reconfig_mutex' is held, because rdev can't be added or rmoved to
conf;
- If there is normal IO inflight, because mddev_suspend() will prevent
rdev to be added or removed to conf;
- If sync thread is running, because remove_and_add_spares() can only
be called from daemon thread when sync thread is done, and
'MD_RECOVERY_RUNNING' is also checked for ioctl/sysfs;
- if rcu_read_lock() or any spinlock is held, because synchronize_rcu()
from step 6 prevent rdev to be freed until rcu_read_unlock() or
spinlock is released;
Thanks,
Kuai
>
> Thanks,
> Kuai
>
>>
>> Thanks,
>> Song
>>
>>>
>>> drivers/md/md-multipath.c | 29 ++---
>>> drivers/md/md.c | 37 +-----
>>> drivers/md/raid1.c | 94 ++++-----------
>>> drivers/md/raid10.c | 248 +++++++++-----------------------------
>>> drivers/md/raid5-cache.c | 11 +-
>>> drivers/md/raid5-ppl.c | 16 +--
>>> drivers/md/raid5.c | 225 ++++++++++------------------------
>>> drivers/md/raid5.h | 4 +-
>>> 8 files changed, 163 insertions(+), 501 deletions(-)
>>>
>>> --
>>> 2.39.2
>>>
>> .
>>
>
> .
>
Powered by blists - more mailing lists