Re: [PATCH 00/12] Save memory for stripe_head buffer

[Yufen Yu <yuyufen@huawei.com>]

On 2020/8/20 8:25, Song Liu wrote:
> On Wed, Aug 12, 2020 at 5:48 AM Yufen Yu <yuyufen@huawei.com> wrote:
>>
>> Hi, all
>>
>>   In current implementation, grow_buffers() uses alloc_page() to allocate
>>   the buffers for each stripe_head, i.e. allocate a page for each dev[i]
>>   in stripe_head.
>>
>>   After setting stripe_size as a configurable value by writing sysfs entry,
>>   it means that we always allocate 64K buffers, but just use 4K of them when
>>   stripe_size is 4K in 64KB arm64.
>>
>>   To save memory, we try to let multiple buffers of stripe_head to share only
>>   one real page when page size is bigger than stripe_size. Detail can be
>>   seen in patch #10.
>>
>>   This patch set is subsequent optimization for configurable stripe_size,
>>   which based on the origin patches[1] but reorganized them.
>>
>>   Patch 1 ~ 2 try to replace current page offset '0' with dev[i].offset.
>>   Patch 3 ~ 5 let xor compute functions support different page offset for raid5.
>>   Patch 6 ~ 9 let syndrome and recovery function support different page offset for raid6.
>>   All of these patch are preparing for shared page. There is no functional change.
>>
>>   Patch 10 ~ 11 actually implement shared page between multiple devices of
>>   stripe_head. But they only make sense for PAGE_SIZE != 4096, likely, 64KB arm64
>>   system. It doesn't make any difference for PAGE_SIZE == 4096 system, likely x86.
> 
> Thanks for the patches.
> 
> I went through the first half of the set, most of the code looks fine.
> However, there is
> one issue: the way you split the changes into 12 patches is not ideal. The most
> significant issue is that build failed after 5/12 or 6/12 (and was
> fixed after 9/12). We
> would like every commit build successfully. I think the proper fix is
> to break 9/12 and
> merge changes in raid5.c to proper patches. Also, I think we can merge
> 1/12 and 2/12.
> 
> Please resubmit after these changes.
> 
> Thanks,
> Song
> .
> 

Thanks a lot for your review. These changes can make patch set more reasonable.

Thanks,
Yufen