From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-09-16 08:57:59
Implementing busy wait loops with cpu_relax() in callers poses
some difficulties for powerpc.
First, we want to put our SMT thread into a low priority mode for the
duration of the loop, but then return to normal priority after exiting
the loop. Dependong on the CPU design, 'HMT_low() ; HMT_medium();' as
cpu_relax() does may have HMT_medium take effect before HMT_low made
any (or much) difference.
Second, it can be beneficial for some implementations to spin on the
exit condition with a statically predicted-not-taken branch (i.e.,
always predict the loop will exit).
This is a quick RFC with a couple of users converted to see what
people think. I don't use a C branch with hints, because we don't want
the compiler moving the loop body out of line, which makes it a bit
messy unfortunately. If there's a better way to do it, I'm all ears.
I would not propose to switch all callers immediately, just some
core synchronisation primitives.
---
arch/powerpc/include/asm/processor.h | 22 ++++++++++++++++++++++
include/asm-generic/barrier.h | 7 ++-----
include/linux/bit_spinlock.h | 5 ++---
include/linux/cgroup.h | 7 ++-----
include/linux/seqlock.h | 10 ++++------
5 files changed, 32 insertions(+), 19 deletions(-)
From: David Laight <hidden> Date: 2016-09-16 11:44:28
From: Nicholas Piggin
Sent: 16 September 2016 09:58
Implementing busy wait loops with cpu_relax() in callers poses
some difficulties for powerpc.
=20
First, we want to put our SMT thread into a low priority mode for the
duration of the loop, but then return to normal priority after exiting
the loop. Dependong on the CPU design, 'HMT_low() ; HMT_medium();' as
cpu_relax() does may have HMT_medium take effect before HMT_low made
any (or much) difference.
=20
Second, it can be beneficial for some implementations to spin on the
exit condition with a statically predicted-not-taken branch (i.e.,
always predict the loop will exit).
=20
This is a quick RFC with a couple of users converted to see what
people think. I don't use a C branch with hints, because we don't want
the compiler moving the loop body out of line, which makes it a bit
messy unfortunately. If there's a better way to do it, I'm all ears.
I think it will still all go wrong if the conditional isn't trivial.
In particular if the condition contains || or && it is likely to
have a branch - which could invert the loop.
David
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-09-16 11:52:10
On Fri, 16 Sep 2016 11:30:58 +0000
David Laight [off-list ref] wrote:
From: Nicholas Piggin
quoted
Sent: 16 September 2016 09:58
Implementing busy wait loops with cpu_relax() in callers poses
some difficulties for powerpc.
First, we want to put our SMT thread into a low priority mode for the
duration of the loop, but then return to normal priority after exiting
the loop. Dependong on the CPU design, 'HMT_low() ; HMT_medium();' as
cpu_relax() does may have HMT_medium take effect before HMT_low made
any (or much) difference.
Second, it can be beneficial for some implementations to spin on the
exit condition with a statically predicted-not-taken branch (i.e.,
always predict the loop will exit).
This is a quick RFC with a couple of users converted to see what
people think. I don't use a C branch with hints, because we don't want
the compiler moving the loop body out of line, which makes it a bit
messy unfortunately. If there's a better way to do it, I'm all ears.
I think it will still all go wrong if the conditional isn't trivial.
In particular if the condition contains || or && it is likely to
have a branch - which could invert the loop.
I don't know that it will.
Yes, if we have exit condition that requires more branches in order to
be computed then we lose our nice property of never taking a branch
miss on loop exit. But we still avoid *this* branch miss, and still
prevent multiple iterations of the wait loop being speculatively
executed concurrently when there's no work to be done.
And C doesn't know about the loop, so it can't do any transformation
except to compute the final condition.
Or have I missed something?
Thanks,
Nick
From: David Laight <hidden> Date: 2016-09-16 11:59:57
From: Nicholas Piggin
Sent: 16 September 2016 12:52
On Fri, 16 Sep 2016 11:30:58 +0000
David Laight [off-list ref] wrote:
=20
quoted
From: Nicholas Piggin
quoted
Sent: 16 September 2016 09:58
Implementing busy wait loops with cpu_relax() in callers poses
some difficulties for powerpc.
First, we want to put our SMT thread into a low priority mode for the
duration of the loop, but then return to normal priority after exitin=
g
quoted
quoted
the loop. Dependong on the CPU design, 'HMT_low() ; HMT_medium();' a=
s
quoted
quoted
cpu_relax() does may have HMT_medium take effect before HMT_low made
any (or much) difference.
Second, it can be beneficial for some implementations to spin on the
exit condition with a statically predicted-not-taken branch (i.e.,
always predict the loop will exit).
This is a quick RFC with a couple of users converted to see what
people think. I don't use a C branch with hints, because we don't wan=
t
quoted
quoted
the compiler moving the loop body out of line, which makes it a bit
messy unfortunately. If there's a better way to do it, I'm all ears.
I think it will still all go wrong if the conditional isn't trivial.
In particular if the condition contains || or && it is likely to
have a branch - which could invert the loop.
=20
I don't know that it will.
=20
Yes, if we have exit condition that requires more branches in order to
be computed then we lose our nice property of never taking a branch
miss on loop exit. But we still avoid *this* branch miss, and still
prevent multiple iterations of the wait loop being speculatively
executed concurrently when there's no work to be done.
=20
And C doesn't know about the loop, so it can't do any transformation
except to compute the final condition.
=20
Or have I missed something?
Try putting the code inside a conditional or at the bottom of a loop.
gcc can replicate code to remove a branch.
So:
for (;;) {
a;
if (b)
c;
d;
}
can become:
x1:
a;
if (b) to x2;
d;
goto x1;
x2:
c;
d;
goto x1;
Which won't work.
David
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-09-16 12:06:44
On Fri, 16 Sep 2016 11:57:37 +0000
David Laight [off-list ref] wrote:
From: Nicholas Piggin
quoted
Sent: 16 September 2016 12:52
On Fri, 16 Sep 2016 11:30:58 +0000
David Laight [off-list ref] wrote:
quoted
From: Nicholas Piggin
quoted
Sent: 16 September 2016 09:58
Implementing busy wait loops with cpu_relax() in callers poses
some difficulties for powerpc.
First, we want to put our SMT thread into a low priority mode for the
duration of the loop, but then return to normal priority after exiting
the loop. Dependong on the CPU design, 'HMT_low() ; HMT_medium();' as
cpu_relax() does may have HMT_medium take effect before HMT_low made
any (or much) difference.
Second, it can be beneficial for some implementations to spin on the
exit condition with a statically predicted-not-taken branch (i.e.,
always predict the loop will exit).
This is a quick RFC with a couple of users converted to see what
people think. I don't use a C branch with hints, because we don't want
the compiler moving the loop body out of line, which makes it a bit
messy unfortunately. If there's a better way to do it, I'm all ears.
I think it will still all go wrong if the conditional isn't trivial.
In particular if the condition contains || or && it is likely to
have a branch - which could invert the loop.
I don't know that it will.
Yes, if we have exit condition that requires more branches in order to
be computed then we lose our nice property of never taking a branch
miss on loop exit. But we still avoid *this* branch miss, and still
prevent multiple iterations of the wait loop being speculatively
executed concurrently when there's no work to be done.
And C doesn't know about the loop, so it can't do any transformation
except to compute the final condition.
Or have I missed something?
Try putting the code inside a conditional or at the bottom of a loop.
gcc can replicate code to remove a branch.
So:
for (;;) {
a;
if (b)
c;
d;
}
That's not what this patch does though. The loop is purely asm. gcc has
no idea about it. Only thing gcc knows is to evaluate the condition and
put it in a register.
Thanks,
Nick
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-09-16 12:59:52
On Fri, 16 Sep 2016 22:06:35 +1000
Nicholas Piggin [off-list ref] wrote:
On Fri, 16 Sep 2016 11:57:37 +0000
David Laight [off-list ref] wrote:
quoted
From: Nicholas Piggin
quoted
Sent: 16 September 2016 12:52
On Fri, 16 Sep 2016 11:30:58 +0000
David Laight [off-list ref] wrote:
quoted
From: Nicholas Piggin
quoted
Sent: 16 September 2016 09:58
Implementing busy wait loops with cpu_relax() in callers poses
some difficulties for powerpc.
First, we want to put our SMT thread into a low priority mode for the
duration of the loop, but then return to normal priority after exiting
the loop. Dependong on the CPU design, 'HMT_low() ; HMT_medium();' as
cpu_relax() does may have HMT_medium take effect before HMT_low made
any (or much) difference.
Second, it can be beneficial for some implementations to spin on the
exit condition with a statically predicted-not-taken branch (i.e.,
always predict the loop will exit).
This is a quick RFC with a couple of users converted to see what
people think. I don't use a C branch with hints, because we don't want
the compiler moving the loop body out of line, which makes it a bit
messy unfortunately. If there's a better way to do it, I'm all ears.
I think it will still all go wrong if the conditional isn't trivial.
In particular if the condition contains || or && it is likely to
have a branch - which could invert the loop.
I don't know that it will.
Yes, if we have exit condition that requires more branches in order to
be computed then we lose our nice property of never taking a branch
miss on loop exit. But we still avoid *this* branch miss, and still
prevent multiple iterations of the wait loop being speculatively
executed concurrently when there's no work to be done.
And C doesn't know about the loop, so it can't do any transformation
except to compute the final condition.
Or have I missed something?
Try putting the code inside a conditional or at the bottom of a loop.
gcc can replicate code to remove a branch.
So:
for (;;) {
a;
if (b)
c;
d;
}
That's not what this patch does though. The loop is purely asm. gcc has
no idea about it. Only thing gcc knows is to evaluate the condition and
put it in a register.
Oh you're right course -- can't branch to random location. Sorry, I didn't
know what you meant at first. It does need to use asm goto I guess.
Thanks,
Nick
Implementing busy wait loops with cpu_relax() in callers poses
some difficulties for powerpc.
First, we want to put our SMT thread into a low priority mode for the
duration of the loop, but then return to normal priority after exiting
the loop. Dependong on the CPU design, 'HMT_low() ; HMT_medium();' as
cpu_relax() does may have HMT_medium take effect before HMT_low made
any (or much) difference.
Second, it can be beneficial for some implementations to spin on the
exit condition with a statically predicted-not-taken branch (i.e.,
always predict the loop will exit).
IIUC, what you are proposing is that cpu_relax() be split such
that on entry we do HMT_low() and on exit do HMT_medium(). I think
that makes a lot of sense, in that it allows the required transition
time from low to medium
quoted hunk
This is a quick RFC with a couple of users converted to see what
people think. I don't use a C branch with hints, because we don't want
the compiler moving the loop body out of line, which makes it a bit
messy unfortunately. If there's a better way to do it, I'm all ears.
I would not propose to switch all callers immediately, just some
core synchronisation primitives.
---
arch/powerpc/include/asm/processor.h | 22 ++++++++++++++++++++++
include/asm-generic/barrier.h | 7 ++-----
include/linux/bit_spinlock.h | 5 ++---
include/linux/cgroup.h | 7 ++-----
include/linux/seqlock.h | 10 ++++------
5 files changed, 32 insertions(+), 19 deletions(-)
Implementing busy wait loops with cpu_relax() in callers poses
some difficulties for powerpc.
First, we want to put our SMT thread into a low priority mode for the
duration of the loop, but then return to normal priority after exiting
the loop. Dependong on the CPU design, 'HMT_low() ; HMT_medium();' as
cpu_relax() does may have HMT_medium take effect before HMT_low made
any (or much) difference.
Second, it can be beneficial for some implementations to spin on the
exit condition with a statically predicted-not-taken branch (i.e.,
always predict the loop will exit).
IIUC, what you are proposing is that cpu_relax() be split such
that on entry we do HMT_low() and on exit do HMT_medium(). I think
that makes a lot of sense, in that it allows the required transition
time from low to medium
Basically yes, although also allowing the loop exit branch to be
overridden by the arch code too. That can possibly benefit some
microarchitectures -- e.g., you want loop exit to not take a branch
miss if possible but it may be acceptable to branch miss for every
other iteration. I'm doing some testing of it now (previous patch
was garbage btw, don't try to use it!)
This is just spin_while(!cond) from an implementation perspective right?
Yes, the only reason I put it in was because such spin loops often
read a bit better the other way from normal loops (you are interested
in the exit condition rather than the loop-again condition).
Hmm. I see what you mean, but I don't know if we should trust open
coded callers to get this right. It could cause weird problems and
isn't easily caught. If we can get something that breaks build,
perhaps. OTOH, I prefer all the logic including SMT priority to be
in the spin loop primitive directly because it's pretty subtle and
might need to be runtime patched.
We could *also* have a cpu_relax_begin/cpu_relax_end pair, although
I'd like to first see callers that don't suit spin_ primitives and
see what should be done.
Thanks,
Nick
From: Christian Borntraeger <hidden> Date: 2016-09-20 11:19:38
On 09/16/2016 10:57 AM, Nicholas Piggin wrote:
Implementing busy wait loops with cpu_relax() in callers poses
some difficulties for powerpc.
First, we want to put our SMT thread into a low priority mode for the
duration of the loop, but then return to normal priority after exiting
the loop. Dependong on the CPU design, 'HMT_low() ; HMT_medium();' as
cpu_relax() does may have HMT_medium take effect before HMT_low made
any (or much) difference.
Second, it can be beneficial for some implementations to spin on the
exit condition with a statically predicted-not-taken branch (i.e.,
always predict the loop will exit).
This is a quick RFC with a couple of users converted to see what
people think. I don't use a C branch with hints, because we don't want
the compiler moving the loop body out of line, which makes it a bit
messy unfortunately. If there's a better way to do it, I'm all ears.
I would not propose to switch all callers immediately, just some
core synchronisation primitives.
Just a FYA,
On s390 we have a private version of cpu_relax that yields the cpu
time slice back to the hypervisor via a hypercall. As this turned out
to be problematic in some cases there is also now a cpu_relax_lowlatency.
Now, this seems still problematic as there are too many places still
using cpu_relax instead of cpu_relax_lowlatency. So my plan is to do
a change of that, make cpu_relax just be a barrier and add a new
cpu_relax_yield that gives up the time slice. (so that s390 cpu_relax
is just like any other cpu_relax)
As far as I can tell the only place where I want to change cpu_relax
to cpu_relax_lowlatency after that change is the stop machine run
code, so I hope to have no conflicts with your changes.
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-09-20 12:27:42
On Tue, 20 Sep 2016 13:19:30 +0200
Christian Borntraeger [off-list ref] wrote:
On 09/16/2016 10:57 AM, Nicholas Piggin wrote:
quoted
Implementing busy wait loops with cpu_relax() in callers poses
some difficulties for powerpc.
First, we want to put our SMT thread into a low priority mode for the
duration of the loop, but then return to normal priority after exiting
the loop. Dependong on the CPU design, 'HMT_low() ; HMT_medium();' as
cpu_relax() does may have HMT_medium take effect before HMT_low made
any (or much) difference.
Second, it can be beneficial for some implementations to spin on the
exit condition with a statically predicted-not-taken branch (i.e.,
always predict the loop will exit).
This is a quick RFC with a couple of users converted to see what
people think. I don't use a C branch with hints, because we don't want
the compiler moving the loop body out of line, which makes it a bit
messy unfortunately. If there's a better way to do it, I'm all ears.
I would not propose to switch all callers immediately, just some
core synchronisation primitives.
Just a FYA,
On s390 we have a private version of cpu_relax that yields the cpu
time slice back to the hypervisor via a hypercall.
The powerpc guest also wants to yield to hypervisor in some busywait
situations.
As this turned out
to be problematic in some cases there is also now a cpu_relax_lowlatency.
Now, this seems still problematic as there are too many places still
using cpu_relax instead of cpu_relax_lowlatency. So my plan is to do
a change of that, make cpu_relax just be a barrier and add a new
cpu_relax_yield that gives up the time slice. (so that s390 cpu_relax
is just like any other cpu_relax)
As far as I can tell the only place where I want to change cpu_relax
to cpu_relax_lowlatency after that change is the stop machine run
code, so I hope to have no conflicts with your changes.
I don't think there should be any conflicts, but it would be good to
make sure busy wait primitives can be usable by s390. So I can add
_yield variants that can do the right thing for s390.
I need to think more about virtualization, so I'm glad you commented.
Powerpc would like to be told when a busywait loop knows the CPU it is
waiting for. So perhaps also a _yield_to_cpu variant as well.
Something that will work with mutex_spin_on_owner and similar would be
nice too. As far as I can tell, powerpc may want to yield to hypervisor
when the owner's vcpu is scheduled off in that case too.
Thanks,
Nick
From: Christian Borntraeger <hidden> Date: 2016-09-20 12:35:57
On 09/20/2016 02:27 PM, Nicholas Piggin wrote:
On Tue, 20 Sep 2016 13:19:30 +0200
Christian Borntraeger [off-list ref] wrote:
quoted
On 09/16/2016 10:57 AM, Nicholas Piggin wrote:
quoted
Implementing busy wait loops with cpu_relax() in callers poses
some difficulties for powerpc.
First, we want to put our SMT thread into a low priority mode for the
duration of the loop, but then return to normal priority after exiting
the loop. Dependong on the CPU design, 'HMT_low() ; HMT_medium();' as
cpu_relax() does may have HMT_medium take effect before HMT_low made
any (or much) difference.
Second, it can be beneficial for some implementations to spin on the
exit condition with a statically predicted-not-taken branch (i.e.,
always predict the loop will exit).
This is a quick RFC with a couple of users converted to see what
people think. I don't use a C branch with hints, because we don't want
the compiler moving the loop body out of line, which makes it a bit
messy unfortunately. If there's a better way to do it, I'm all ears.
I would not propose to switch all callers immediately, just some
core synchronisation primitives.
Just a FYA,
On s390 we have a private version of cpu_relax that yields the cpu
time slice back to the hypervisor via a hypercall.
The powerpc guest also wants to yield to hypervisor in some busywait
situations.
quoted
As this turned out
to be problematic in some cases there is also now a cpu_relax_lowlatency.
Now, this seems still problematic as there are too many places still
using cpu_relax instead of cpu_relax_lowlatency. So my plan is to do
a change of that, make cpu_relax just be a barrier and add a new
cpu_relax_yield that gives up the time slice. (so that s390 cpu_relax
is just like any other cpu_relax)
As far as I can tell the only place where I want to change cpu_relax
to cpu_relax_lowlatency after that change is the stop machine run
code, so I hope to have no conflicts with your changes.
I don't think there should be any conflicts, but it would be good to
make sure busy wait primitives can be usable by s390. So I can add
_yield variants that can do the right thing for s390.
I was distracted by "more important work" (TM) but I will put you on
CC when ready.
I need to think more about virtualization, so I'm glad you commented.
Powerpc would like to be told when a busywait loop knows the CPU it is
waiting for. So perhaps also a _yield_to_cpu variant as well.
Yes, we also have 2 hypercalls: one that yields somehow and one that yields
to a specific CPU. The latter is strongly preferred.
Something that will work with mutex_spin_on_owner and similar would be
nice too. As far as I can tell, powerpc may want to yield to hypervisor
when the owner's vcpu is scheduled off in that case too.
From: Nicholas Piggin <npiggin@gmail.com> Date: 2016-09-20 12:46:49
On Tue, 20 Sep 2016 14:35:45 +0200
Christian Borntraeger [off-list ref] wrote:
On 09/20/2016 02:27 PM, Nicholas Piggin wrote:
quoted
On Tue, 20 Sep 2016 13:19:30 +0200
Christian Borntraeger [off-list ref] wrote:
quoted
On 09/16/2016 10:57 AM, Nicholas Piggin wrote:
quoted
Implementing busy wait loops with cpu_relax() in callers poses
some difficulties for powerpc.
First, we want to put our SMT thread into a low priority mode for the
duration of the loop, but then return to normal priority after exiting
the loop. Dependong on the CPU design, 'HMT_low() ; HMT_medium();' as
cpu_relax() does may have HMT_medium take effect before HMT_low made
any (or much) difference.
Second, it can be beneficial for some implementations to spin on the
exit condition with a statically predicted-not-taken branch (i.e.,
always predict the loop will exit).
This is a quick RFC with a couple of users converted to see what
people think. I don't use a C branch with hints, because we don't want
the compiler moving the loop body out of line, which makes it a bit
messy unfortunately. If there's a better way to do it, I'm all ears.
I would not propose to switch all callers immediately, just some
core synchronisation primitives.
Just a FYA,
On s390 we have a private version of cpu_relax that yields the cpu
time slice back to the hypervisor via a hypercall.
The powerpc guest also wants to yield to hypervisor in some busywait
situations.
quoted
As this turned out
to be problematic in some cases there is also now a cpu_relax_lowlatency.
Now, this seems still problematic as there are too many places still
using cpu_relax instead of cpu_relax_lowlatency. So my plan is to do
a change of that, make cpu_relax just be a barrier and add a new
cpu_relax_yield that gives up the time slice. (so that s390 cpu_relax
is just like any other cpu_relax)
As far as I can tell the only place where I want to change cpu_relax
to cpu_relax_lowlatency after that change is the stop machine run
code, so I hope to have no conflicts with your changes.
I don't think there should be any conflicts, but it would be good to
make sure busy wait primitives can be usable by s390. So I can add
_yield variants that can do the right thing for s390.
I was distracted by "more important work" (TM) but I will put you on
CC when ready.
quoted
I need to think more about virtualization, so I'm glad you commented.
Powerpc would like to be told when a busywait loop knows the CPU it is
waiting for. So perhaps also a _yield_to_cpu variant as well.
Yes, we also have 2 hypercalls: one that yields somehow and one that yields
to a specific CPU. The latter is strongly preferred.
Okay, sounds good. I'll send out some updated patches soon too, so I'll
cc you on those. It would be good to come up with some basic guidelines
for when to use each variant too.
Thanks,
Nick