* [PATCH v2] powerpc/powernv: Fix race in updating core_idle_state
@ 2015-07-06 20:09 Shreyas B. Prabhu
2015-07-07 0:22 ` Michael Ellerman
2015-07-07 10:38 ` [v2] " Michael Ellerman
0 siblings, 2 replies; 7+ messages in thread
From: Shreyas B. Prabhu @ 2015-07-06 20:09 UTC (permalink / raw)
To: Paul Mackerras, mpe
Cc: Benjamin Herrenschmidt, linuxppc-dev, linux-kernel, Shreyas B. Prabhu
core_idle_state is maintained for each core. It uses 0-7 bits to track
whether a thread in the core has entered fastsleep or winkle. 8th bit is
used as a lock bit.
The lock bit is set in these 2 scenarios-
- The thread is first in subcore to wakeup from sleep/winkle.
- If its the last thread in the core about to enter sleep/winkle
While the lock bit is set, if any other thread in the core wakes up, it
loops until the lock bit is cleared before proceeding in the wakeup
path. This helps prevent race conditions w.r.t fastsleep workaround and
prevents threads from switching to process context before core/subcore
resources are restored.
But, in the path to sleep/winkle entry, we currently don't check for
lock-bit. This exposes us to following race when running with subcore
on-
First thread in the subcorea Another thread in the same
waking up core entering sleep/winkle
lwarx r15,0,r14
ori r15,r15,PNV_CORE_IDLE_LOCK_BIT
stwcx. r15,0,r14
[Code to restore subcore state]
lwarx r15,0,r14
[clear thread bit]
stwcx. r15,0,r14
andi. r15,r15,PNV_CORE_IDLE_THREAD_BITS
stw r15,0(r14)
Here, after the thread entering sleep clears its thread bit in
core_idle_state, the value is overwritten by the thread waking up.
In such cases when the core enters fastsleep, code mistakes an idle
thread as running. Because of this, the first thread waking up from
fastsleep which is supposed to resync timebase skips it. So we can
end up having a core with stale timebase value.
This patch fixes the above race by looping on the lock bit even while
entering the idle states.
Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
Fixes: 7b54e9f213f76 'powernv/powerpc: Add winkle support for offline
cpus'
---
This needs to go into stable 3.19+
Changes in v2:
-------------
- Based on suggestion from Micheal, core_idle_lock_held changed to a
function call so that the same block can be called in entry and exit path.
- More detail on how this bug manifests added to commit message.
arch/powerpc/kernel/idle_power7.S | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/idle_power7.S b/arch/powerpc/kernel/idle_power7.S
index ccde8f0..4d04315 100644
--- a/arch/powerpc/kernel/idle_power7.S
+++ b/arch/powerpc/kernel/idle_power7.S
@@ -52,6 +52,22 @@
.text
/*
+ * Used by threads when the lock bit of core_idle_state is set.
+ * Threads will spin in HMT_LOW until the lock bit is cleared.
+ * r14 - pointer to core_idle_state
+ * r15 - used to load contents of core_idle_state
+ */
+
+core_idle_lock_held:
+ HMT_LOW
+3: lwz r15,0(r14)
+ andi. r15,r15,PNV_CORE_IDLE_LOCK_BIT
+ bne 3b
+ HMT_MEDIUM
+ lwarx r15,0,r14
+ blr
+
+/*
* Pass requested state in r3:
* r3 - PNV_THREAD_NAP/SLEEP/WINKLE
*
@@ -150,6 +166,10 @@ power7_enter_nap_mode:
ld r14,PACA_CORE_IDLE_STATE_PTR(r13)
lwarx_loop1:
lwarx r15,0,r14
+
+ andi. r9,r15,PNV_CORE_IDLE_LOCK_BIT
+ bnel core_idle_lock_held
+
andc r15,r15,r7 /* Clear thread bit */
andi. r15,r15,PNV_CORE_IDLE_THREAD_BITS
@@ -294,7 +314,7 @@ lwarx_loop2:
* workaround undo code or resyncing timebase or restoring context
* In either case loop until the lock bit is cleared.
*/
- bne core_idle_lock_held
+ bnel core_idle_lock_held
cmpwi cr2,r15,0
lbz r4,PACA_SUBCORE_SIBLING_MASK(r13)
@@ -319,15 +339,6 @@ lwarx_loop2:
isync
b common_exit
-core_idle_lock_held:
- HMT_LOW
-core_idle_lock_loop:
- lwz r15,0(14)
- andi. r9,r15,PNV_CORE_IDLE_LOCK_BIT
- bne core_idle_lock_loop
- HMT_MEDIUM
- b lwarx_loop2
-
first_thread_in_subcore:
/* First thread in subcore to wakeup */
ori r15,r15,PNV_CORE_IDLE_LOCK_BIT
--
1.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] powerpc/powernv: Fix race in updating core_idle_state
2015-07-06 20:09 [PATCH v2] powerpc/powernv: Fix race in updating core_idle_state Shreyas B. Prabhu
@ 2015-07-07 0:22 ` Michael Ellerman
2015-07-07 1:17 ` Shreyas B Prabhu
2015-07-09 4:41 ` Daniel Axtens
2015-07-07 10:38 ` [v2] " Michael Ellerman
1 sibling, 2 replies; 7+ messages in thread
From: Michael Ellerman @ 2015-07-07 0:22 UTC (permalink / raw)
To: Shreyas B. Prabhu
Cc: Paul Mackerras, Benjamin Herrenschmidt, linuxppc-dev, linux-kernel
On Tue, 2015-07-07 at 01:39 +0530, Shreyas B. Prabhu wrote:
> core_idle_state is maintained for each core. It uses 0-7 bits to track
> whether a thread in the core has entered fastsleep or winkle. 8th bit is
> used as a lock bit.
...
> This patch fixes the above race by looping on the lock bit even while
> entering the idle states.
>
> Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
> Fixes: 7b54e9f213f76 'powernv/powerpc: Add winkle support for offline
> cpus'
The sha is wrong, it should be 77b54e9f213f.
Also please don't wrap the description.
I recommend creating an alias or script that does:
$ git log --pretty=fixes -n 1 $commit | xclip
I've fixed it up, no need to resend.
cheers
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] powerpc/powernv: Fix race in updating core_idle_state
2015-07-07 0:22 ` Michael Ellerman
@ 2015-07-07 1:17 ` Shreyas B Prabhu
2015-07-09 4:41 ` Daniel Axtens
1 sibling, 0 replies; 7+ messages in thread
From: Shreyas B Prabhu @ 2015-07-07 1:17 UTC (permalink / raw)
To: Michael Ellerman
Cc: Paul Mackerras, Benjamin Herrenschmidt, linuxppc-dev, linux-kernel
On 07/07/2015 05:52 AM, Michael Ellerman wrote:
> On Tue, 2015-07-07 at 01:39 +0530, Shreyas B. Prabhu wrote:
>> Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
>> Fixes: 7b54e9f213f76 'powernv/powerpc: Add winkle support for offline
>> cpus'
>
> The sha is wrong, it should be 77b54e9f213f.
>
Argh! Sorry
> Also please don't wrap the description.
>
> I recommend creating an alias or script that does:
>
> $ git log --pretty=fixes -n 1 $commit | xclip
>
Will use this from next time.
>
> I've fixed it up, no need to resend.
>
Thanks!
--Shreyas
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [v2] powerpc/powernv: Fix race in updating core_idle_state
2015-07-06 20:09 [PATCH v2] powerpc/powernv: Fix race in updating core_idle_state Shreyas B. Prabhu
2015-07-07 0:22 ` Michael Ellerman
@ 2015-07-07 10:38 ` Michael Ellerman
1 sibling, 0 replies; 7+ messages in thread
From: Michael Ellerman @ 2015-07-07 10:38 UTC (permalink / raw)
To: Shreyas B. Prabhu, Paul Mackerras
Cc: linuxppc-dev, linux-kernel, Shreyas B. Prabhu
On Mon, 2015-06-07 at 20:09:23 UTC, "Shreyas B. Prabhu" wrote:
> core_idle_state is maintained for each core. It uses 0-7 bits to track
> whether a thread in the core has entered fastsleep or winkle. 8th bit is
> used as a lock bit.
...
> Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
> Fixes: 7b54e9f213f76 'powernv/powerpc: Add winkle support for offline
> cpus'
Applied to powerpc fixes, thanks.
https://git.kernel.org/cgit/linux/kernel/git/powerpc/linux.git/commit/?h=fixes&id=b32aadc1a8ed84afbe924cd2ced31cd6a2e67074
cheers
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] powerpc/powernv: Fix race in updating core_idle_state
2015-07-07 0:22 ` Michael Ellerman
2015-07-07 1:17 ` Shreyas B Prabhu
@ 2015-07-09 4:41 ` Daniel Axtens
2015-07-09 6:03 ` Shreyas B Prabhu
2015-07-09 7:07 ` Michael Ellerman
1 sibling, 2 replies; 7+ messages in thread
From: Daniel Axtens @ 2015-07-09 4:41 UTC (permalink / raw)
To: Michael Ellerman
Cc: Shreyas B. Prabhu, Paul Mackerras, linuxppc-dev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 439 bytes --]
> I recommend creating an alias or script that does:
>
> $ git log --pretty=fixes -n 1 $commit | xclip
>
FWIW, having finally got around to doing this, I found I first needed
the following snippet in ~/.gitconfig from
https://www.kernel.org/doc/Documentation/SubmittingPatches
[core]
abbrev = 12
[pretty]
fixes = Fixes: %h (\"%s\")
Otherwise git doesn't know what the pretty format is.
--
Regards,
Daniel
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 860 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] powerpc/powernv: Fix race in updating core_idle_state
2015-07-09 4:41 ` Daniel Axtens
@ 2015-07-09 6:03 ` Shreyas B Prabhu
2015-07-09 7:07 ` Michael Ellerman
1 sibling, 0 replies; 7+ messages in thread
From: Shreyas B Prabhu @ 2015-07-09 6:03 UTC (permalink / raw)
To: Daniel Axtens
Cc: Michael Ellerman, Paul Mackerras, linuxppc-dev, linux-kernel
On 07/09/2015 10:11 AM, Daniel Axtens wrote:
>> I recommend creating an alias or script that does:
>>
>> $ git log --pretty=fixes -n 1 $commit | xclip
>>
>
> FWIW, having finally got around to doing this, I found I first needed
> the following snippet in ~/.gitconfig from
> https://www.kernel.org/doc/Documentation/SubmittingPatches
>
>
> [core]
> abbrev = 12
> [pretty]
> fixes = Fixes: %h (\"%s\")
>
> Otherwise git doesn't know what the pretty format is.
>
Right, thanks for the pointer!
Thanks,
Shreyas
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] powerpc/powernv: Fix race in updating core_idle_state
2015-07-09 4:41 ` Daniel Axtens
2015-07-09 6:03 ` Shreyas B Prabhu
@ 2015-07-09 7:07 ` Michael Ellerman
1 sibling, 0 replies; 7+ messages in thread
From: Michael Ellerman @ 2015-07-09 7:07 UTC (permalink / raw)
To: Daniel Axtens
Cc: Shreyas B. Prabhu, Paul Mackerras, linuxppc-dev, linux-kernel
On Thu, 2015-07-09 at 14:41 +1000, Daniel Axtens wrote:
> > I recommend creating an alias or script that does:
> >
> > $ git log --pretty=fixes -n 1 $commit | xclip
> >
>
> FWIW, having finally got around to doing this, I found I first needed
> the following snippet in ~/.gitconfig from
> https://www.kernel.org/doc/Documentation/SubmittingPatches
>
>
> [core]
> abbrev = 12
> [pretty]
> fixes = Fixes: %h (\"%s\")
>
> Otherwise git doesn't know what the pretty format is.
Oh right, yeah. Doesn't everyone have my dotfiles!?
cheers
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-07-09 7:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-06 20:09 [PATCH v2] powerpc/powernv: Fix race in updating core_idle_state Shreyas B. Prabhu
2015-07-07 0:22 ` Michael Ellerman
2015-07-07 1:17 ` Shreyas B Prabhu
2015-07-09 4:41 ` Daniel Axtens
2015-07-09 6:03 ` Shreyas B Prabhu
2015-07-09 7:07 ` Michael Ellerman
2015-07-07 10:38 ` [v2] " Michael Ellerman
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).