Re: [PATCH v2] md/raid1: Fix stack memory use after return in raid1_reshape

[Wang Jinchao <wangjinchao600@gmail.com>]

On 6/13/25 19:53, Wang Jinchao wrote:
> On 2025/6/13 17:15, Yu Kuai wrote:
>> Hi,
>>
>> 在 2025/06/12 20:21, Wang Jinchao 写道:
>>> BTW, I feel raid1_reshape can be better coding with following：
>>
>> And another hint:
>>
>> The poolinfo can be removed directly, the only other place to use it
>> is r1buf_pool, and can covert to pass in conf or &conf->raid_disks.
> Thanks, I'll review these relationships carefully before refactoring.
>>
>> Thanks,
>> Kuai
>>
> 
Hi Kuai,

After reading the related code, I’d like to discuss the possibility of 
rewriting raid1_reshape.

I am considering converting r1bio_pool to use 
mempool_create_kmalloc_pool. However, mempool_create_kmalloc_pool 
requires the element size as a parameter, but the size is calculated 
dynamically in r1bio_pool_alloc(). Because of this, I feel that 
mempool_create() may be more suitable here.

I noticed that mempool_create() was used historically and was later 
replaced by mempool_init() in commit afeee514ce7f. Using 
mempool_create() would essentially be a partial revert of that commit, 
so I’m not sure whether this is appropriate.

Regarding raid1_info and pool_info, I feel the original design might be 
more suitable for the reshape process.

The goals of raid1_reshape() are:

- Keep the array usable for as long as possible.
- Be able to restore the previous state if reshape fails.
So I think we need to follow some constraints:

- conf should not be modified before freeze_array().
- We should try to prepare everything possible before freeze_array().
- If an error occurs after freeze_array(), it must be possible to roll back.

Now, regarding the idea of rewriting raid1_info or pool_info:

Convert raid1_info using krealloc:

According to rule 1, krealloc() must be called after freeze_array(). 
According to rule 2, it should be called before freeze_array(). → So 
this approach seems to violate one of the rules.

Use conf instead of pool_info:

According to rule 1, conf->raid_disks must be modified after 
freeze_array(). According to rule 2, conf->raid_disks needs to be 
updated before calling mempool_create(), i.e., before freeze_array().
These also seem to conflict.

For now, I’m not considering rule 3, as that would make the logic even 
more complex.

I’d really appreciate your thoughts on whether this direction makes 
sense or if there’s a better approach.

Thank you for your time and guidance.