Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1324911 > unrolled thread

Re: [RFC PATCH] alispinlock: acceleration from lock integration on multi-core platform

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH] alispinlock: acceleration from lock integration on  multi-core platform Ling Ma <ling.ma.program@gmail.com> - 2016-02-03 05:50 +0100
    Re: [RFC PATCH] alispinlock: acceleration from lock integration on  multi-core platform Ling Ma <ling.ma.program@gmail.com> - 2016-02-03 07:10 +0100
    Re: [RFC PATCH] alispinlock: acceleration from lock integration on  multi-core platform Waiman Long <waiman.long@hpe.com> - 2016-02-03 22:50 +0100
      Re: [RFC PATCH] alispinlock: acceleration from lock integration on  multi-core platform Ling Ma <ling.ma.program@gmail.com> - 2016-02-04 08:10 +0100

#1324911 — Re: [RFC PATCH] alispinlock: acceleration from lock integration on multi-core platform

[Multipart message — attachments visible in raw view] — view raw

Longman,

The attachment include user space code(thread.c), and kernel
patch(ali_work_queue.patch) based on 4.3.0-rc4,
we replaced all original spinlock (list_lock) in slab.h/c  with the
new mechanism.

The thread.c in user space caused lots of hot kernel spinlock from
__kmalloc and kfree,
perf top -d1 shows ~25%  before ali_work_queue.patch,after appending
this patch ,
the synchronous operation consumption from __kmalloc and kfree is
reduced from 25% to ~15% on Intel E5-2699V3
(we also observed the output from user space code (thread.c) is
improved clearly)

Peter, we will send the update version according to your comments.

Thanks
Ling


2016-01-19 23:36 GMT+08:00 Waiman Long <waiman.long@hpe.com>:
> On 01/19/2016 03:52 AM, Ling Ma wrote:
>>
>> Is it acceptable for performance improvement or more comments on this
>> patch?
>>
>> Thanks
>> Ling
>>
>>
>
> Your alispinlock patchset should also include a use case where the lock is
> used by some code within the kernel with test that can show a performance
> improvement so that the reviewers can independently try it out and play
> around with it. The kernel community will not accept any patch without a use
> case in the kernel.
>
> Your lock_test.tar file is not good enough as it is not a performance test
> of the patch that you sent out.
>
> Cheers,
> Longman

[toc] | [next] | [standalone]

#1324943

[Multipart message — attachments visible in raw view] — view raw

The attachment(thread.c) can tell us the new mechanism improve output
from  the user space code (thread,c) by 1.14x (1174810406/1026910602,
kernel spinlock consumption is reduced from 25% to 15%) as below:

  ORG         NEW
38186815 43644156
38340186 43121265
38383155 44087753
38567102 43532586
38027878 43622700
38011581 43396376
37861959 43322857
37963215 43375528
38039247 43618315
37989106 43406187
37916912 44163029
39053184 43138581
37928359 43247866
37967417 43390352
37909796 43218250
37727531 43256009
38032818 43460496
38001860 43536100
38019929 44231331
37846621 43550597
37823231 44229887
38108158 43142689
37771900 43228168
37652536 43901042
37649114 43172690
37591314 43380004
38539678 43435592

Total 1026910602 1174810406

Thanks
Ling

2016-02-03 12:40 GMT+08:00 Ling Ma <ling.ma.program@gmail.com>:
> Longman,
>
> The attachment include user space code(thread.c), and kernel
> patch(ali_work_queue.patch) based on 4.3.0-rc4,
> we replaced all original spinlock (list_lock) in slab.h/c  with the
> new mechanism.
>
> The thread.c in user space caused lots of hot kernel spinlock from
> __kmalloc and kfree,
> perf top -d1 shows ~25%  before ali_work_queue.patch,after appending
> this patch ,
> the synchronous operation consumption from __kmalloc and kfree is
> reduced from 25% to ~15% on Intel E5-2699V3
> (we also observed the output from user space code (thread.c) is
> improved clearly)
>
> Peter, we will send the update version according to your comments.
>
> Thanks
> Ling
>
>
> 2016-01-19 23:36 GMT+08:00 Waiman Long <waiman.long@hpe.com>:
>> On 01/19/2016 03:52 AM, Ling Ma wrote:
>>>
>>> Is it acceptable for performance improvement or more comments on this
>>> patch?
>>>
>>> Thanks
>>> Ling
>>>
>>>
>>
>> Your alispinlock patchset should also include a use case where the lock is
>> used by some code within the kernel with test that can show a performance
>> improvement so that the reviewers can independently try it out and play
>> around with it. The kernel community will not accept any patch without a use
>> case in the kernel.
>>
>> Your lock_test.tar file is not good enough as it is not a performance test
>> of the patch that you sent out.
>>
>> Cheers,
>> Longman

[toc] | [prev] | [next] | [standalone]

