Re: [PATCH v3 3/3] md/raid1: check array size before reshape

[Yu Kuai <yukuai1@huaweicloud.com>]

Hi,

在 2023/07/19 19:51, Xueshi Hu 写道:
> On Wed, Jul 19, 2023 at 09:38:26AM +0200, Paul Menzel wrote:
>> Dear Xueshi,
>>
>>
>> Thank you for your patches.
>>
>> Am 19.07.23 um 09:09 schrieb Xueshi Hu:
>>> If array size doesn't changed, nothing need to do.
>>
>> Maybe: … nothing needs to be done.
> What a hurry, I'll be cautious next time and check the sentences with
> tools.
>>
>> Do you have a test case to reproduce it?
>>
> Userspace command:
> 
> 	echo 4 > /sys/devices/virtual/block/md10/md/raid_disks
> 
> Kernel function calling flow:
> 
> md_attr_store()
> 	raid_disks_store()
> 		update_raid_disks()
> 			raid1_reshape()
> 
> Maybe I shall provide more information when submit patches, thank you for
> reminding me.
>>
>> Kind regards,
>>
>> Paul
>>
>>
>>> Signed-off-by: Xueshi Hu <xueshi.hu@smartx.com>
>>> ---
>>>    drivers/md/raid1.c | 9 ++++++---
>>>    1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>>> index 62e86b7d1561..5840b8b0f9b7 100644
>>> --- a/drivers/md/raid1.c
>>> +++ b/drivers/md/raid1.c
>>> @@ -3282,9 +3282,6 @@ static int raid1_reshape(struct mddev *mddev)
>>>    	int d, d2;
>>>    	int ret;
>>> -	memset(&newpool, 0, sizeof(newpool));
>>> -	memset(&oldpool, 0, sizeof(oldpool));
>>> -
>>>    	/* Cannot change chunk_size, layout, or level */
>>>    	if (mddev->chunk_sectors != mddev->new_chunk_sectors ||
>>>    	    mddev->layout != mddev->new_layout ||
>>> @@ -3295,6 +3292,12 @@ static int raid1_reshape(struct mddev *mddev)
>>>    		return -EINVAL;
>>>    	}
>>> +	if (mddev->delta_disks == 0)
>>> +		return 0; /* nothing to do */

I think this is wrong, you should at least keep following:

         set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
         set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
         md_wakeup_thread(mddev->thread);

Thanks,
Kuai

>>> +
>>> +	memset(&newpool, 0, sizeof(newpool));
>>> +	memset(&oldpool, 0, sizeof(oldpool));
>>> +
>>>    	if (!mddev_is_clustered(mddev))
>>>    		md_allow_write(mddev);
> .
>