raid5: create multiple threads to handle stripes

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

On 2012-08-09 16:58 Shaohua Li <[email protected]> Wrote:

Quoted 16 lines

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Quoted 4 lines

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

I tested and found two problem(maybe not).

1:print cpulist of auxth, you maybe lost print the '\n'.

drivers/md/raid5.c +1 -0

index 7c8151a..3700cdc 100644
@@ -4911,9 +4911,13 @@ struct raid5_auxth_sysfs {
 static ssize_t raid5_show_thread_cpulist(struct mddev *mddev,
        struct raid5_auxth *thread, char *page)
 {
+       int n;

2: Test 'dd if=/dev/zero of=/dev/md0 bs=2M ', the performance regress remarkable. auxthread_number=0, 200MB/s;
auxthread_number=4, 95MB/s.

2012/8/11 Jianpeng Ma <[email protected]>:

Quoted 42 lines

On 2012-08-09 16:58 Shaohua Li <[email protected]> Wrote:

Quoted 16 lines

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Quoted 4 lines

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

I tested and found two problem(maybe not).

1:print cpulist of auxth, you maybe lost print the '\n'.

drivers/md/raid5.c +5 -1

index 7c8151a..3700cdc 100644
@@ -4911,9 +4911,13 @@ struct raid5_auxth_sysfs {
 static ssize_t raid5_show_thread_cpulist(struct mddev *mddev,
        struct raid5_auxth *thread, char *page)
 {
+       int n;
        if (!mddev->private)
                return 0;
-       return cpulist_scnprintf(page, PAGE_SIZE, &thread->work_mask);
+       n = cpulist_scnprintf(page, PAGE_SIZE - 2, &thread->work_mask);
+       page[n++] = '\n';
+       page[n] = 0;
+       return n;
 }
 

drivers/md/raid5.c struct raid5_auxth_sysfs { · line 4923

Quoted 1 line

drivers/md/raid5.c +0 -0

 static ssize_t

some sysfs entries print out '\n', some not, I don't mind add it

Quoted 4 lines

2: Test 'dd if=/dev/zero of=/dev/md0 bs=2M ', the performance regress remarkable. auxthread_number=0, 200MB/s;
auxthread_number=4, 95MB/s.

So multiple threads handle stripes reduce request merge. In your workload, raid5d isn't a bottleneck at all. In practice, I thought only array which can drive high IOPS needs enable multi thread. And if you create multiple threads, better let the threads handle different cpus.

Thanks,
Shaohua

On 2012-08-13 08:21 Shaohua Li <[email protected]> Wrote:

Quoted 43 lines

2012/8/11 Jianpeng Ma <[email protected]>:

Quoted 42 lines

On 2012-08-09 16:58 Shaohua Li <[email protected]> Wrote:

Quoted 16 lines

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Quoted 4 lines

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

I tested and found two problem(maybe not).

1:print cpulist of auxth, you maybe lost print the '\n'.

drivers/md/raid5.c +5 -1

index 7c8151a..3700cdc 100644
@@ -4911,9 +4911,13 @@ struct raid5_auxth_sysfs {
 static ssize_t raid5_show_thread_cpulist(struct mddev *mddev,
        struct raid5_auxth *thread, char *page)
 {
+       int n;
        if (!mddev->private)
                return 0;
-       return cpulist_scnprintf(page, PAGE_SIZE, &thread->work_mask);
+       n = cpulist_scnprintf(page, PAGE_SIZE - 2, &thread->work_mask);
+       page[n++] = '\n';
+       page[n] = 0;
+       return n;
 }
 

Quoted 3 lines

drivers/md/raid5.c struct raid5_auxth_sysfs { · line 4923

Quoted 1 line

drivers/md/raid5.c +0 -0

 static ssize_t

some sysfs entries print out '\n', some not, I don't mind add it

I search kernel code found places which like this print out '\n'; Can you tell rule which use or not?
Thanks!

Quoted 9 lines

Quoted 3 lines

2: Test 'dd if=/dev/zero of=/dev/md0 bs=2M ', the performance regress remarkable. auxthread_number=0, 200MB/s;
auxthread_number=4, 95MB/s.

So multiple threads handle stripes reduce request merge. In your workload, raid5d isn't a bottleneck at all. In practice, I thought only array which can drive high IOPS needs enable multi thread. And if you create multiple threads, better let the threads handle different cpus.

I will test for multiple threads.

Quoted 44 lines

On 2012-08-13 08:21 Shaohua Li <[email protected]> Wrote:

Quoted 43 lines

2012/8/11 Jianpeng Ma <[email protected]>:

Quoted 42 lines

On 2012-08-09 16:58 Shaohua Li <[email protected]> Wrote:

Quoted 16 lines

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Quoted 4 lines

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

I tested and found two problem(maybe not).

1:print cpulist of auxth, you maybe lost print the '\n'.

drivers/md/raid5.c +5 -1

index 7c8151a..3700cdc 100644
@@ -4911,9 +4911,13 @@ struct raid5_auxth_sysfs {
 static ssize_t raid5_show_thread_cpulist(struct mddev *mddev,
        struct raid5_auxth *thread, char *page)
 {
+       int n;
        if (!mddev->private)
                return 0;
-       return cpulist_scnprintf(page, PAGE_SIZE, &thread->work_mask);
+       n = cpulist_scnprintf(page, PAGE_SIZE - 2, &thread->work_mask);
+       page[n++] = '\n';
+       page[n] = 0;
+       return n;
 }
 

Quoted 6 lines

Quoted 3 lines

drivers/md/raid5.c struct raid5_auxth_sysfs { · line 4923

Quoted 1 line

drivers/md/raid5.c +0 -0

 static ssize_t

some sysfs entries print out '\n', some not, I don't mind add it

I search kernel code found places which like this print out '\n'; Can you tell rule which use or not?
Thanks!

I'm not aware any rule about this

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

2: Test 'dd if=/dev/zero of=/dev/md0 bs=2M ', the performance regress remarkable. auxthread_number=0, 200MB/s;
auxthread_number=4, 95MB/s.

Quoted 6 lines

Quoted 5 lines

So multiple threads handle stripes reduce request merge. In your workload, raid5d isn't a bottleneck at all. In practice, I thought only array which can drive high IOPS needs enable multi thread. And if you create multiple threads, better let the threads handle different cpus.

I will test for multiple threads.

Thanks

2012/8/13 Shaohua Li <[email protected]>:

Quoted 63 lines · Jianpeng Ma

Quoted 44 lines

On 2012-08-13 08:21 Shaohua Li <[email protected]> Wrote:

Quoted 43 lines

2012/8/11 Jianpeng Ma <[email protected]>:

Quoted 42 lines

On 2012-08-09 16:58 Shaohua Li <[email protected]> Wrote:

Quoted 16 lines

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Quoted 4 lines

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

I tested and found two problem(maybe not).

1:print cpulist of auxth, you maybe lost print the '\n'.

drivers/md/raid5.c +5 -1

index 7c8151a..3700cdc 100644
@@ -4911,9 +4911,13 @@ struct raid5_auxth_sysfs {
 static ssize_t raid5_show_thread_cpulist(struct mddev *mddev,
        struct raid5_auxth *thread, char *page)
 {
+       int n;
        if (!mddev->private)
                return 0;
-       return cpulist_scnprintf(page, PAGE_SIZE, &thread->work_mask);
+       n = cpulist_scnprintf(page, PAGE_SIZE - 2, &thread->work_mask);
+       page[n++] = '\n';
+       page[n] = 0;
+       return n;
 }
 

Quoted 6 lines

Quoted 3 lines

drivers/md/raid5.c struct raid5_auxth_sysfs { · line 4923

Quoted 1 line

drivers/md/raid5.c +0 -0

 static ssize_t

some sysfs entries print out '\n', some not, I don't mind add it

I search kernel code found places which like this print out '\n'; Can you tell rule which use or not?
Thanks!

I'm not aware any rule about this

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

2: Test 'dd if=/dev/zero of=/dev/md0 bs=2M ', the performance regress remarkable. auxthread_number=0, 200MB/s;
auxthread_number=4, 95MB/s.

So multiple threads handle stripes reduce request merge. In your workload, raid5d isn't a bottleneck at all. In practice, I thought only array which can drive high IOPS needs enable multi thread. And if you create multiple threads, better let the threads handle different cpus.

Quoted 2 lines

Quoted 1 line

I will test for multiple threads.

Thanks

BTW, can you try below patch for the above dd workload?
http://git.kernel.dk/?p=linux-block.git;a=commitdiff;h=274193224cdabd687d804a26e0150bb20f2dd52c
That one is reverted in upstream, but eventually we should make it enter again after some CFQ issues are fixed.

Quoted 19 lines

Quoted 19 lines

Quoted 19 lines

Quoted 19 lines

1:print cpulist of auxth, you maybe lost print the '\n'.

drivers/md/raid5.c +5 -1

index 7c8151a..3700cdc 100644
@@ -4911,9 +4911,13 @@ struct raid5_auxth_sysfs {
 static ssize_t raid5_show_thread_cpulist(struct mddev *mddev,
        struct raid5_auxth *thread, char *page)
 {
+       int n;
        if (!mddev->private)
                return 0;
-       return cpulist_scnprintf(page, PAGE_SIZE, &thread->work_mask);
+       n = cpulist_scnprintf(page, PAGE_SIZE - 2, &thread->work_mask);
+       page[n++] = '\n';
+       page[n] = 0;
+       return n;
 }
 

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

drivers/md/raid5.c struct raid5_auxth_sysfs { · line 4923

Quoted 1 line

drivers/md/raid5.c +0 -0

 static ssize_t

some sysfs entries print out '\n', some not, I don't mind add it

I search kernel code found places which like this print out '\n'; Can you tell rule which use or not?
Thanks!

I'm not aware any rule about this

2012/8/13 Shaohua Li <[email protected]>:

Quoted 65 lines · Jianpeng Ma

Quoted 44 lines

On 2012-08-13 08:21 Shaohua Li <[email protected]> Wrote:

Quoted 43 lines

2012/8/11 Jianpeng Ma <[email protected]>:

Quoted 42 lines

On 2012-08-09 16:58 Shaohua Li <[email protected]> Wrote:

Quoted 16 lines

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Quoted 4 lines

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

I tested and found two problem(maybe not).

1:print cpulist of auxth, you maybe lost print the '\n'.

drivers/md/raid5.c +5 -1

index 7c8151a..3700cdc 100644
@@ -4911,9 +4911,13 @@ struct raid5_auxth_sysfs {
 static ssize_t raid5_show_thread_cpulist(struct mddev *mddev,
        struct raid5_auxth *thread, char *page)
 {
+       int n;
        if (!mddev->private)
                return 0;
-       return cpulist_scnprintf(page, PAGE_SIZE, &thread->work_mask);
+       n = cpulist_scnprintf(page, PAGE_SIZE - 2, &thread->work_mask);
+       page[n++] = '\n';
+       page[n] = 0;
+       return n;
 }
 

Quoted 6 lines

Quoted 3 lines

drivers/md/raid5.c struct raid5_auxth_sysfs { · line 4923

Quoted 1 line

drivers/md/raid5.c +0 -0

 static ssize_t

some sysfs entries print out '\n', some not, I don't mind add it

I search kernel code found places which like this print out '\n'; Can you tell rule which use or not?
Thanks!

I'm not aware any rule about this

Quoted 10 lines

Quoted 9 lines

Quoted 3 lines

2: Test 'dd if=/dev/zero of=/dev/md0 bs=2M ', the performance regress remarkable. auxthread_number=0, 200MB/s;
auxthread_number=4, 95MB/s.

So multiple threads handle stripes reduce request merge. In your workload, raid5d isn't a bottleneck at all. In practice, I thought only array which can drive high IOPS needs enable multi thread. And if you create multiple threads, better let the threads handle different cpus.

I will test for multiple threads.

Thanks

BTW, can you try below patch for the above dd workload?
http://git.kernel.dk/?p=linux-block.git;a=commitdiff;h=274193224cdabd687d804a26e0150bb20f2dd52c
That one is reverted in upstream, but eventually we should make it enter again after some CFQ issues are fixed.

On 2012-08-13 10:20 Shaohua Li <[email protected]> Wrote:

Quoted 64 lines

2012/8/13 Shaohua Li <[email protected]>:

Quoted 63 lines · Jianpeng Ma

Quoted 44 lines

On 2012-08-13 08:21 Shaohua Li <[email protected]> Wrote:

Quoted 43 lines

2012/8/11 Jianpeng Ma <[email protected]>:

Quoted 42 lines

On 2012-08-09 16:58 Shaohua Li <[email protected]> Wrote:

Quoted 16 lines

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Quoted 4 lines

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

I tested and found two problem(maybe not).

1:print cpulist of auxth, you maybe lost print the '\n'.

drivers/md/raid5.c +5 -1

index 7c8151a..3700cdc 100644
@@ -4911,9 +4911,13 @@ struct raid5_auxth_sysfs {
 static ssize_t raid5_show_thread_cpulist(struct mddev *mddev,
        struct raid5_auxth *thread, char *page)
 {
+       int n;
        if (!mddev->private)
                return 0;
-       return cpulist_scnprintf(page, PAGE_SIZE, &thread->work_mask);
+       n = cpulist_scnprintf(page, PAGE_SIZE - 2, &thread->work_mask);
+       page[n++] = '\n';
+       page[n] = 0;
+       return n;
 }
 

Quoted 6 lines

Quoted 3 lines

drivers/md/raid5.c struct raid5_auxth_sysfs { · line 4923

Quoted 1 line

drivers/md/raid5.c +0 -0

 static ssize_t

some sysfs entries print out '\n', some not, I don't mind add it

I search kernel code found places which like this print out '\n'; Can you tell rule which use or not?
Thanks!

I'm not aware any rule about this

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

2: Test 'dd if=/dev/zero of=/dev/md0 bs=2M ', the performance regress remarkable. auxthread_number=0, 200MB/s;
auxthread_number=4, 95MB/s.

So multiple threads handle stripes reduce request merge. In your workload, raid5d isn't a bottleneck at all. In practice, I thought only array which can drive high IOPS needs enable multi thread. And if you create multiple threads, better let the threads handle different cpus.

Quoted 2 lines

Quoted 2 lines

Quoted 1 line

I will test for multiple threads.

Thanks

I used fio for randwrite test using four thread which run different cpus.
The bs is 4k/8k/16k.
The result isn't increase regardless of whether using authread(four authread which run different cpu) or not? Maybe my test config had problem?

Quoted 4 lines

BTW, can you try below patch for the above dd workload?
http://git.kernel.dk/?p=linux-block.git;a=commitdiff;h=274193224cdabd687d804a26e0150bb20f2dd52c
That one is reverted in upstream, but eventually we should make it enter again after some CFQ issues are fixed.

I tested this patch.And not found problem.And the performance did not increase.

2012/8/14 Jianpeng Ma <[email protected]>:

Quoted 66 lines

On 2012-08-13 10:20 Shaohua Li <[email protected]> Wrote:

Quoted 65 lines

2012/8/13 Shaohua Li <[email protected]>:

Quoted 64 lines · Jianpeng Ma

Quoted 44 lines

On 2012-08-13 08:21 Shaohua Li <[email protected]> Wrote:

Quoted 43 lines

2012/8/11 Jianpeng Ma <[email protected]>:

Quoted 42 lines

On 2012-08-09 16:58 Shaohua Li <[email protected]> Wrote:

Quoted 16 lines

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Quoted 4 lines

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

I tested and found two problem(maybe not).

1:print cpulist of auxth, you maybe lost print the '\n'.

drivers/md/raid5.c +5 -1

index 7c8151a..3700cdc 100644
@@ -4911,9 +4911,13 @@ struct raid5_auxth_sysfs {
 static ssize_t raid5_show_thread_cpulist(struct mddev *mddev,
        struct raid5_auxth *thread, char *page)
 {
+       int n;
        if (!mddev->private)
                return 0;
-       return cpulist_scnprintf(page, PAGE_SIZE, &thread->work_mask);
+       n = cpulist_scnprintf(page, PAGE_SIZE - 2, &thread->work_mask);
+       page[n++] = '\n';
+       page[n] = 0;
+       return n;
 }
 

Quoted 6 lines

Quoted 3 lines

drivers/md/raid5.c struct raid5_auxth_sysfs { · line 4923

Quoted 1 line

drivers/md/raid5.c +0 -0

 static ssize_t

some sysfs entries print out '\n', some not, I don't mind add it

I search kernel code found places which like this print out '\n'; Can you tell rule which use or not?
Thanks!

I'm not aware any rule about this

Quoted 10 lines

Quoted 9 lines

Quoted 3 lines

2: Test 'dd if=/dev/zero of=/dev/md0 bs=2M ', the performance regress remarkable. auxthread_number=0, 200MB/s;
auxthread_number=4, 95MB/s.

So multiple threads handle stripes reduce request merge. In your workload, raid5d isn't a bottleneck at all. In practice, I thought only array which can drive high IOPS needs enable multi thread. And if you create multiple threads, better let the threads handle different cpus.

I will test for multiple threads.

Quoted 5 lines

Quoted 1 line

Quoted 1 line

Thanks

I used fio for randwrite test using four thread which run different cpus.
The bs is 4k/8k/16k.
The result isn't increase regardless of whether using authread(four authread which run different cpu) or not? Maybe my test config had problem?

how fast is your raid? If your raid can't drive high IOPS, it's not strange multithread makes no difference.

Quoted 5 lines

Quoted 4 lines

BTW, can you try below patch for the above dd workload?
http://git.kernel.dk/?p=linux-block.git;a=commitdiff;h=274193224cdabd687d804a26e0150bb20f2dd52c
That one is reverted in upstream, but eventually we should make it enter again after some CFQ issues are fixed.

I tested this patch.And not found problem.And the performance did not increase.

Ok, each thread delivers request in random time, so merge doesn't work even with that patch. I didn't worry about big size request too much, since if you set correct affinity for the auxthread, the issue should go away. And mulithread is for fast storage, I suppose it has no advantages for harddisk raid. On the other hand, maybe we can make MAX_STRIPE_BATCH bigger. Currently it's 8, so the auxthread will dispatch 8*4k request for the workload. Changing it to 16 (16*4=64k) should be good enough even for hard disk raid.

static void raid5_wakeup_stripe_thread(struct stripe_head *sh)

{
      struct r5conf *conf = sh->raid_conf;
      struct raid5_percpu *percpu;
      int i, orphaned = 1;

      percpu = per_cpu_ptr(conf->percpu, sh->cpu);
      for_each_cpu(i, &percpu->handle_threads) {
              md_wakeup_thread(conf->aux_threads[i]->thread);
              orphaned = 0;
      }

if (el_ret == ELEVATOR_BACK_MERGE) {
		if (bio_attempt_back_merge(q, req, bio)) {
			elv_bio_merged(q, req, bio);
			if (!attempt_back_merge(q, req))
				elv_merged_request(q, req, el_ret);
			goto out_unlock;

		}
	} else if (el_ret == ELEVATOR_FRONT_MERGE) {
		if (bio_attempt_front_merge(q, req, bio)) {
			elv_bio_merged(q, req, bio);
			if (!attempt_front_merge(q, req))
				elv_merged_request(q, req, el_ret);
			goto out_unlock;
		}

On 2012-08-15 11:51 Shaohua Li <[email protected]> Wrote:

Quoted 67 lines

2012/8/14 Jianpeng Ma <[email protected]>:

Quoted 66 lines

On 2012-08-13 10:20 Shaohua Li <[email protected]> Wrote:

Quoted 65 lines

2012/8/13 Shaohua Li <[email protected]>:

Quoted 64 lines · Jianpeng Ma

Quoted 44 lines

On 2012-08-13 08:21 Shaohua Li <[email protected]> Wrote:

Quoted 43 lines

2012/8/11 Jianpeng Ma <[email protected]>:

Quoted 42 lines

On 2012-08-09 16:58 Shaohua Li <[email protected]> Wrote:

Quoted 16 lines

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Quoted 4 lines

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

I tested and found two problem(maybe not).

1:print cpulist of auxth, you maybe lost print the '\n'.

drivers/md/raid5.c +5 -1

index 7c8151a..3700cdc 100644
@@ -4911,9 +4911,13 @@ struct raid5_auxth_sysfs {
 static ssize_t raid5_show_thread_cpulist(struct mddev *mddev,
        struct raid5_auxth *thread, char *page)
 {
+       int n;
        if (!mddev->private)
                return 0;
-       return cpulist_scnprintf(page, PAGE_SIZE, &thread->work_mask);
+       n = cpulist_scnprintf(page, PAGE_SIZE - 2, &thread->work_mask);
+       page[n++] = '\n';
+       page[n] = 0;
+       return n;
 }
 

Quoted 6 lines

Quoted 3 lines

drivers/md/raid5.c struct raid5_auxth_sysfs { · line 4923

Quoted 1 line

drivers/md/raid5.c +0 -0

 static ssize_t

some sysfs entries print out '\n', some not, I don't mind add it

I search kernel code found places which like this print out '\n'; Can you tell rule which use or not?
Thanks!

I'm not aware any rule about this

Quoted 10 lines

Quoted 9 lines

Quoted 3 lines

2: Test 'dd if=/dev/zero of=/dev/md0 bs=2M ', the performance regress remarkable. auxthread_number=0, 200MB/s;
auxthread_number=4, 95MB/s.

So multiple threads handle stripes reduce request merge. In your workload, raid5d isn't a bottleneck at all. In practice, I thought only array which can drive high IOPS needs enable multi thread. And if you create multiple threads, better let the threads handle different cpus.

I will test for multiple threads.

Quoted 9 lines

Quoted 5 lines

Quoted 1 line

Quoted 1 line

Thanks

I used fio for randwrite test using four thread which run different cpus.
The bs is 4k/8k/16k.
The result isn't increase regardless of whether using authread(four authread which run different cpu) or not? Maybe my test config had problem?

how fast is your raid? If your raid can't drive high IOPS, it's not strange multithread makes no difference.

Only 175 for 4K. I think your patch for harddisk dose not effect. Maybe it's only for ssd.

Quoted 6 lines

Quoted 5 lines

Quoted 4 lines

BTW, can you try below patch for the above dd workload?
http://git.kernel.dk/?p=linux-block.git;a=commitdiff;h=274193224cdabd687d804a26e0150bb20f2dd52c
That one is reverted in upstream, but eventually we should make it enter again after some CFQ issues are fixed.

I tested this patch.And not found problem.And the performance did not increase.

Quoted 9 lines

Ok, each thread delivers request in random time, so merge doesn't work even with that patch. I didn't worry about big size request too much, since if you set correct affinity for the auxthread, the issue should go away. And mulithread is for fast storage, I suppose it has no advantages for harddisk raid. On the other hand, maybe we can make MAX_STRIPE_BATCH bigger. Currently it's 8, so the auxthread will dispatch 8*4k request for the workload. Changing it to 16 (16*4=64k) should be good enough even for hard disk raid.

I review your code and have a question about wakeup authread:

Quoted 6 lines

static void raid5_wakeup_stripe_thread(struct stripe_head *sh)

{
      struct r5conf *conf = sh->raid_conf;
      struct raid5_percpu *percpu;
      int i, orphaned = 1;

Quoted 5 lines

      percpu = per_cpu_ptr(conf->percpu, sh->cpu);
      for_each_cpu(i, &percpu->handle_threads) {
              md_wakeup_thread(conf->aux_threads[i]->thread);
              orphaned = 0;
      }

If there are small stripes in handle_threads of cpu0.But the authread0/1 can run cpu0.
It's no necessary to wakup all thread.authread0 may exec all stripe,but the authread1 only wakeup and sleep,but it will spin_lock_irq(&conf->device_lock). I think you should add some limited to do .

2012/8/15 Jianpeng Ma <[email protected]>:

Quoted 101 lines

On 2012-08-15 11:51 Shaohua Li <[email protected]> Wrote:

Quoted 67 lines

2012/8/14 Jianpeng Ma <[email protected]>:

Quoted 66 lines

On 2012-08-13 10:20 Shaohua Li <[email protected]> Wrote:

Quoted 65 lines

2012/8/13 Shaohua Li <[email protected]>:

Quoted 64 lines · Jianpeng Ma

Quoted 44 lines

On 2012-08-13 08:21 Shaohua Li <[email protected]> Wrote:

Quoted 43 lines

2012/8/11 Jianpeng Ma <[email protected]>:

Quoted 42 lines

On 2012-08-09 16:58 Shaohua Li <[email protected]> Wrote:

Quoted 16 lines

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Quoted 4 lines

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

I tested and found two problem(maybe not).

1:print cpulist of auxth, you maybe lost print the '\n'.

drivers/md/raid5.c +5 -1

index 7c8151a..3700cdc 100644
@@ -4911,9 +4911,13 @@ struct raid5_auxth_sysfs {
 static ssize_t raid5_show_thread_cpulist(struct mddev *mddev,
        struct raid5_auxth *thread, char *page)
 {
+       int n;
        if (!mddev->private)
                return 0;
-       return cpulist_scnprintf(page, PAGE_SIZE, &thread->work_mask);
+       n = cpulist_scnprintf(page, PAGE_SIZE - 2, &thread->work_mask);
+       page[n++] = '\n';
+       page[n] = 0;
+       return n;
 }
 

Quoted 6 lines

Quoted 3 lines

drivers/md/raid5.c struct raid5_auxth_sysfs { · line 4923

Quoted 1 line

drivers/md/raid5.c +0 -0

 static ssize_t

some sysfs entries print out '\n', some not, I don't mind add it

I search kernel code found places which like this print out '\n'; Can you tell rule which use or not?
Thanks!

I'm not aware any rule about this

Quoted 10 lines

Quoted 9 lines

Quoted 3 lines

2: Test 'dd if=/dev/zero of=/dev/md0 bs=2M ', the performance regress remarkable. auxthread_number=0, 200MB/s;
auxthread_number=4, 95MB/s.

So multiple threads handle stripes reduce request merge. In your workload, raid5d isn't a bottleneck at all. In practice, I thought only array which can drive high IOPS needs enable multi thread. And if you create multiple threads, better let the threads handle different cpus.

I will test for multiple threads.

Quoted 9 lines

Quoted 5 lines

Quoted 1 line

Quoted 1 line

Thanks

I used fio for randwrite test using four thread which run different cpus.
The bs is 4k/8k/16k.
The result isn't increase regardless of whether using authread(four authread which run different cpu) or not? Maybe my test config had problem?

how fast is your raid? If your raid can't drive high IOPS, it's not strange multithread makes no difference.

Only 175 for 4K. I think your patch for harddisk dose not effect. Maybe it's only for ssd.

Quoted 6 lines

Quoted 5 lines

Quoted 4 lines

BTW, can you try below patch for the above dd workload?
http://git.kernel.dk/?p=linux-block.git;a=commitdiff;h=274193224cdabd687d804a26e0150bb20f2dd52c
That one is reverted in upstream, but eventually we should make it enter again after some CFQ issues are fixed.

I tested this patch.And not found problem.And the performance did not increase.

Quoted 9 lines

Ok, each thread delivers request in random time, so merge doesn't work even with that patch. I didn't worry about big size request too much, since if you set correct affinity for the auxthread, the issue should go away. And mulithread is for fast storage, I suppose it has no advantages for harddisk raid. On the other hand, maybe we can make MAX_STRIPE_BATCH bigger. Currently it's 8, so the auxthread will dispatch 8*4k request for the workload. Changing it to 16 (16*4=64k) should be good enough even for hard disk raid.

I review your code and have a question about wakeup authread:

Quoted 6 lines

static void raid5_wakeup_stripe_thread(struct stripe_head *sh)

{
      struct r5conf *conf = sh->raid_conf;
      struct raid5_percpu *percpu;
      int i, orphaned = 1;

Quoted 8 lines

Quoted 5 lines

      percpu = per_cpu_ptr(conf->percpu, sh->cpu);
      for_each_cpu(i, &percpu->handle_threads) {
              md_wakeup_thread(conf->aux_threads[i]->thread);
              orphaned = 0;
      }

If there are small stripes in handle_threads of cpu0.But the authread0/1 can run cpu0.
It's no necessary to wakup all thread.authread0 may exec all stripe,but the authread1 only wakeup and sleep,but it will spin_lock_irq(&conf->device_lock). I think you should add some limited to do .

I used to have a counter for stripes queued, with it, we can determine how many threads should be waken up. That is what I did when each each thread can handle stripes from any CPU. I thought this problem isn't sever now since each thread can just handle strips from one or several CPUs. If this really is a problem, we can fix it later, but my test doesn't show this is a problem.

Quoted 1 line

conf->mddev->resync_mismatches += STRIPE_SECTORS;

Sorry for using this title to send this.

This code maybe error,so it should correct(If multiple-thread dosen't enable,it maybe also error,but no obviously). In func handle_parity_checks5:

Quoted 1 line

Quoted 1 line

conf->mddev->resync_mismatches += STRIPE_SECTORS;

If multi-thread handle stripe on different cpu, there will be raced on resync_mismatches.

So the patch is:

Subject: [PATCH] md:change resync_mismatches to atomic64_t to avoid races

Signed-off-by Jianpeng Ma <[email protected]>

---
 drivers/md/md.c     |    7 ++++---
 drivers/md/md.h     |    2 +-
 drivers/md/raid1.c  |    2 +-
 drivers/md/raid10.c |    2 +-
 drivers/md/raid5.c  |    4 ++--
 5 files changed, 9 insertions(+), 8 deletions(-)

drivers/md/md.c +4 -3

index 308e87b..b0b9474 100644
@@ -4278,7 +4278,8 @@ static ssize_t
 mismatch_cnt_show(struct mddev *mddev, char *page)
 {
 	return sprintf(page, "%llu\n",
-		       (unsigned long long) mddev->resync_mismatches);
+		       (unsigned long long)
+		       atomic64_read(&mddev->resync_mismatches));
 }
 
 static struct md_sysfs_entry md_scan_mode =

@@ -5245,7 +5246,7 @@ static void md_clean(struct mddev *mddev)
 	mddev->new_layout = 0;
 	mddev->new_chunk_sectors = 0;
 	mddev->curr_resync = 0;
-	mddev->resync_mismatches = 0;
+	atomic64_set(&mddev->resync_mismatches, 0);
 	mddev->suspend_lo = mddev->suspend_hi = 0;
 	mddev->sync_speed_min = mddev->sync_speed_max = 0;
 	mddev->recovery = 0;

@@ -7349,7 +7350,7 @@ void md_do_sync(struct mddev *mddev)
 		 * which defaults to physical size, but can be virtual size
 		 */
 		max_sectors = mddev->resync_max_sectors;
-		mddev->resync_mismatches = 0;
+		atomic64_set(&mddev->resync_mismatches, 0);
 		/* we don't use the checkpoint if there's a bitmap */
 		if (test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery))
 			j = mddev->resync_min;

drivers/md/md.h +1 -1

index f385b03..8f341d8 100644
@@ -282,7 +282,7 @@ struct mddev {
 
 	sector_t			resync_max_sectors; /* may be set by personality */
 
-	sector_t			resync_mismatches; /* count of sectors where
+	atomic64_t			resync_mismatches; /* count of sectors where
 							    * parity/replica mismatch found
 							    */
 

drivers/md/raid1.c +1 -1

index 611b5f7..ebceb98 100644
@@ -1867,7 +1867,7 @@ static int process_checks(struct r1bio *r1_bio)
 		} else
 			j = 0;
 		if (j >= 0)
-			mddev->resync_mismatches += r1_bio->sectors;
+			atomic64_add(r1_bio->sectors, &mddev->resync_mismatches);
 		if (j < 0 || (test_bit(MD_RECOVERY_CHECK, &mddev->recovery)
 			      && test_bit(BIO_UPTODATE, &sbio->bi_flags))) {
 			/* No need to write to this device. */

drivers/md/raid10.c +1 -1

index 1c2eb38..ad0a8b7 100644
@@ -1950,7 +1950,7 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
 					break;
 			if (j == vcnt)
 				continue;
-			mddev->resync_mismatches += r10_bio->sectors;
+			atomic64_add(r10_bio->sectors, &mddev->resync_mismatches);
 			if (test_bit(MD_RECOVERY_CHECK, &mddev->recovery))
 				/* Don't fix anything. */
 				continue;

drivers/md/raid5.c +2 -2

index 7031b86..0a2702f 100644
@@ -2931,7 +2931,7 @@ static void handle_parity_checks5(struct r5conf *conf, struct stripe_head *sh,
 			 */
 			set_bit(STRIPE_INSYNC, &sh->state);
 		else {
-			conf->mddev->resync_mismatches += STRIPE_SECTORS;
+			atomic64_add(STRIPE_SECTORS, &conf->mddev->resync_mismatches);
 			if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery))
 				/* don't try to repair!! */
 				set_bit(STRIPE_INSYNC, &sh->state);

@@ -3083,7 +3083,7 @@ static void handle_parity_checks6(struct r5conf *conf, struct stripe_head *sh,
 				 */
 			}
 		} else {
-			conf->mddev->resync_mismatches += STRIPE_SECTORS;
+			atomic64_add(STRIPE_SECTORS, &conf->mddev->resync_mismatches);
 			if (test_bit(MD_RECOVERY_CHECK, &conf->mddev->recovery))
 				/* don't try to repair!! */
 				set_bit(STRIPE_INSYNC, &sh->state);

2012/8/11 Jianpeng Ma <[email protected]>:

Quoted 3 lines

On 2012-08-09 16:58 Shaohua Li <[email protected]> Wrote:

Quoted 2 lines

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It

Quoted 3 lines

2: Test 'dd if=/dev/zero of=/dev/md0 bs=2M ', the performance regress remarkable. auxthread_number=0, 200MB/s;
auxthread_number=4, 95MB/s.

So multiple threads handle stripes reduce request merge. In your workload, raid5d isn't a bottleneck at all. In practice, I thought only array which can drive high IOPS needs enable multi thread. And if you create multiple threads, better let the threads handle different cpus.

Thanks,
Shaohua

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

Signed-off-by Shaohua Li <[email protected]>

---
 drivers/md/md.c    |    8 -
 drivers/md/md.h    |    8 +
 drivers/md/raid5.c |  406 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 drivers/md/raid5.h |   19 ++
 4 files changed, 418 insertions(+), 23 deletions(-)

Index: linux/drivers/md/raid5.c
===================================================================

drivers/md/raid5.c +19 -0

@@ -196,6 +196,21 @@ static int stripe_operations_active(stru
 	       test_bit(STRIPE_COMPUTE_RUN, &sh->state);
 }
 
+static void raid5_wakeup_stripe_thread(struct stripe_head *sh)
+{
+	struct r5conf *conf = sh->raid_conf;
+	struct raid5_percpu *percpu;
+	int i, orphaned = 1;
+
+	percpu = per_cpu_ptr(conf->percpu, sh->cpu);
+	for_each_cpu(i, &percpu->handle_threads) {
+		md_wakeup_thread(conf->aux_threads[i]->thread);
+		orphaned = 0;
+	}
+	if (orphaned)
+		md_wakeup_thread(conf->mddev->thread);
+}
+
 static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh)
 {
 	BUG_ON(!list_empty(&sh->lru));

@@ -208,9 +223,19 @@ static void do_release_stripe(struct r5c
 			   sh->bm_seq - conf->seq_write > 0)
 			list_add_tail(&sh->lru, &conf->bitmap_list);
 		else {
+			int cpu = sh->cpu;
+			struct raid5_percpu *percpu;
+			if (!cpu_online(cpu)) {
+				cpu = cpumask_any(cpu_online_mask);

drivers/md/raid5.c +2 -0

 		}
 		md_wakeup_thread(conf->mddev->thread);
 	} else {

@@ -355,6 +380,7 @@ static void init_stripe(struct stripe_he
 		raid5_build_block(sh, i, previous);
 	}
 	insert_hash(conf, sh);
+	sh->cpu = smp_processor_id();
 }
 
 static struct stripe_head *__find_stripe(struct r5conf *conf, sector_t sector,

@@ -3689,12 +3715,19 @@ static void raid5_activate_delayed(struc
 		while (!list_empty(&conf->delayed_list)) {
 			struct list_head *l = conf->delayed_list.next;
 			struct stripe_head *sh;
+			int cpu;
 			sh = list_entry(l, struct stripe_head, lru);
 			list_del_init(l);

Quoted 62 lines · On Thu, 9 Aug 2012 16:58:08 +0800 Shaohua Li <[email protected]> wrote:

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

Signed-off-by Shaohua Li <[email protected]>

---
 drivers/md/md.c    |    8 -
 drivers/md/md.h    |    8 +
 drivers/md/raid5.c |  406 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 drivers/md/raid5.h |   19 ++
 4 files changed, 418 insertions(+), 23 deletions(-)

Index: linux/drivers/md/raid5.c
===================================================================

drivers/md/raid5.c +19 -0

@@ -196,6 +196,21 @@ static int stripe_operations_active(stru
 	       test_bit(STRIPE_COMPUTE_RUN, &sh->state);
 }
 
+static void raid5_wakeup_stripe_thread(struct stripe_head *sh)
+{
+	struct r5conf *conf = sh->raid_conf;
+	struct raid5_percpu *percpu;
+	int i, orphaned = 1;
+
+	percpu = per_cpu_ptr(conf->percpu, sh->cpu);
+	for_each_cpu(i, &percpu->handle_threads) {
+		md_wakeup_thread(conf->aux_threads[i]->thread);
+		orphaned = 0;
+	}
+	if (orphaned)
+		md_wakeup_thread(conf->mddev->thread);
+}
+
 static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh)
 {
 	BUG_ON(!list_empty(&sh->lru));

@@ -208,9 +223,19 @@ static void do_release_stripe(struct r5c
 			   sh->bm_seq - conf->seq_write > 0)
 			list_add_tail(&sh->lru, &conf->bitmap_list);
 		else {
+			int cpu = sh->cpu;
+			struct raid5_percpu *percpu;
+			if (!cpu_online(cpu)) {
+				cpu = cpumask_any(cpu_online_mask);

drivers/md/raid5.c static void do_release_stripe(struct r5c · line 238

Quoted 10 lines

drivers/md/raid5.c +7 -1

+				sh->cpu = cpu;
+			}
+			percpu = per_cpu_ptr(conf->percpu, cpu);
+
 			clear_bit(STRIPE_DELAYED, &sh->state);
 			clear_bit(STRIPE_BIT_DELAY, &sh->state);
-			list_add_tail(&sh->lru, &conf->handle_list);
+			list_add_tail(&sh->lru, &percpu->handle_list);
+			raid5_wakeup_stripe_thread(sh);
+			return;

I confess that I don't know a lot about cpu hotplug, but this looks like it should have some locking. In particular, "get_online_cpus()" before we check "cpu_online()", and "put_online_cpus()" after we have added to the per_cpu->handle_list().

Maybe that isn't needed, but if it isn't I'd like to understand why.

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

Signed-off-by Shaohua Li <[email protected]>

---
 drivers/md/md.c    |    8 -
 drivers/md/md.h    |    8 +
 drivers/md/raid5.c |  406 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 drivers/md/raid5.h |   19 ++
 4 files changed, 418 insertions(+), 23 deletions(-)

Index: linux/drivers/md/raid5.c
===================================================================

drivers/md/raid5.c +386 -20

@@ -196,6 +196,21 @@ static int stripe_operations_active(stru
 	       test_bit(STRIPE_COMPUTE_RUN, &sh->state);
 }
 
+static void raid5_wakeup_stripe_thread(struct stripe_head *sh)
+{
+	struct r5conf *conf = sh->raid_conf;
+	struct raid5_percpu *percpu;
+	int i, orphaned = 1;
+
+	percpu = per_cpu_ptr(conf->percpu, sh->cpu);
+	for_each_cpu(i, &percpu->handle_threads) {
+		md_wakeup_thread(conf->aux_threads[i]->thread);
+		orphaned = 0;
+	}
+	if (orphaned)
+		md_wakeup_thread(conf->mddev->thread);
+}
+
 static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh)
 {
 	BUG_ON(!list_empty(&sh->lru));

@@ -208,9 +223,19 @@ static void do_release_stripe(struct r5c
 			   sh->bm_seq - conf->seq_write > 0)
 			list_add_tail(&sh->lru, &conf->bitmap_list);
 		else {
+			int cpu = sh->cpu;
+			struct raid5_percpu *percpu;
+			if (!cpu_online(cpu)) {
+				cpu = cpumask_any(cpu_online_mask);
+				sh->cpu = cpu;
+			}
+			percpu = per_cpu_ptr(conf->percpu, cpu);
+
 			clear_bit(STRIPE_DELAYED, &sh->state);
 			clear_bit(STRIPE_BIT_DELAY, &sh->state);
-			list_add_tail(&sh->lru, &conf->handle_list);
+			list_add_tail(&sh->lru, &percpu->handle_list);
+			raid5_wakeup_stripe_thread(sh);
+			return;
 		}
 		md_wakeup_thread(conf->mddev->thread);
 	} else {

@@ -355,6 +380,7 @@ static void init_stripe(struct stripe_he
 		raid5_build_block(sh, i, previous);
 	}
 	insert_hash(conf, sh);
+	sh->cpu = smp_processor_id();
 }
 
 static struct stripe_head *__find_stripe(struct r5conf *conf, sector_t sector,

@@ -3689,12 +3715,19 @@ static void raid5_activate_delayed(struc
 		while (!list_empty(&conf->delayed_list)) {
 			struct list_head *l = conf->delayed_list.next;
 			struct stripe_head *sh;
+			int cpu;
 			sh = list_entry(l, struct stripe_head, lru);
 			list_del_init(l);
 			clear_bit(STRIPE_DELAYED, &sh->state);
 			if (!test_and_set_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
 				atomic_inc(&conf->preread_active_stripes);
 			list_add_tail(&sh->lru, &conf->hold_list);
+			cpu = sh->cpu;
+			if (!cpu_online(cpu)) {
+				cpu = cpumask_any(cpu_online_mask);
+				sh->cpu = cpu;
+			}
+			raid5_wakeup_stripe_thread(sh);
 		}
 	}
 }

@@ -3968,18 +4001,29 @@ static int chunk_aligned_read(struct mdd
  * head of the hold_list has changed, i.e. the head was promoted to the
  * handle_list.
  */
-static struct stripe_head *__get_priority_stripe(struct r5conf *conf)
+static struct stripe_head *__get_priority_stripe(struct r5conf *conf,
+	cpumask_t *mask)
 {
-	struct stripe_head *sh;
-
+	struct stripe_head *sh = NULL, *tmp;
+	struct list_head *handle_list = NULL;
+	int cpu;
+
+	/* Should we take action to avoid starvation of latter CPUs ? */
+	for_each_cpu(cpu, mask) {
+		struct raid5_percpu *percpu = per_cpu_ptr(conf->percpu, cpu);
+		if (!list_empty(&percpu->handle_list)) {
+			handle_list = &percpu->handle_list;
+			break;
+		}
+	}
 	pr_debug("%s: handle: %s hold: %s full_writes: %d bypass_count: %d\n",
 		  __func__,
-		  list_empty(&conf->handle_list) ? "empty" : "busy",
+		  !handle_list ? "empty" : "busy",
 		  list_empty(&conf->hold_list) ? "empty" : "busy",
 		  atomic_read(&conf->pending_full_writes), conf->bypass_count);
 
-	if (!list_empty(&conf->handle_list)) {
-		sh = list_entry(conf->handle_list.next, typeof(*sh), lru);
+	if (handle_list) {
+		sh = list_entry(handle_list->next, typeof(*sh), lru);
 
 		if (list_empty(&conf->hold_list))
 			conf->bypass_count = 0;

@@ -3997,12 +4041,23 @@ static struct stripe_head *__get_priorit
 		   ((conf->bypass_threshold &&
 		     conf->bypass_count > conf->bypass_threshold) ||
 		    atomic_read(&conf->pending_full_writes) == 0)) {
-		sh = list_entry(conf->hold_list.next,
-				typeof(*sh), lru);
-		conf->bypass_count -= conf->bypass_threshold;
-		if (conf->bypass_count < 0)
-			conf->bypass_count = 0;
-	} else
+
+		list_for_each_entry(tmp, &conf->hold_list,  lru) {
+			if (cpumask_test_cpu(tmp->cpu, mask) ||
+			    !cpu_online(tmp->cpu)) {
+				sh = tmp;
+				break;
+			}
+		}
+
+		if (sh) {
+			conf->bypass_count -= conf->bypass_threshold;
+			if (conf->bypass_count < 0)
+				conf->bypass_count = 0;
+		}
+	}
+
+	if (!sh)
 		return NULL;
 
 	list_del_init(&sh->lru);

@@ -4594,13 +4649,13 @@ static int  retry_aligned_read(struct r5
 }
 
 #define MAX_STRIPE_BATCH 8
-static int handle_active_stripes(struct r5conf *conf)
+static int handle_active_stripes(struct r5conf *conf, cpumask_t *mask)
 {
 	struct stripe_head *batch[MAX_STRIPE_BATCH], *sh;
 	int i, batch_size = 0;
 
 	while (batch_size < MAX_STRIPE_BATCH &&
-			(sh = __get_priority_stripe(conf)) != NULL)
+			(sh = __get_priority_stripe(conf, mask)) != NULL)
 		batch[batch_size++] = sh;
 
 	if (batch_size == 0)

@@ -4618,6 +4673,35 @@ static int handle_active_stripes(struct
 	return batch_size;
 }
 
+static void raid5auxd(struct md_thread *thread)
+{
+	struct mddev *mddev = thread->mddev;
+	struct r5conf *conf = mddev->private;
+	struct blk_plug plug;
+	int handled;
+	struct raid5_auxth *auxth = thread->private;
+
+	pr_debug("+++ raid5auxd active\n");
+
+	blk_start_plug(&plug);
+	handled = 0;
+	spin_lock_irq(&conf->device_lock);
+	while (1) {
+		int batch_size;
+
+		batch_size = handle_active_stripes(conf, &auxth->work_mask);
+		if (!batch_size)
+			break;
+		handled += batch_size;
+	}
+	pr_debug("%d stripes handled\n", handled);
+
+	spin_unlock_irq(&conf->device_lock);
+	blk_finish_plug(&plug);
+
+	pr_debug("--- raid5auxd inactive\n");
+}
+
 /*
  * This is our raid5 kernel thread.
  *

@@ -4665,7 +4749,7 @@ static void raid5d(struct md_thread *thr
 			handled++;
 		}
 
-		batch_size = handle_active_stripes(conf);
+		batch_size = handle_active_stripes(conf, &conf->work_mask);
 		if (!batch_size)
 			break;
 		handled += batch_size;

@@ -4794,10 +4878,270 @@ stripe_cache_active_show(struct mddev *m
 static struct md_sysfs_entry
 raid5_stripecache_active = __ATTR_RO(stripe_cache_active);
 
+static void raid5_update_threads_handle_mask(struct mddev *mddev)
+{
+	int cpu, i;
+	struct raid5_percpu *percpu;
+	struct r5conf *conf = mddev->private;
+
+	for_each_online_cpu(cpu) {
+		percpu = per_cpu_ptr(conf->percpu, cpu);
+		cpumask_clear(&percpu->handle_threads);
+	}
+	cpumask_copy(&conf->work_mask, cpu_online_mask);
+
+	for (i = 0; i < conf->aux_thread_num; i++) {
+		cpumask_t *work_mask = &conf->aux_threads[i]->work_mask;
+		for_each_cpu(cpu, work_mask) {
+			percpu = per_cpu_ptr(conf->percpu, cpu);
+			cpumask_set_cpu(i, &percpu->handle_threads);
+		}
+		cpumask_andnot(&conf->work_mask, &conf->work_mask,
+				work_mask);
+	}
+}
+
+struct raid5_auxth_sysfs {
+	struct attribute attr;
+	ssize_t (*show)(struct mddev *, struct raid5_auxth *, char *);
+	ssize_t (*store)(struct mddev *, struct raid5_auxth *,
+		const char *, size_t);
+};
+
+static ssize_t raid5_show_thread_cpulist(struct mddev *mddev,
+	struct raid5_auxth *thread, char *page)
+{
+	if (!mddev->private)
+		return 0;
+	return cpulist_scnprintf(page, PAGE_SIZE, &thread->work_mask);
+}
+
+static ssize_t
+raid5_store_thread_cpulist(struct mddev *mddev, struct raid5_auxth *thread,
+	const char *page, size_t len)
+{
+	struct r5conf *conf = mddev->private;
+	cpumask_var_t mask;
+
+	if (!conf)
+		return -ENODEV;
+	if (!alloc_cpumask_var(&mask, GFP_KERNEL))
+		return -ENOMEM;
+
+	if (cpulist_parse(page, mask)) {
+		free_cpumask_var(mask);
+		return -EINVAL;
+	}
+
+	get_online_cpus();
+	spin_lock_irq(&conf->device_lock);
+	cpumask_copy(&thread->work_mask, mask);
+	raid5_update_threads_handle_mask(mddev);
+	spin_unlock_irq(&conf->device_lock);
+	put_online_cpus();
+	set_cpus_allowed_ptr(thread->thread->tsk, mask);
+
+	free_cpumask_var(mask);
+	return len;
+}
+
+static struct raid5_auxth_sysfs thread_cpulist =
+__ATTR(cpulist, S_IRUGO|S_IWUSR, raid5_show_thread_cpulist,
+	raid5_store_thread_cpulist);
+
+static struct attribute *auxth_attrs[] = {
+	&thread_cpulist.attr,
+	NULL,
+};
+
+static ssize_t
+raid5_auxth_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
+{
+	struct raid5_auxth_sysfs *entry = container_of(attr,
+		struct raid5_auxth_sysfs, attr);
+	struct raid5_auxth *thread = container_of(kobj,
+		struct raid5_auxth, kobj);
+	struct mddev *mddev = thread->thread->mddev;
+	ssize_t ret;
+
+	if (!entry->show)
+		return -EIO;
+	mddev_lock(mddev);
+	ret = entry->show(mddev, thread, page);
+	mddev_unlock(mddev);
+	return ret;
+}
+
+static ssize_t
+raid5_auxth_attr_store(struct kobject *kobj, struct attribute *attr,
+	      const char *page, size_t length)
+{
+	struct raid5_auxth_sysfs *entry = container_of(attr,
+		struct raid5_auxth_sysfs, attr);
+	struct raid5_auxth *thread = container_of(kobj,
+		struct raid5_auxth, kobj);
+	struct mddev *mddev = thread->thread->mddev;
+	ssize_t ret;
+
+	if (!entry->store)
+		return -EIO;
+	if (!capable(CAP_SYS_ADMIN))
+		return -EACCES;
+	mddev_lock(mddev);
+	ret = entry->store(mddev, thread, page, length);
+	mddev_unlock(mddev);
+	return ret;
+}
+
+static void raid5_auxth_release(struct kobject *kobj)
+{
+	struct raid5_auxth *thread = container_of(kobj,
+		struct raid5_auxth, kobj);
+	kfree(thread);
+}
+
+static const struct sysfs_ops raid5_auxth_sysfsops = {
+	.show = raid5_auxth_attr_show,
+	.store = raid5_auxth_attr_store,
+};
+static struct kobj_type raid5_auxth_ktype = {
+	.release = raid5_auxth_release,
+	.sysfs_ops = &raid5_auxth_sysfsops,
+	.default_attrs = auxth_attrs,
+};
+
+static ssize_t
+raid5_show_auxthread_number(struct mddev *mddev, char *page)
+{
+	struct r5conf *conf = mddev->private;
+	if (conf)
+		return sprintf(page, "%d\n", conf->aux_thread_num);
+	else
+		return 0;
+}
+
+static void raid5_auxth_delete(struct work_struct *ws)
+{
+	struct raid5_auxth *thread = container_of(ws, struct raid5_auxth,
+		del_work);
+
+	kobject_del(&thread->kobj);
+	kobject_put(&thread->kobj);
+}
+
+static void __free_aux_thread(struct mddev *mddev, struct raid5_auxth *thread)
+{
+	md_unregister_thread(&thread->thread);
+	INIT_WORK(&thread->del_work, raid5_auxth_delete);
+	kobject_get(&thread->kobj);
+	md_queue_misc_work(&thread->del_work);
+}
+
+static struct raid5_auxth *__create_aux_thread(struct mddev *mddev, int i)
+{
+	struct raid5_auxth *thread;
+	char name[10];
+
+	thread = kzalloc(sizeof(*thread), GFP_KERNEL);
+	if (!thread)
+		return NULL;
+	snprintf(name, 10, "aux%d", i);
+	thread->thread = md_register_thread(raid5auxd, mddev, name);
+	if (!thread->thread) {
+		kfree(thread);
+		return NULL;
+	}
+	thread->thread->private = thread;
+
+	cpumask_copy(&thread->work_mask, cpu_online_mask);
+
+	if (kobject_init_and_add(&thread->kobj, &raid5_auxth_ktype,
+	    &mddev->kobj, "auxth%d", i)) {
+		md_unregister_thread(&thread->thread);
+		kfree(thread);
+		return NULL;
+	}
+	return thread;
+}
+
+static ssize_t
+raid5_store_auxthread_number(struct mddev *mddev, const char *page, size_t len)
+{
+	struct r5conf *conf = mddev->private;
+	unsigned long new;
+	int i;
+	struct raid5_auxth **threads;
+
+	if (len >= PAGE_SIZE)
+		return -EINVAL;
+	if (!conf)
+		return -ENODEV;
+
+	if (kstrtoul(page, 10, &new))
+		return -EINVAL;
+
+	if (new == conf->aux_thread_num)
+		return len;
+
+	/* There is no point creating more threads than cpu number */
+	if (new > num_online_cpus())
+		return -EINVAL;
+
+	if (new > conf->aux_thread_num) {
+		threads = kzalloc(sizeof(struct raid5_auxth *) * new,
+				GFP_KERNEL);
+		if (!threads)
+			return -ENOMEM;
+
+		i = conf->aux_thread_num;
+		while (i < new) {
+			threads[i] = __create_aux_thread(mddev, i);
+			if (!threads[i])
+				goto error;
+
+			i++;
+		}
+		memcpy(threads, conf->aux_threads,
+			sizeof(struct raid5_auxth *) * conf->aux_thread_num);
+		get_online_cpus();
+		spin_lock_irq(&conf->device_lock);
+		kfree(conf->aux_threads);
+		conf->aux_threads = threads;
+		conf->aux_thread_num = new;
+		raid5_update_threads_handle_mask(mddev);
+		spin_unlock_irq(&conf->device_lock);
+		put_online_cpus();
+	} else {
+		int old = conf->aux_thread_num;
+
+		get_online_cpus();
+		spin_lock_irq(&conf->device_lock);
+		conf->aux_thread_num = new;
+		raid5_update_threads_handle_mask(mddev);
+		spin_unlock_irq(&conf->device_lock);
+		put_online_cpus();
+		for (i = new; i < old; i++)
+			__free_aux_thread(mddev, conf->aux_threads[i]);
+	}
+
+	return len;
+error:
+	while (--i >= conf->aux_thread_num)
+		__free_aux_thread(mddev, threads[i]);
+	kfree(threads);
+	return -ENOMEM;
+}
+
+static struct md_sysfs_entry
+raid5_auxthread_number = __ATTR(auxthread_number, S_IRUGO|S_IWUSR,
+				raid5_show_auxthread_number,
+				raid5_store_auxthread_number);
+
 static struct attribute *raid5_attrs[] =  {
 	&raid5_stripecache_size.attr,
 	&raid5_stripecache_active.attr,
 	&raid5_preread_bypass_threshold.attr,
+	&raid5_auxthread_number.attr,
 	NULL,
 };
 static struct attribute_group raid5_attrs_group = {

@@ -4845,6 +5189,7 @@ static void raid5_free_percpu(struct r5c
 
 static void free_conf(struct r5conf *conf)
 {
+	kfree(conf->aux_threads);
 	shrink_stripes(conf);
 	raid5_free_percpu(conf);
 	kfree(conf->disks);

@@ -4857,7 +5202,7 @@ static int raid456_cpu_notify(struct not
 			      void *hcpu)
 {
 	struct r5conf *conf = container_of(nfb, struct r5conf, cpu_notify);
-	long cpu = (long)hcpu;
+	long cpu = (long)hcpu, anycpu;
 	struct raid5_percpu *percpu = per_cpu_ptr(conf->percpu, cpu);
 
 	switch (action) {

@@ -4876,9 +5221,17 @@ static int raid456_cpu_notify(struct not
 			       __func__, cpu);
 			return notifier_from_errno(-ENOMEM);
 		}
+		INIT_LIST_HEAD(&(percpu->handle_list));
+		cpumask_clear(&percpu->handle_threads);
 		break;
 	case CPU_DEAD:
 	case CPU_DEAD_FROZEN:
+		spin_lock_irq(&conf->device_lock);
+		anycpu = cpumask_any(cpu_online_mask);
+		list_splice_tail_init(&percpu->handle_list,
+			&per_cpu_ptr(conf->percpu, anycpu)->handle_list);
+		spin_unlock_irq(&conf->device_lock);
+
 		safe_put_page(percpu->spare_page);
 		kfree(percpu->scribble);
 		percpu->spare_page = NULL;

@@ -4887,6 +5240,10 @@ static int raid456_cpu_notify(struct not
 	default:
 		break;
 	}
+
+	spin_lock_irq(&conf->device_lock);
+	raid5_update_threads_handle_mask(conf->mddev);
+	spin_unlock_irq(&conf->device_lock);
 	return NOTIFY_OK;
 }
 #endif

@@ -4907,20 +5264,24 @@ static int raid5_alloc_percpu(struct r5c
 	get_online_cpus();
 	err = 0;
 	for_each_present_cpu(cpu) {
+		struct raid5_percpu *percpu = per_cpu_ptr(conf->percpu, cpu);
+
 		if (conf->level == 6) {
 			spare_page = alloc_page(GFP_KERNEL);
 			if (!spare_page) {
 				err = -ENOMEM;
 				break;
 			}
-			per_cpu_ptr(conf->percpu, cpu)->spare_page = spare_page;
+			percpu->spare_page = spare_page;
 		}
 		scribble = kmalloc(conf->scribble_len, GFP_KERNEL);
 		if (!scribble) {
 			err = -ENOMEM;
 			break;
 		}
-		per_cpu_ptr(conf->percpu, cpu)->scribble = scribble;
+		percpu->scribble = scribble;
+		INIT_LIST_HEAD(&percpu->handle_list);
+		cpumask_clear(&percpu->handle_threads);
 	}
 #ifdef CONFIG_HOTPLUG_CPU
 	conf->cpu_notify.notifier_call = raid456_cpu_notify;

@@ -4976,7 +5337,6 @@ static struct r5conf *setup_conf(struct
 	spin_lock_init(&conf->device_lock);
 	init_waitqueue_head(&conf->wait_for_stripe);
 	init_waitqueue_head(&conf->wait_for_overlap);
-	INIT_LIST_HEAD(&conf->handle_list);
 	INIT_LIST_HEAD(&conf->hold_list);
 	INIT_LIST_HEAD(&conf->delayed_list);
 	INIT_LIST_HEAD(&conf->bitmap_list);

@@ -4987,6 +5347,8 @@ static struct r5conf *setup_conf(struct
 	conf->bypass_threshold = BYPASS_THRESHOLD;
 	conf->recovery_disabled = mddev->recovery_disabled - 1;
 
+	cpumask_copy(&conf->work_mask, cpu_online_mask);
+
 	conf->raid_disks = mddev->raid_disks;
 	if (mddev->reshape_position == MaxSector)
 		conf->previous_raid_disks = mddev->raid_disks;

@@ -5403,6 +5765,10 @@ abort:
 static int stop(struct mddev *mddev)
 {
 	struct r5conf *conf = mddev->private;
+	int i;
+
+	for (i = 0; i < conf->aux_thread_num; i++)
+		__free_aux_thread(mddev, conf->aux_threads[i]);
 
 	md_unregister_thread(&mddev->thread);
 	if (mddev->queue)

Index: linux/drivers/md/raid5.h
===================================================================

drivers/md/raid5.h +19 -0

@@ -211,6 +211,7 @@ struct stripe_head {
 	enum check_states	check_state;
 	enum reconstruct_states reconstruct_state;
 	spinlock_t		stripe_lock;
+	int			cpu;
 	/**
 	 * struct stripe_operations
 	 * @target - STRIPE_OP_COMPUTE_BLK target

@@ -364,6 +365,14 @@ struct disk_info {
 	struct md_rdev	*rdev, *replacement;
 };
 
+struct raid5_auxth {
+	struct md_thread	*thread;
+	/* which CPUs should the auxiliary thread handle stripes from */
+	cpumask_t		work_mask;
+	struct kobject		kobj;
+	struct work_struct	del_work;
+};
+
 struct r5conf {
 	struct hlist_head	*stripe_hashtbl;
 	struct mddev		*mddev;

@@ -432,6 +441,12 @@ struct r5conf {
 					      * lists and performing address
 					      * conversions
 					      */
+		struct list_head handle_list;
+		cpumask_t	handle_threads; /* Which threads can the CPU's
+						 * stripes be handled. It really
+						 * is a bitmap to aux_threads[],
+						 * but has max bits NR_CPUS
+						 */
 	} __percpu *percpu;
 	size_t			scribble_len; /* size of scribble region must be
 					       * associated with conf to handle

@@ -459,6 +474,10 @@ struct r5conf {
 	 * the new thread here until we fully activate the array.
 	 */
 	struct md_thread	*thread;
+	int			aux_thread_num;
+	struct raid5_auxth	**aux_threads;
+	/* which CPUs should raid5d thread handle stripes from */
+	cpumask_t		work_mask;
 };
 
 /*

Index: linux/drivers/md/md.c
===================================================================

drivers/md/md.c +5 -3

@@ -640,9 +640,9 @@ static struct mddev * mddev_find(dev_t u
 	goto retry;
 }
 
-static inline int mddev_lock(struct mddev * mddev)
+int md_queue_misc_work(struct work_struct *work)
 {
-	return mutex_lock_interruptible(&mddev->reconfig_mutex);
+	return queue_work(md_misc_wq, work);
 }
 
 static inline int mddev_is_locked(struct mddev *mddev)

@@ -657,7 +657,7 @@ static inline int mddev_trylock(struct m
 
 static struct attribute_group md_redundancy_group;
 
-static void mddev_unlock(struct mddev * mddev)
+void mddev_unlock(struct mddev * mddev)
 {
 	if (mddev->to_remove) {
 		/* These cannot be removed under reconfig_mutex as

@@ -8569,6 +8569,8 @@ EXPORT_SYMBOL(md_register_thread);
 EXPORT_SYMBOL(md_unregister_thread);
 EXPORT_SYMBOL(md_wakeup_thread);
 EXPORT_SYMBOL(md_check_recovery);
+EXPORT_SYMBOL(mddev_unlock);
+EXPORT_SYMBOL(md_queue_misc_work);
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("MD RAID framework");
 MODULE_ALIAS("md");

Index: linux/drivers/md/md.h
===================================================================

drivers/md/md.h +8 -0

@@ -636,4 +636,12 @@ static inline int mddev_check_plugged(st
 	return !!blk_check_plugged(md_unplug, mddev,
 				   sizeof(struct blk_plug_cb));
 }
+
+static inline int mddev_lock(struct mddev * mddev)
+{
+	return mutex_lock_interruptible(&mddev->reconfig_mutex);
+}
+extern void mddev_unlock(struct mddev *mddev);
+extern int md_queue_misc_work(struct work_struct *work);
+
 #endif /* _MD_MD_H */

Hi Neil,

I just noticed this one is missed. It used to be in your branch, but get dropped some time. Is anything I missed? I can resend it if you like.

Thanks,
Shaohua

Quoted 19 lines · Shaohua Li

This is a new tempt to make raid5 handle stripes in multiple threads, as suggested by Neil to have maxium flexibility and better numa binding. It basically is a combination of my first and second generation patches. By default, no multiple thread is enabled (all stripes are handled by raid5d).

An example to enable multiple threads:
#echo 3 > /sys/block/md0/md/auxthread_number
This will create 3 auxiliary threads to handle stripes. The threads can run on any cpus and handle stripes produced by any cpus.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

If stripes produced by a CPU aren't handled by any auxiliary thread, such stripes will be handled by raid5d. Otherwise, raid5d doesn't handle any stripes.

On Thu, 7 Mar 2013 15:31:23 +0800 Shaohua Li <[email protected]> wrote:

Quoted 5 lines

Quoted 5 lines

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Hi Shaohua,
I still have this sitting in my queue, but I haven't had a chance to look at is properly yet - I'm sorry about that. I'll try to get to it soon.

Quoted 1 line · NeilBrown

On Thu, 7 Mar 2013 15:31:23 +0800 Shaohua Li <[email protected]> wrote:

...

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Would it not be better to use the existing cpusets infrastructure for this, instead of manually binding threads to specific cores or sets of cores?

Also, I understand the hot cache issue driving the desire to have a raid thread only process stripes created by its CPU. But what of the scenario where an HPC user pins application threads to cores and needs all the L1/L2 cache? Say this user has a dual socket 24 core NUMA system with 2 NUMA nodes per socket, 4 nodes total. Each NUMA node has 6 cores and shared L3 cache. The user pins 5 processes to 5 cores in each node, and wants to pin a raid thread to the remaining core in each node to handle the write IO generated by the 5 user threads on the node.

Does your patch series allow this? Using the above example, if the user creates 4 cpusets, can he assign a raid thread to that set and the thread will execute on any core in the set, and only that set, on any stripes created by any CPU in that set, and only that set?

The infrastructure for this already exists, has since 2004. And it seems is more flexible than what you've implemented here. I suggest we make use of it, as it is the kernel standard for doing such things.

See: http://man7.org/linux/man-pages/man7/cpuset.7.html

Quoted 3 lines

Hi Shaohua,
I still have this sitting in my queue, but I haven't had a chance to look at is properly yet - I'm sorry about that. I'll try to get to it soon.

Quoted 33 lines · On Tue, 12 Mar 2013 19:44:19 -0500 Stan Hoeppner <[email protected]> wrote:

Quoted 1 line · NeilBrown

On Thu, 7 Mar 2013 15:31:23 +0800 Shaohua Li <[email protected]> wrote:

...

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Would it not be better to use the existing cpusets infrastructure for this, instead of manually binding threads to specific cores or sets of cores?

Also, I understand the hot cache issue driving the desire to have a raid thread only process stripes created by its CPU. But what of the scenario where an HPC user pins application threads to cores and needs all the L1/L2 cache? Say this user has a dual socket 24 core NUMA system with 2 NUMA nodes per socket, 4 nodes total. Each NUMA node has 6 cores and shared L3 cache. The user pins 5 processes to 5 cores in each node, and wants to pin a raid thread to the remaining core in each node to handle the write IO generated by the 5 user threads on the node.

Does your patch series allow this? Using the above example, if the user creates 4 cpusets, can he assign a raid thread to that set and the thread will execute on any core in the set, and only that set, on any stripes created by any CPU in that set, and only that set?

The infrastructure for this already exists, has since 2004. And it seems is more flexible than what you've implemented here. I suggest we make use of it, as it is the kernel standard for doing such things.

See: http://man7.org/linux/man-pages/man7/cpuset.7.html

Quoted 4 lines

Quoted 3 lines

Hi Shaohua,
I still have this sitting in my queue, but I haven't had a chance to look at is properly yet - I'm sorry about that. I'll try to get to it soon.

Thanks for this feedback. The interface is the thing I am most concerned about getting right at this stage, and is exactly what you are commenting on.

The current code allows you to request N separate raid threads, and to tie each one to a subset of processors. This tying is in two senses. The thread can only run on cpus in the subset, and the requests queued by any given processor will preferentially be processed by threads tied to that processor.

It does sound a lot like cpusets could be used instead of lists of CPUs. However it does merge the two different cpuset concepts which you seem to suggest might not be ideal, and maybe it isn't.

A completely general solution might be to allow each thread to handle requests from one cpuset, and run on any processor in another cpuset. Would that be too much flexibility?

cpusets are a config option, so we would need to only enable multithreads if CONFIG_CPUSETS were set. Is this unnecessarily restrictive?

Are there any
other cases of kernel threads binding to cpusets? If there aren't I'd be a but cautious of being the first, I as have very little familiarity with this stuff.

I still like the idea of an 'ioctl' which a process can call and will cause it to start handling requests.

The process could bind itself to whatever cpu or cpuset it wanted to, then could call the ioctl on the relevant md array, and pass in a bitmap of cpus which indicate which requests it wants to be responsible for. The current kernel thread will then only handle requests that no-one else has put their hand up for. This leave all the details of configuration in user-space (where I think it belongs).

Shaohua - have you given any thought to that approach.

If anyone else has any familiarity with multithreading and numa and cpu affinity and would like to share some thoughts, I am all ears.

Quoted 27 lines · On Tue, 12 Mar 2013 19:44:19 -0500 Stan Hoeppner <[email protected]> wrote:

Quoted 1 line · NeilBrown

On Thu, 7 Mar 2013 15:31:23 +0800 Shaohua Li <[email protected]> wrote:

...

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist
This will bind auxiliary thread 0 to cpu 1-3, and this thread will only handle stripes produced by cpu 1-3. User tool can further change the thread's affinity, but the thread can only handle stripes produced by cpu 1-3 till the sysfs entry is changed again.

Would it not be better to use the existing cpusets infrastructure for this, instead of manually binding threads to specific cores or sets of cores?

Also, I understand the hot cache issue driving the desire to have a raid thread only process stripes created by its CPU. But what of the scenario where an HPC user pins application threads to cores and needs all the L1/L2 cache? Say this user has a dual socket 24 core NUMA system with 2 NUMA nodes per socket, 4 nodes total. Each NUMA node has 6 cores and shared L3 cache. The user pins 5 processes to 5 cores in each node, and wants to pin a raid thread to the remaining core in each node to handle the write IO generated by the 5 user threads on the node.

Does your patch series allow this? Using the above example, if the user creates 4 cpusets, can he assign a raid thread to that set and the thread will execute on any core in the set, and only that set, on any stripes created by any CPU in that set, and only that set?

Quoted 10 lines

The infrastructure for this already exists, has since 2004. And it seems is more flexible than what you've implemented here. I suggest we make use of it, as it is the kernel standard for doing such things.

See: http://man7.org/linux/man-pages/man7/cpuset.7.html

Quoted 3 lines

Hi Shaohua,
I still have this sitting in my queue, but I haven't had a chance to look at is properly yet - I'm sorry about that. I'll try to get to it soon.

Thanks for this feedback. The interface is the thing I am most concerned about getting right at this stage, and is exactly what you are commenting on.

The current code allows you to request N separate raid threads, and to tie each one to a subset of processors. This tying is in two senses. The thread can only run on cpus in the subset, and the requests queued by any given processor will preferentially be processed by threads tied to that processor.

It does sound a lot like cpusets could be used instead of lists of CPUs. However it does merge the two different cpuset concepts which you seem to suggest might not be ideal, and maybe it isn't.

A completely general solution might be to allow each thread to handle requests from one cpuset, and run on any processor in another cpuset. Would that be too much flexibility?

cpusets are a config option, so we would need to only enable multithreads if CONFIG_CPUSETS were set. Is this unnecessarily restrictive? Are there any other cases of kernel threads binding to cpusets? If there aren't I'd be a but cautious of being the first, I as have very little familiarity with this stuff.

I still like the idea of an 'ioctl' which a process can call and will cause it to start handling requests.
The process could bind itself to whatever cpu or cpuset it wanted to, then could call the ioctl on the relevant md array, and pass in a bitmap of cpus which indicate which requests it wants to be responsible for. The current kernel thread will then only handle requests that no-one else has put their hand up for. This leave all the details of configuration in user-space (where I think it belongs).

Frankly I don't like the cpuset way. It might just work, but it's just another API to control process affinity and has no essential difference against my approach (which directly sets process affinity). Generally we use cpuset instead of process affinity is because of something like inherit affinity. While the raid5 process doesn't involve those.

Quoted 8 lines

I still like the idea of an 'ioctl' which a process can call and will cause it to start handling requests.
The process could bind itself to whatever cpu or cpuset it wanted to, then could call the ioctl on the relevant md array, and pass in a bitmap of cpus which indicate which requests it wants to be responsible for. The current kernel thread will then only handle requests that no-one else has put their hand up for. This leave all the details of configuration in user-space (where I think it belongs).

The 'ioctl' way is interesting. But there are something we need answer:

1. How kernel knows if there will be process to handle one cpu's requests before an 'ioctl' is called? I suppose you want 2 ioctls. One ioctl telles kernel the process handles request from cpus of a cpumask. The other ioctl does request handling. The process must sleep in the ioctl to wait requests.

2. If a process is killed in the middle, how kernel knows? Do you want to hook something in task management code? For normal process exit, we need another ioctl to tell kernel the process is exiting.

The only difference between this way and my way is if the request handling task is userspace or kernel space. In both ways, you need set affinity and uses ioctl/sysfs to control requests source for the process.

I'm CC'ing Joe Landman as he's already building systems of the caliber that would benefit from this write threading and may need configurable CPU scheduling. Joe I've not seen a post from you on linux-raid in a while so I don't know if you've been following this topic. Shaohua has created patch sets to eliminate, or dramatically mitigate, the horrible single threaded write performance of md/RAID 1, 10, 5, 6 on SSD. Throughput no longer hits a wall due to peaking one core, as with the currently shipping kernel code. Your thoughts?

On 3/28/2013 9:34 PM, Shaohua Li wrote:
...

Quoted 5 lines

Frankly I don't like the cpuset way. It might just work, but it's just another API to control process affinity and has no essential difference against my approach (which directly sets process affinity). Generally we use cpuset instead of process affinity is because of something like inherit affinity. While the raid5 process doesn't involve those.

First I should again state I'm not a developer, but a sysadmin, and this is the viewpoint from which I speak.

The essential difference I see is the user interface the sysadmin will employ to tweak thread placement/behavior. Hypothetically, say I have a 64 socket Altix UV machine w/8 core CPUs, 512 cores. Each node board has two sockets, two distinct NUMA nodes, 64 total, but these share a NUMALink hub interface chip connection to the rest of the machine, and share a PCIe mezzanine interface.

We obviously want to keep md/RAID housekeeping bandwidth (stripe cache, RMW reads, etc) isolated to the node where it is attached so it doesn't needlessly traverse NUMALink eating precious, limited, 'high' latency NUMAlink system interconnect bandwidth. We need to keep that free for our parallel application which is eating 100% of the other 504 cores and saturating NUMAlink with MPI and file IO traffic.

So lets say I have one NUMA node out of 64 dedicated to block device IO.
It has a PCIe x8 v2 IB 4x QDR HBA (4GB/s) connection to a SAN box with
18 SSDs (and 128 SAS rust). The SAN RAID ASIC can't keep up with SSD RAID5 IO rates while also doing RAID for the rust. So we export the SSDs individually and we make 2x 9 drive md/RAID5 arrays. I've already created a cpuset with this NUMA node for strictly storage related processes including but not limited to XFS utils, backup processes, snapshots, etc, so that the only block IO traversing NUMAlink is user application data. Now I add another 18 SSDs to the SAN chassis, and another IB HBA to this node board.

Ideally, my md/RAID write threads should already be bound to this cpuset. So all I should need to do is add this 2nd node to the cpuset and I'm done. No need to monkey with additional md/RAID specific interfaces.

Now, that's the simple scenario. On this particular machine's architecture, you have two NUMA nodes per physical node, so expanding storage hardware on the same node board should be straightforward above.
However, most Altix UV machines will have storage HBAs plugged into
many node boards. If we create one cpuset and put all the md/RAID write theads in it, then we get housekeeping RAID IO traversing the NUMAlink interconnect. So in this case we'd want to pin the threads to the physical node board where the PCIe cards, and thus disks, are attached.

The 'easy' way to do this is simply create multiple cpusets, one for each storage node. But then you have the downside of administration headaches, because you may need to pin your FS utils, backup, etc to a different storage cpuset depending on which HBAs the filesystem resides, and do this each and every time, which is a nightmare with scheduled jobs. Thus in this case its probably best to retain the single storage cpuset and simply make sure the node boards share the same upstream switch hop, keeping the traffic as local as possible. The kernel scheduler might already have some NUMA scheduling intelligence here that works automagically even within a cpuset, to minimize this. I simply lack knowledge in this area.

Quoted 16 lines

Quoted 8 lines

I still like the idea of an 'ioctl' which a process can call and will cause it to start handling requests.
The process could bind itself to whatever cpu or cpuset it wanted to, then could call the ioctl on the relevant md array, and pass in a bitmap of cpus which indicate which requests it wants to be responsible for. The current kernel thread will then only handle requests that no-one else has put their hand up for. This leave all the details of configuration in user-space (where I think it belongs).

The 'ioctl' way is interesting. But there are something we need answer:

1. How kernel knows if there will be process to handle one cpu's requests before an 'ioctl' is called? I suppose you want 2 ioctls. One ioctl telles kernel the process handles request from cpus of a cpumask. The other ioctl does request handling. The process must sleep in the ioctl to wait requests.

Quoted 7 lines

2. If a process is killed in the middle, how kernel knows? Do you want to hook something in task management code? For normal process exit, we need another ioctl to tell kernel the process is exiting.

The only difference between this way and my way is if the request handling task is userspace or kernel space. In both ways, you need set affinity and uses ioctl/sysfs to control requests source for the process.

Being a non dev I lack requisite knowledge to comment on ioctls. I'll simply reiterate that whatever you go with should make use of an existing familiar user interface where this same scheduling is already handled, which is cpusets. The only difference being kernel vs user space. Which may turn out to be a problem, I dunno.

Quoted 95 lines · Stan Hoeppner

I'm CC'ing Joe Landman as he's already building systems of the caliber that would benefit from this write threading and may need configurable CPU scheduling. Joe I've not seen a post from you on linux-raid in a while so I don't know if you've been following this topic. Shaohua has created patch sets to eliminate, or dramatically mitigate, the horrible single threaded write performance of md/RAID 1, 10, 5, 6 on SSD. Throughput no longer hits a wall due to peaking one core, as with the currently shipping kernel code. Your thoughts?

On 3/28/2013 9:34 PM, Shaohua Li wrote:
...

Quoted 5 lines

Frankly I don't like the cpuset way. It might just work, but it's just another API to control process affinity and has no essential difference against my approach (which directly sets process affinity). Generally we use cpuset instead of process affinity is because of something like inherit affinity. While the raid5 process doesn't involve those.

First I should again state I'm not a developer, but a sysadmin, and this is the viewpoint from which I speak.

The essential difference I see is the user interface the sysadmin will employ to tweak thread placement/behavior. Hypothetically, say I have a 64 socket Altix UV machine w/8 core CPUs, 512 cores. Each node board has two sockets, two distinct NUMA nodes, 64 total, but these share a NUMALink hub interface chip connection to the rest of the machine, and share a PCIe mezzanine interface.

We obviously want to keep md/RAID housekeeping bandwidth (stripe cache, RMW reads, etc) isolated to the node where it is attached so it doesn't needlessly traverse NUMALink eating precious, limited, 'high' latency NUMAlink system interconnect bandwidth. We need to keep that free for our parallel application which is eating 100% of the other 504 cores and saturating NUMAlink with MPI and file IO traffic.

So lets say I have one NUMA node out of 64 dedicated to block device IO.
It has a PCIe x8 v2 IB 4x QDR HBA (4GB/s) connection to a SAN box with
18 SSDs (and 128 SAS rust). The SAN RAID ASIC can't keep up with SSD RAID5 IO rates while also doing RAID for the rust. So we export the SSDs individually and we make 2x 9 drive md/RAID5 arrays. I've already created a cpuset with this NUMA node for strictly storage related processes including but not limited to XFS utils, backup processes, snapshots, etc, so that the only block IO traversing NUMAlink is user application data. Now I add another 18 SSDs to the SAN chassis, and another IB HBA to this node board.

Ideally, my md/RAID write threads should already be bound to this cpuset. So all I should need to do is add this 2nd node to the cpuset and I'm done. No need to monkey with additional md/RAID specific interfaces.

Now, that's the simple scenario. On this particular machine's architecture, you have two NUMA nodes per physical node, so expanding storage hardware on the same node board should be straightforward above.
However, most Altix UV machines will have storage HBAs plugged into
many node boards. If we create one cpuset and put all the md/RAID write theads in it, then we get housekeeping RAID IO traversing the NUMAlink interconnect. So in this case we'd want to pin the threads to the physical node board where the PCIe cards, and thus disks, are attached.

The 'easy' way to do this is simply create multiple cpusets, one for each storage node. But then you have the downside of administration headaches, because you may need to pin your FS utils, backup, etc to a different storage cpuset depending on which HBAs the filesystem resides, and do this each and every time, which is a nightmare with scheduled jobs. Thus in this case its probably best to retain the single storage cpuset and simply make sure the node boards share the same upstream switch hop, keeping the traffic as local as possible. The kernel scheduler might already have some NUMA scheduling intelligence here that works automagically even within a cpuset, to minimize this. I simply lack knowledge in this area.

Quoted 23 lines

Quoted 8 lines

I still like the idea of an 'ioctl' which a process can call and will cause it to start handling requests.
The process could bind itself to whatever cpu or cpuset it wanted to, then could call the ioctl on the relevant md array, and pass in a bitmap of cpus which indicate which requests it wants to be responsible for. The current kernel thread will then only handle requests that no-one else has put their hand up for. This leave all the details of configuration in user-space (where I think it belongs).

The 'ioctl' way is interesting. But there are something we need answer:

1. How kernel knows if there will be process to handle one cpu's requests before an 'ioctl' is called? I suppose you want 2 ioctls. One ioctl telles kernel the process handles request from cpus of a cpumask. The other ioctl does request handling. The process must sleep in the ioctl to wait requests.

2. If a process is killed in the middle, how kernel knows? Do you want to hook something in task management code? For normal process exit, we need another ioctl to tell kernel the process is exiting.

The only difference between this way and my way is if the request handling task is userspace or kernel space. In both ways, you need set affinity and uses ioctl/sysfs to control requests source for the process.

Quoted 5 lines

Being a non dev I lack requisite knowledge to comment on ioctls. I'll simply reiterate that whatever you go with should make use of an existing familiar user interface where this same scheduling is already handled, which is cpusets. The only difference being kernel vs user space. Which may turn out to be a problem, I dunno.

Hmm, there might be misunderstanding here. In my way:

#echo 3 > /sys/block/md0/md/auxthread_number. Create several kernel threads to handle requests. You can use any approach to set smp affinity for the threads. You can use cpuset to bind the threads too.

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist. This doesn't set above threads' affinity. It sets which CPU's requests the thread should handle. Regardless using my way, cpuset or ioctl, we need the similar way to notify worker thread which CPU's requests it should handle (unless we have a hook in scheduler, when a thread's affinity is changed, we get a notification)

In the sumary, my approach doesn't prevent you to use CPUSET. Did I miss something?

Quoted 96 lines · Shaohua Li

Quoted 95 lines · Stan Hoeppner

I'm CC'ing Joe Landman as he's already building systems of the caliber that would benefit from this write threading and may need configurable CPU scheduling. Joe I've not seen a post from you on linux-raid in a while so I don't know if you've been following this topic. Shaohua has created patch sets to eliminate, or dramatically mitigate, the horrible single threaded write performance of md/RAID 1, 10, 5, 6 on SSD. Throughput no longer hits a wall due to peaking one core, as with the currently shipping kernel code. Your thoughts?

On 3/28/2013 9:34 PM, Shaohua Li wrote:
...

Quoted 5 lines

Frankly I don't like the cpuset way. It might just work, but it's just another API to control process affinity and has no essential difference against my approach (which directly sets process affinity). Generally we use cpuset instead of process affinity is because of something like inherit affinity. While the raid5 process doesn't involve those.

First I should again state I'm not a developer, but a sysadmin, and this is the viewpoint from which I speak.

The essential difference I see is the user interface the sysadmin will employ to tweak thread placement/behavior. Hypothetically, say I have a 64 socket Altix UV machine w/8 core CPUs, 512 cores. Each node board has two sockets, two distinct NUMA nodes, 64 total, but these share a NUMALink hub interface chip connection to the rest of the machine, and share a PCIe mezzanine interface.

We obviously want to keep md/RAID housekeeping bandwidth (stripe cache, RMW reads, etc) isolated to the node where it is attached so it doesn't needlessly traverse NUMALink eating precious, limited, 'high' latency NUMAlink system interconnect bandwidth. We need to keep that free for our parallel application which is eating 100% of the other 504 cores and saturating NUMAlink with MPI and file IO traffic.

So lets say I have one NUMA node out of 64 dedicated to block device IO.
It has a PCIe x8 v2 IB 4x QDR HBA (4GB/s) connection to a SAN box with
18 SSDs (and 128 SAS rust). The SAN RAID ASIC can't keep up with SSD RAID5 IO rates while also doing RAID for the rust. So we export the SSDs individually and we make 2x 9 drive md/RAID5 arrays. I've already created a cpuset with this NUMA node for strictly storage related processes including but not limited to XFS utils, backup processes, snapshots, etc, so that the only block IO traversing NUMAlink is user application data. Now I add another 18 SSDs to the SAN chassis, and another IB HBA to this node board.

Ideally, my md/RAID write threads should already be bound to this cpuset. So all I should need to do is add this 2nd node to the cpuset and I'm done. No need to monkey with additional md/RAID specific interfaces.

Now, that's the simple scenario. On this particular machine's architecture, you have two NUMA nodes per physical node, so expanding storage hardware on the same node board should be straightforward above.
However, most Altix UV machines will have storage HBAs plugged into
many node boards. If we create one cpuset and put all the md/RAID write theads in it, then we get housekeeping RAID IO traversing the NUMAlink interconnect. So in this case we'd want to pin the threads to the physical node board where the PCIe cards, and thus disks, are attached.

The 'easy' way to do this is simply create multiple cpusets, one for each storage node. But then you have the downside of administration headaches, because you may need to pin your FS utils, backup, etc to a different storage cpuset depending on which HBAs the filesystem resides, and do this each and every time, which is a nightmare with scheduled jobs. Thus in this case its probably best to retain the single storage cpuset and simply make sure the node boards share the same upstream switch hop, keeping the traffic as local as possible. The kernel scheduler might already have some NUMA scheduling intelligence here that works automagically even within a cpuset, to minimize this. I simply lack knowledge in this area.

Quoted 23 lines

Quoted 8 lines

I still like the idea of an 'ioctl' which a process can call and will cause it to start handling requests.
The process could bind itself to whatever cpu or cpuset it wanted to, then could call the ioctl on the relevant md array, and pass in a bitmap of cpus which indicate which requests it wants to be responsible for. The current kernel thread will then only handle requests that no-one else has put their hand up for. This leave all the details of configuration in user-space (where I think it belongs).

The 'ioctl' way is interesting. But there are something we need answer:

1. How kernel knows if there will be process to handle one cpu's requests before an 'ioctl' is called? I suppose you want 2 ioctls. One ioctl telles kernel the process handles request from cpus of a cpumask. The other ioctl does request handling. The process must sleep in the ioctl to wait requests.

2. If a process is killed in the middle, how kernel knows? Do you want to hook something in task management code? For normal process exit, we need another ioctl to tell kernel the process is exiting.

The only difference between this way and my way is if the request handling task is userspace or kernel space. In both ways, you need set affinity and uses ioctl/sysfs to control requests source for the process.

Quoted 7 lines

Quoted 5 lines

Being a non dev I lack requisite knowledge to comment on ioctls. I'll simply reiterate that whatever you go with should make use of an existing familiar user interface where this same scheduling is already handled, which is cpusets. The only difference being kernel vs user space. Which may turn out to be a problem, I dunno.

Hmm, there might be misunderstanding here. In my way:

Very likely.

Quoted 3 lines

#echo 3 > /sys/block/md0/md/auxthread_number. Create several kernel threads to handle requests. You can use any approach to set smp affinity for the threads. You can use cpuset to bind the threads too.

So you have verified that these kernel threads can be placed by the cpuset calls and shell commands? Cool, then we're over one hurdle, so to speak. So say I create 8 threads with a boot script. I want to place 4 each in 2 different cpusets. Will this work be left for every sysadmin to figure out and create him/herself, or will you include scripts/docs/etc to facilitate this integration?

Quoted 5 lines

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist. This doesn't set above threads' affinity. It sets which CPU's requests the thread should handle. Regardless using my way, cpuset or ioctl, we need the similar way to notify worker thread which CPU's requests it should handle (unless we have a hook in scheduler, when a thread's affinity is changed, we get a notification)

I don't even know if this is necessary. From a NUMA perspective, and all systems are now NUMA, it's far more critical to make sure a RAID thread is executing on a core/socket to which the HBA is attached via the PCIe bridge. You should make it a priority to write code to identify this path and automatically set RAID thread affinity to that set of cores. This keeps the extra mirror and parity write data, RMW read data, and stripe cache accesses off the NUMA interconnect, as I stated in a previous email. This is critical to system performance, no matter how large or small the system.

Once this is accomplished, I see zero downside, from a NUMA standpoint, to having every RAID thread be able to service every core. Obviously this would require some kind of hashing so we don't generate hot spots.
Does your code already prevent this? Anyway, I think you can simply
eliminate this tunable parm altogether.

On that note, it would make sense to modify every md/RAID driver to participate in this hashing. Users run multiple RAID levels on a given box, and we want the bandwidth and CPU load spread as evenly as possible I would think.

Quoted 1 line

In the sumary, my approach doesn't prevent you to use CPUSET. Did I miss something?

IMO, it's not enough to simply make it work with cpusets, but to get some seamless integration. Now that I think more about this, it should be possible to get optimal affinity automatically by identifying the attachment point of the HBA(s), and sticking all RAID threads to cores on that socket. If the optimal number of threads to create could be calculated for any system, you could eliminate all of these tunables, and everything be be fully automatic. No need for user defined parms, and no need for cpusets.

Quoted 111 lines · Stan Hoeppner

Quoted 96 lines · Shaohua Li

Quoted 95 lines · Stan Hoeppner

I'm CC'ing Joe Landman as he's already building systems of the caliber that would benefit from this write threading and may need configurable CPU scheduling. Joe I've not seen a post from you on linux-raid in a while so I don't know if you've been following this topic. Shaohua has created patch sets to eliminate, or dramatically mitigate, the horrible single threaded write performance of md/RAID 1, 10, 5, 6 on SSD. Throughput no longer hits a wall due to peaking one core, as with the currently shipping kernel code. Your thoughts?

On 3/28/2013 9:34 PM, Shaohua Li wrote:
...

Quoted 5 lines

Frankly I don't like the cpuset way. It might just work, but it's just another API to control process affinity and has no essential difference against my approach (which directly sets process affinity). Generally we use cpuset instead of process affinity is because of something like inherit affinity. While the raid5 process doesn't involve those.

First I should again state I'm not a developer, but a sysadmin, and this is the viewpoint from which I speak.

The essential difference I see is the user interface the sysadmin will employ to tweak thread placement/behavior. Hypothetically, say I have a 64 socket Altix UV machine w/8 core CPUs, 512 cores. Each node board has two sockets, two distinct NUMA nodes, 64 total, but these share a NUMALink hub interface chip connection to the rest of the machine, and share a PCIe mezzanine interface.

We obviously want to keep md/RAID housekeeping bandwidth (stripe cache, RMW reads, etc) isolated to the node where it is attached so it doesn't needlessly traverse NUMALink eating precious, limited, 'high' latency NUMAlink system interconnect bandwidth. We need to keep that free for our parallel application which is eating 100% of the other 504 cores and saturating NUMAlink with MPI and file IO traffic.

So lets say I have one NUMA node out of 64 dedicated to block device IO.
It has a PCIe x8 v2 IB 4x QDR HBA (4GB/s) connection to a SAN box with
18 SSDs (and 128 SAS rust). The SAN RAID ASIC can't keep up with SSD RAID5 IO rates while also doing RAID for the rust. So we export the SSDs individually and we make 2x 9 drive md/RAID5 arrays. I've already created a cpuset with this NUMA node for strictly storage related processes including but not limited to XFS utils, backup processes, snapshots, etc, so that the only block IO traversing NUMAlink is user application data. Now I add another 18 SSDs to the SAN chassis, and another IB HBA to this node board.

Ideally, my md/RAID write threads should already be bound to this cpuset. So all I should need to do is add this 2nd node to the cpuset and I'm done. No need to monkey with additional md/RAID specific interfaces.

Now, that's the simple scenario. On this particular machine's architecture, you have two NUMA nodes per physical node, so expanding storage hardware on the same node board should be straightforward above.
However, most Altix UV machines will have storage HBAs plugged into
many node boards. If we create one cpuset and put all the md/RAID write theads in it, then we get housekeeping RAID IO traversing the NUMAlink interconnect. So in this case we'd want to pin the threads to the physical node board where the PCIe cards, and thus disks, are attached.

The 'easy' way to do this is simply create multiple cpusets, one for each storage node. But then you have the downside of administration headaches, because you may need to pin your FS utils, backup, etc to a different storage cpuset depending on which HBAs the filesystem resides, and do this each and every time, which is a nightmare with scheduled jobs. Thus in this case its probably best to retain the single storage cpuset and simply make sure the node boards share the same upstream switch hop, keeping the traffic as local as possible. The kernel scheduler might already have some NUMA scheduling intelligence here that works automagically even within a cpuset, to minimize this. I simply lack knowledge in this area.

Quoted 23 lines

Quoted 8 lines

I still like the idea of an 'ioctl' which a process can call and will cause it to start handling requests.
The process could bind itself to whatever cpu or cpuset it wanted to, then could call the ioctl on the relevant md array, and pass in a bitmap of cpus which indicate which requests it wants to be responsible for. The current kernel thread will then only handle requests that no-one else has put their hand up for. This leave all the details of configuration in user-space (where I think it belongs).

The 'ioctl' way is interesting. But there are something we need answer:

1. How kernel knows if there will be process to handle one cpu's requests before an 'ioctl' is called? I suppose you want 2 ioctls. One ioctl telles kernel the process handles request from cpus of a cpumask. The other ioctl does request handling. The process must sleep in the ioctl to wait requests.

2. If a process is killed in the middle, how kernel knows? Do you want to hook something in task management code? For normal process exit, we need another ioctl to tell kernel the process is exiting.

The only difference between this way and my way is if the request handling task is userspace or kernel space. In both ways, you need set affinity and uses ioctl/sysfs to control requests source for the process.

Quoted 7 lines

Quoted 5 lines

Being a non dev I lack requisite knowledge to comment on ioctls. I'll simply reiterate that whatever you go with should make use of an existing familiar user interface where this same scheduling is already handled, which is cpusets. The only difference being kernel vs user space. Which may turn out to be a problem, I dunno.

Hmm, there might be misunderstanding here. In my way:

Very likely.

Quoted 3 lines

#echo 3 > /sys/block/md0/md/auxthread_number. Create several kernel threads to handle requests. You can use any approach to set smp affinity for the threads. You can use cpuset to bind the threads too.

Quoted 6 lines

So you have verified that these kernel threads can be placed by the cpuset calls and shell commands? Cool, then we're over one hurdle, so to speak. So say I create 8 threads with a boot script. I want to place 4 each in 2 different cpusets. Will this work be left for every sysadmin to figure out and create him/herself, or will you include scripts/docs/etc to facilitate this integration?

Sure, verified cpuset can apply to kernel threads.

No, I don't have scripts.

Quoted 29 lines

Quoted 5 lines

#echo 1-3 > /sys/block/md0/md/auxth0/cpulist. This doesn't set above threads' affinity. It sets which CPU's requests the thread should handle. Regardless using my way, cpuset or ioctl, we need the similar way to notify worker thread which CPU's requests it should handle (unless we have a hook in scheduler, when a thread's affinity is changed, we get a notification)

I don't even know if this is necessary. From a NUMA perspective, and all systems are now NUMA, it's far more critical to make sure a RAID thread is executing on a core/socket to which the HBA is attached via the PCIe bridge. You should make it a priority to write code to identify this path and automatically set RAID thread affinity to that set of cores. This keeps the extra mirror and parity write data, RMW read data, and stripe cache accesses off the NUMA interconnect, as I stated in a previous email. This is critical to system performance, no matter how large or small the system.

Once this is accomplished, I see zero downside, from a NUMA standpoint, to having every RAID thread be able to service every core. Obviously this would require some kind of hashing so we don't generate hot spots.
Does your code already prevent this? Anyway, I think you can simply
eliminate this tunable parm altogether.

On that note, it would make sense to modify every md/RAID driver to participate in this hashing. Users run multiple RAID levels on a given box, and we want the bandwidth and CPU load spread as evenly as possible I would think.

Quoted 1 line

In the sumary, my approach doesn't prevent you to use CPUSET. Did I miss something?

Quoted 8 lines

IMO, it's not enough to simply make it work with cpusets, but to get some seamless integration. Now that I think more about this, it should be possible to get optimal affinity automatically by identifying the attachment point of the HBA(s), and sticking all RAID threads to cores on that socket. If the optimal number of threads to create could be calculated for any system, you could eliminate all of these tunables, and everything be be fully automatic. No need for user defined parms, and no need for cpusets.

I understand. It's always preferred everything is automatic set with best performance. But last time I checked, different optimal thread number applies in different setup/workload. After some discussions, we decided to add some tunables. This isn't convenient from user point of view, but it's hard to determine the optimal tunable value.