* [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec
@ 2016-06-03 9:48 Pan Xinhui
2016-06-03 11:37 ` Peter Zijlstra
2016-06-03 21:20 ` Waiman Long
0 siblings, 2 replies; 6+ messages in thread
From: Pan Xinhui @ 2016-06-03 9:48 UTC (permalink / raw)
To: linux-kernel; +Cc: mingo, peterz, waiman.long, Pan Xinhui
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec
2016-06-03 9:48 [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec Pan Xinhui
@ 2016-06-03 11:37 ` Peter Zijlstra
2016-06-06 3:21 ` xinhui
2016-06-03 21:20 ` Waiman Long
1 sibling, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2016-06-03 11:37 UTC (permalink / raw)
To: Pan Xinhui; +Cc: linux-kernel, mingo, waiman.long
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec
2016-06-03 11:37 ` Peter Zijlstra
@ 2016-06-06 3:21 ` xinhui
0 siblings, 0 replies; 6+ messages in thread
From: xinhui @ 2016-06-06 3:21 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: linux-kernel, mingo, waiman.long
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec
2016-06-03 9:48 [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec Pan Xinhui
2016-06-03 11:37 ` Peter Zijlstra
@ 2016-06-03 21:20 ` Waiman Long
2016-06-03 21:35 ` Peter Zijlstra
1 sibling, 1 reply; 6+ messages in thread
From: Waiman Long @ 2016-06-03 21:20 UTC (permalink / raw)
To: Pan Xinhui; +Cc: linux-kernel, mingo, peterz
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec
2016-06-03 21:20 ` Waiman Long
@ 2016-06-03 21:35 ` Peter Zijlstra
2016-06-06 4:48 ` xinhui
0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2016-06-03 21:35 UTC (permalink / raw)
To: Waiman Long; +Cc: Pan Xinhui, linux-kernel, mingo
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec
2016-06-03 21:35 ` Peter Zijlstra
@ 2016-06-06 4:48 ` xinhui
0 siblings, 0 replies; 6+ messages in thread
From: xinhui @ 2016-06-06 4:48 UTC (permalink / raw)
To: Peter Zijlstra, Waiman Long; +Cc: linux-kernel, mingo
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-06 4:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-03 9:48 [PATCH] locking/qspinlock: Use this_cpu_ptr instead of this_cpu_dec Pan Xinhui
2016-06-03 11:37 ` Peter Zijlstra
2016-06-06 3:21 ` xinhui
2016-06-03 21:20 ` Waiman Long
2016-06-03 21:35 ` Peter Zijlstra
2016-06-06 4:48 ` xinhui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).