[-next,0/6] md: remvoe rcu protection to access rdev from conf

  From: Yu Kuai <yukuai3@huawei.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

 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(-)
  

On Sun, Oct 15, 2023 at 6:28 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>
> From: Yu Kuai <yukuai3@huawei.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.

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
>
  

Hi,

在 2023/10/19 1:58, Song Liu 写道:
> On Sun, Oct 15, 2023 at 6:28 PM Yu Kuai <yukuai1@huaweicloud.com> wrote:
>>
>> From: Yu Kuai <yukuai3@huawei.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.

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
>>
> .
>
  

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@huaweicloud.com> wrote:
>>>
>>> From: Yu Kuai <yukuai3@huawei.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
>>>
>> .
>>
> 
> .
>