#1325988

On 02/02/2016 11:40 PM, Ling Ma wrote:
> Longman,
>
> The attachment include user space code(thread.c), and kernel
> patch(ali_work_queue.patch) based on 4.3.0-rc4,
> we replaced all original spinlock (list_lock) in slab.h/c  with the
> new mechanism.
>
> The thread.c in user space caused lots of hot kernel spinlock from
> __kmalloc and kfree,
> perf top -d1 shows ~25%  before ali_work_queue.patch,after appending
> this patch ,
> the synchronous operation consumption from __kmalloc and kfree is
> reduced from 25% to ~15% on Intel E5-2699V3
> (we also observed the output from user space code (thread.c) is
> improved clearly)

I have 2 major comments here. First of all, you should break up your 
patch into smaller ones. Large patch like the one in the tar ball is 
hard to review. Secondly, you are modifying over 1000 lines of code in 
mm/slab.c with some modest increase in performance. That can be hard to 
justify. Maybe you should find other use cases that involve less 
changes, but still have noticeable performance improvement. That will 
make it easier to be accepted.

Cheers,
Longman

[toc] | [prev] | [next] | [standalone]

#1326451

[Multipart message — attachments visible in raw view] — view raw

> I have 2 major comments here. First of all, you should break up your patch
> into smaller ones. Large patch like the one in the tar ball is hard to
> review.

Ok, we will do it.

>Secondly, you are modifying over 1000 lines of code in mm/slab.c
> with some modest increase in performance. That can be hard to justify. Maybe
> you should find other use cases that involve less changes, but still have
> noticeable performance improvement. That will make it easier to be accepted.

In order to be justified the attachment in this letter include 3 files:

1. user space code (thread.c), which  can cause lots of hot kernel spinlock from
__kmalloc and kfree on multi-core platform

2. ali_work_queue.patch , the kernel patch for 4.3.0-rc4,
when we  run user space code (thread.c) based on the patch,
the synchronous operation consumption from __kmalloc and kfree is
about 15% on Intel E5-2699V3

3. org_spin_lock.patch, which is based on above ali_work_queue.patch,
when we  run user space code thread.c based on the patch,
the synchronous operation consumption from __kmalloc and kfree is
about 25% on Intel E5-2699V3


the main difference between ali_work_queue.patch and
org_spin_lock.patch as below:

diff --git a/mm/slab.h b/mm/slab.h
...
-   ali_spinlock_t list_lock;
+   spinlock_t list_lock;
...

diff --git a/mm/slab.c b/mm/slab.c
...
-   alispinlock(lock, &info);
+   spin_lock((spinlock_t *)lock);
+   fn(para);
+   spin_unlock((spinlock_t *)lock);
...

The above operations remove all performance noise from program modification.

We run  user space code thread.c with ali_work_queue.patch, and
org_spin_lock.patch respectively
 the output from thread.c as below:

ORG         NEW
38923684 43380604
38100464 44163011
37769241 43354266
37908638 43554022
37900994 43457066
38495073 43421394
37340217 43146352
38083979 43506951
37713263 43775215
37749871 43487289
37843224 43366055
38173823 43270225
38303612 43214675
37886717 44083950
37736455 43060728
37529307 44607597
38862690 43541484
37992824 44749925
38013454 43572225
37783135 45240502
37745372 44712540
38721413 43584658
38097842 43235392

            ORG            NEW
TOTAL 874675292 1005486126

So the data tell us the new mechanism can improve performance 14% (
1005486126/874675292) ,
and the operation can be justified fairly.

Thanks
Ling

2016-02-04 5:42 GMT+08:00 Waiman Long <waiman.long@hpe.com>:
> On 02/02/2016 11:40 PM, Ling Ma wrote:
>>
>> Longman,
>>
>> The attachment include user space code(thread.c), and kernel
>> patch(ali_work_queue.patch) based on 4.3.0-rc4,
>> we replaced all original spinlock (list_lock) in slab.h/c  with the
>> new mechanism.
>>
>> The thread.c in user space caused lots of hot kernel spinlock from
>> __kmalloc and kfree,
>> perf top -d1 shows ~25%  before ali_work_queue.patch,after appending
>> this patch ,
>> the synchronous operation consumption from __kmalloc and kfree is
>> reduced from 25% to ~15% on Intel E5-2699V3
>> (we also observed the output from user space code (thread.c) is
>> improved clearly)
>
>
> I have 2 major comments here. First of all, you should break up your patch
> into smaller ones. Large patch like the one in the tar ball is hard to
> review. Secondly, you are modifying over 1000 lines of code in mm/slab.c
> with some modest increase in performance. That can be hard to justify. Maybe
> you should find other use cases that involve less changes, but still have
> noticeable performance improvement. That will make it easier to be accepted.
>
> Cheers,
> Longman
>
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web