[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPhsuW7gJkcpEV32EUesMNycRbN0t7t+00-q9RiVTgRiY2sFwA@mail.gmail.com>
Date: Fri, 24 Nov 2023 00:13:50 -0800
From: Song Liu <song@...nel.org>
To: Yu Kuai <yukuai1@...weicloud.com>
Cc: linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org,
yukuai3@...wei.com, yi.zhang@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH -next v2 0/6] md: remove rcu protection to access rdev
from conf
On Fri, Oct 20, 2023 at 7:25 PM Yu Kuai <yukuai1@...weicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@...wei.com>
>
> 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;
>
> mddev_lock()
> // triggered by sysfs/ioctl
> rdev_removeable();
> pers->hot_remove_disk();
> rcu_assign_pointer(conf->rdev, NULL);
> synchronize_rcu();
> mddev_unlock()
>
> // triggered by daemon
> 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 separate 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 wait for
> IO to be done and 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 any spinlock or rcu_read_lock() is held, because synchronize_rcu()
> from step 6 prevent rdev to be freed until spinlock is released or
> rcu_read_unlock();
Thanks for the cover letter.
Song
>
> 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
>
> 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