Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1412979 > unrolled thread
| Started by | Pan Xinhui <xinhui.pan@linux.vnet.ibm.com> |
|---|---|
| First post | 2016-06-03 11:50 +0200 |
| Last post | 2016-06-06 06:50 +0200 |
| Articles | 6 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec Pan Xinhui <xinhui.pan@linux.vnet.ibm.com> - 2016-06-03 11:50 +0200
Re: [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec Peter Zijlstra <peterz@infradead.org> - 2016-06-03 13:40 +0200
Re: [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec xinhui <xinhui.pan@linux.vnet.ibm.com> - 2016-06-06 05:30 +0200
Re: [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec Waiman Long <waiman.long@hpe.com> - 2016-06-03 23:30 +0200
Re: [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec Peter Zijlstra <peterz@infradead.org> - 2016-06-03 23:40 +0200
Re: [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec xinhui <xinhui.pan@linux.vnet.ibm.com> - 2016-06-06 06:50 +0200
| From | Pan Xinhui <xinhui.pan@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-06-03 11:50 +0200 |
| Subject | [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec |
| Message-ID | <rFTTQ-1rX-9@gated-at.bofh.it> |
queued_spin_lock_slowpath should not worry about interrupt change node->count by accident because ->count is inc and dec when we enter/leave queued_spin_lock_slowpath. So this_cpu_dec() does some no point things here, lets use this_cpu_ptr for a small optimization. Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com> --- kernel/locking/qspinlock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c index 99f31e4..2b4daac 100644 --- a/kernel/locking/qspinlock.c +++ b/kernel/locking/qspinlock.c @@ -492,7 +492,7 @@ release: /* * release the node */ - this_cpu_dec(mcs_nodes[0].count); + this_cpu_ptr(&mcs_nodes[0])->count--; } EXPORT_SYMBOL(queued_spin_lock_slowpath); -- 1.9.1
[toc] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-06-03 13:40 +0200 |
| Subject | Re: [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec |
| Message-ID | <rFVCh-2xA-5@gated-at.bofh.it> |
| In reply to | #1412979 |
On Fri, Jun 03, 2016 at 05:48:50PM +0800, Pan Xinhui wrote: > queued_spin_lock_slowpath should not worry about interrupt change > node->count by accident because ->count is inc and dec when we > enter/leave queued_spin_lock_slowpath. > > So this_cpu_dec() does some no point things here, lets use this_cpu_ptr > for a small optimization. Uhm, have you actually looked at what that does on x86?
[toc] | [prev] | [next] | [standalone]
| From | xinhui <xinhui.pan@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-06-06 05:30 +0200 |
| Message-ID | <rGToK-7AE-27@gated-at.bofh.it> |
| In reply to | #1413145 |
On 2016年06月03日 19:37, Peter Zijlstra wrote: > On Fri, Jun 03, 2016 at 05:48:50PM +0800, Pan Xinhui wrote: >> queued_spin_lock_slowpath should not worry about interrupt change >> node->count by accident because ->count is inc and dec when we >> enter/leave queued_spin_lock_slowpath. >> >> So this_cpu_dec() does some no point things here, lets use this_cpu_ptr >> for a small optimization. > > Uhm, have you actually looked at what that does on x86? > yep, just one instruction inc/dec. well, with my patch, there are two instructions.
[toc] | [prev] | [next] | [standalone]
| From | Waiman Long <waiman.long@hpe.com> |
|---|---|
| Date | 2016-06-03 23:30 +0200 |
| Message-ID | <rG4Pf-8ss-5@gated-at.bofh.it> |
| In reply to | #1412979 |
On 06/03/2016 05:48 AM, Pan Xinhui wrote: > queued_spin_lock_slowpath should not worry about interrupt change > node->count by accident because ->count is inc and dec when we > enter/leave queued_spin_lock_slowpath. > > So this_cpu_dec() does some no point things here, lets use this_cpu_ptr > for a small optimization. > > Signed-off-by: Pan Xinhui<xinhui.pan@linux.vnet.ibm.com> > --- > kernel/locking/qspinlock.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c > index 99f31e4..2b4daac 100644 > --- a/kernel/locking/qspinlock.c > +++ b/kernel/locking/qspinlock.c > @@ -492,7 +492,7 @@ release: > /* > * release the node > */ > - this_cpu_dec(mcs_nodes[0].count); > + this_cpu_ptr(&mcs_nodes[0])->count--; > } > EXPORT_SYMBOL(queued_spin_lock_slowpath); > Is this going to generate better code for PPC? For x86, I think it will cause more instruction to be issued. Cheers, Longman
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2016-06-03 23:40 +0200 |
| Subject | Re: [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec |
| Message-ID | <rG4YV-8vw-5@gated-at.bofh.it> |
| In reply to | #1413495 |
On Fri, Jun 03, 2016 at 05:20:10PM -0400, Waiman Long wrote: > On 06/03/2016 05:48 AM, Pan Xinhui wrote: > >queued_spin_lock_slowpath should not worry about interrupt change > >node->count by accident because ->count is inc and dec when we > >enter/leave queued_spin_lock_slowpath. > > > >So this_cpu_dec() does some no point things here, lets use this_cpu_ptr > >for a small optimization. > > > >Signed-off-by: Pan Xinhui<xinhui.pan@linux.vnet.ibm.com> > >--- > > kernel/locking/qspinlock.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c > >index 99f31e4..2b4daac 100644 > >--- a/kernel/locking/qspinlock.c > >+++ b/kernel/locking/qspinlock.c > >@@ -492,7 +492,7 @@ release: > > /* > > * release the node > > */ > >- this_cpu_dec(mcs_nodes[0].count); > >+ this_cpu_ptr(&mcs_nodes[0])->count--; > > } > > EXPORT_SYMBOL(queued_spin_lock_slowpath); > > > > Is this going to generate better code for PPC? For x86, I think it will > cause more instruction to be issued. It does; I think he wants __this_cpu_dec() instead, but the Changelog needs improvement to explain why that is ok.
[toc] | [prev] | [next] | [standalone]
| From | xinhui <xinhui.pan@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-06-06 06:50 +0200 |
| Message-ID | <rGUEa-7u-7@gated-at.bofh.it> |
| In reply to | #1413500 |
On 2016年06月04日 05:35, Peter Zijlstra wrote: > On Fri, Jun 03, 2016 at 05:20:10PM -0400, Waiman Long wrote: >> On 06/03/2016 05:48 AM, Pan Xinhui wrote: >>> queued_spin_lock_slowpath should not worry about interrupt change >>> node->count by accident because ->count is inc and dec when we >>> enter/leave queued_spin_lock_slowpath. >>> >>> So this_cpu_dec() does some no point things here, lets use this_cpu_ptr >>> for a small optimization. >>> >>> Signed-off-by: Pan Xinhui<xinhui.pan@linux.vnet.ibm.com> >>> --- >>> kernel/locking/qspinlock.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c >>> index 99f31e4..2b4daac 100644 >>> --- a/kernel/locking/qspinlock.c >>> +++ b/kernel/locking/qspinlock.c >>> @@ -492,7 +492,7 @@ release: >>> /* >>> * release the node >>> */ >>> - this_cpu_dec(mcs_nodes[0].count); >>> + this_cpu_ptr(&mcs_nodes[0])->count--; >>> } >>> EXPORT_SYMBOL(queued_spin_lock_slowpath); >>> >> >> Is this going to generate better code for PPC? For x86, I think it will >> cause more instruction to be issued. > yes, ppc will do some check when restore irq flags. it's really heavy. and yes, such change cause more instructions to be issued. there is a RELOC_HIDE macro in this_cpu_ptr and gcc can't do optimization. How about ICC. I once used ICC when I was in intel but i forgot the result. > It does; I think he wants __this_cpu_dec() instead, but the Changelog > needs improvement to explain why that is ok. > oh, great. __this_cpu_dec() is fine. thanks no any irq flags save/restore again. :) thanks xinhui
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web