Re: [PATCH v5 00/16] md/raid5: set STRIPE_SIZE as a configurable value

Yufen Yu
2020-07-09T13:27:42Z

On 2020/7/9 7:55, Song Liu wrote:
> On Wed, Jul 8, 2020 at 6:15 AM Yufen Yu <yuyufen@huawei.com> wrote:
>>
>>
>>
>> On 2020/7/3 7:00, Song Liu wrote:
>>> On Thu, Jul 2, 2020 at 5:05 AM Yufen Yu <yuyufen@huawei.com> wrote:
>>>>
>>>> Hi, all
>>>>
>>>> For now, STRIPE_SIZE is equal to the value of PAGE_SIZE. That means, RAID5
>>>> will issue each bio to disk at least 64KB when PAGE_SIZE is 64KB in arm64.
>>>> However, filesystem usually issue bio in the unit of 4KB. Then, RAID5 may
>>>> waste resource of disk bandwidth.
>>>>
>>>> To solve the problem, this patchset try to set stripe_size as a configuare
>>>> value. The default value is 4096. We will add a new sysfs entry and set it
>>>> by writing a new value, likely:
>>>>
>>>> echo 16384 > /sys/block/md1/md/stripe_size
>>>
>>> Higher level question: do we need to support page size that is NOT 4kB
>>> times power
>>> of 2? Meaning, do we need to support 12kB, 20kB, 24kB, etc. If we only
>>> supports, 4kB,
>>> 8kB, 16kB, 32kB, etc. some of the logic can be simpler.
>>
>> Yeah, I think we just support 4kb, 8kb, 16kb, 32kb... is enough.
>> But Sorry that I don't know what logic can be simpler in current implementation.
>> I mean it also need to allocate page, and record page offset.
>
> I was thinking about replacing multiplication/division with bit
> operations (shift left/right).
> But I am not very sure how much that matters in modern ARM CPUs. Would you mind
> running some benchmarks with this?

To test multiplication/division and bit operation, I write a simple test case:

$ cat normal.c

int page_size = 65536;
int stripe_size = 32768; //32KB

int main(int argc, char *argv[])
{
int i, j, count;
int page, offset;

if (argc != 2)
return -1;

count = atol(argv[1]);

for (i = 0; i < count; i++) {
for (j = 0; j < 4; j++) {
page = page_size / stripe_size;
offset = j * stripe_size;
}
}
}

$ cat shift.c

int page_shift = 16; //64KB
int stripe_shift = 15; //32KB

int main(int argc, char *argv[])
{
int i, j, count;
int page, offset;

if (argc != 2)
return -1;

count = atol(argv[1]);

for (i = 0; i < count; i++) {
for (j = 0; j < 4; j++) {
page = 1 << (page_shift - stripe_shift);
offset = j << stripe_shift;
}
}
}

Test them on a arm64 server, the result show there is a minor
performance gap between multiplication/division and shift operation.

[root@localhost shift]# time ./normal 104857600

real 0m1.199s
user 0m1.198s
sys 0m0.000s

[root@localhost shift]# time ./shift 104857600

real 0m1.166s
user 0m1.166s
sys 0m0.000s

For our implementation, page address and page offset are just computed
when allocate stripe_size. After that, we just use the recorded value
in sh->dev[i].page and sh->dev[i].offset. So, I think current implementation
may not cause much overhead.