On 2020/8/20 8:25, Song Liu wrote:

On Wed, Aug 12, 2020 at 5:48 AM Yufen Yu [off-list ref] wrote:

quoted

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