* [PATCH v2] powerpc/fadump: fix race between pstore write and fadump crash trigger
@ 2020-04-01 10:02 ` Sourabh Jain
0 siblings, 0 replies; 2+ messages in thread
From: Sourabh Jain @ 2020-04-01 10:02 UTC (permalink / raw)
To: mpe; +Cc: hbathini, mahesh, linux-kernel, linuxppc-dev, Sourabh Jain
When we enter into fadump crash path via system reset we fail to update
the pstore.
On the system reset path we first update the pstore then we go for fadump
crash. But the problem here is when all the CPUs try to get the pstore
lock to initiate the pstore write, only one CPUs will acquire the lock
and proceed with the pstore write. Since it in NMI context CPUs that fail
to get lock do not wait for their turn to write to the pstore and simply
proceed with the next operation which is fadump crash. One of the CPU who
proceeded with fadump crash path triggers the crash and does not wait for
the CPU who gets the pstore lock to complete the pstore update.
Timeline diagram to depicts the sequence of events that leads to an
unsuccessful pstore update when we hit fadump crash path via system reset.
1 2 3 ... n CPU Threads
| | | |
| | | |
Reached to -->|--->|---->| ----------->|
system reset | | | |
path | | | |
| | | |
Try to -->|--->|---->|------------>|
acquire the | | | |
pstore lock | | | |
| | | |
| | | |
Got the -->| +->| | |<-+
pstore lock | | | | | |--> Didn't get the
| --------------------------+ lock and moving
| | | | ahead on fadump
| | | | crash path
| | | |
Begins the -->| | | |
process to | | | |<-- Got the chance to
update the | | | | trigger the crash
pstore | -> | | ... <- |
| | | | | |
| | | | | |<-- Triggers the
| | | | | | crash
| | | | | | ^
| | | | | | |
Writing to -->| | | | | | |
pstore | | | | | | |
| | |
^ |__________________| |
| CPU Relax |
| |
+-----------------------------------------+
|
v
Race: crash triggered before pstore
update completes
To avoid the race between the CPU who proceeds with the pstore and the CPU
who triggers the crash, a delay of 500 milliseconds is added on fadump
crash path to allow pstore update to complete before we trigger the crash.
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
arch/powerpc/kernel/fadump.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
---
Changelog:
v1 -> v2
- crash delay has been increased to 500 milliseconds
- race happens in NMI context so updated the commit
message to convey the same.
---
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index ff0114aeba9b..471171c40e64 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -32,6 +32,16 @@
#include <asm/fadump-internal.h>
#include <asm/setup.h>
+
+/*
+ * The CPU who acquired the lock to trigger the fadump crash should
+ * wait for other CPUs to complete their tasks (for example updating
+ * pstore) before triggering the crash.
+ *
+ * The timeout is in milliseconds.
+ */
+#define CRASH_TIMEOUT 500
+
static struct fw_dump fw_dump;
static void __init fadump_reserve_crash_area(u64 base);
@@ -634,6 +644,14 @@ void crash_fadump(struct pt_regs *regs, const char *str)
fdh->online_mask = *cpu_online_mask;
+ /*
+ * If we reached here via system reset path then let's
+ * wait for other CPUs to complete their task like pstore
+ * update.
+ */
+ if (TRAP(regs) == 0x100)
+ mdelay(CRASH_TIMEOUT);
+
fw_dump.ops->fadump_trigger(fdh, str);
}
--
2.21.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH v2] powerpc/fadump: fix race between pstore write and fadump crash trigger
@ 2020-04-01 10:02 ` Sourabh Jain
0 siblings, 0 replies; 2+ messages in thread
From: Sourabh Jain @ 2020-04-01 10:02 UTC (permalink / raw)
To: mpe; +Cc: Sourabh Jain, mahesh, linux-kernel, hbathini, linuxppc-dev
When we enter into fadump crash path via system reset we fail to update
the pstore.
On the system reset path we first update the pstore then we go for fadump
crash. But the problem here is when all the CPUs try to get the pstore
lock to initiate the pstore write, only one CPUs will acquire the lock
and proceed with the pstore write. Since it in NMI context CPUs that fail
to get lock do not wait for their turn to write to the pstore and simply
proceed with the next operation which is fadump crash. One of the CPU who
proceeded with fadump crash path triggers the crash and does not wait for
the CPU who gets the pstore lock to complete the pstore update.
Timeline diagram to depicts the sequence of events that leads to an
unsuccessful pstore update when we hit fadump crash path via system reset.
1 2 3 ... n CPU Threads
| | | |
| | | |
Reached to -->|--->|---->| ----------->|
system reset | | | |
path | | | |
| | | |
Try to -->|--->|---->|------------>|
acquire the | | | |
pstore lock | | | |
| | | |
| | | |
Got the -->| +->| | |<-+
pstore lock | | | | | |--> Didn't get the
| --------------------------+ lock and moving
| | | | ahead on fadump
| | | | crash path
| | | |
Begins the -->| | | |
process to | | | |<-- Got the chance to
update the | | | | trigger the crash
pstore | -> | | ... <- |
| | | | | |
| | | | | |<-- Triggers the
| | | | | | crash
| | | | | | ^
| | | | | | |
Writing to -->| | | | | | |
pstore | | | | | | |
| | |
^ |__________________| |
| CPU Relax |
| |
+-----------------------------------------+
|
v
Race: crash triggered before pstore
update completes
To avoid the race between the CPU who proceeds with the pstore and the CPU
who triggers the crash, a delay of 500 milliseconds is added on fadump
crash path to allow pstore update to complete before we trigger the crash.
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
arch/powerpc/kernel/fadump.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
---
Changelog:
v1 -> v2
- crash delay has been increased to 500 milliseconds
- race happens in NMI context so updated the commit
message to convey the same.
---
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index ff0114aeba9b..471171c40e64 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -32,6 +32,16 @@
#include <asm/fadump-internal.h>
#include <asm/setup.h>
+
+/*
+ * The CPU who acquired the lock to trigger the fadump crash should
+ * wait for other CPUs to complete their tasks (for example updating
+ * pstore) before triggering the crash.
+ *
+ * The timeout is in milliseconds.
+ */
+#define CRASH_TIMEOUT 500
+
static struct fw_dump fw_dump;
static void __init fadump_reserve_crash_area(u64 base);
@@ -634,6 +644,14 @@ void crash_fadump(struct pt_regs *regs, const char *str)
fdh->online_mask = *cpu_online_mask;
+ /*
+ * If we reached here via system reset path then let's
+ * wait for other CPUs to complete their task like pstore
+ * update.
+ */
+ if (TRAP(regs) == 0x100)
+ mdelay(CRASH_TIMEOUT);
+
fw_dump.ops->fadump_trigger(fdh, str);
}
--
2.21.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-04-01 10:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-01 10:02 [PATCH v2] powerpc/fadump: fix race between pstore write and fadump crash trigger Sourabh Jain
2020-04-01 10:02 ` Sourabh Jain
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.