* [PATCH] powerpc/xmon: Fix invocation inside lock region
@ 2018-11-08 17:12 Breno Leitao
2018-11-09 16:22 ` Thiago Jung Bauermann
2018-12-07 13:07 ` Michael Ellerman
0 siblings, 2 replies; 3+ messages in thread
From: Breno Leitao @ 2018-11-08 17:12 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Breno Leitao
Currently xmon needs to get devtree_lock (through rtas_token()) during its
invocation (at crash time). If there is a crash while devtree_lock is being
held, then xmon tries to get the lock but spins forever and never get into
the interactive debugger, as in the following case:
int *ptr = NULL;
raw_spin_lock_irqsave(&devtree_lock, flags);
*ptr = 0xdeadbeef;
This patch avoids calling rtas_token(), thus trying to get the same lock,
at crash time. This new mechanism proposes getting the token at
initialization time (xmon_init()) and just consuming it at crash time.
This would allow xmon to be possible invoked independent of devtree_lock
being held or not.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
arch/powerpc/xmon/xmon.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 36b8dc47a3c3..b566203d09c5 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -75,6 +75,9 @@ static int xmon_gate;
#define xmon_owner 0
#endif /* CONFIG_SMP */
+#ifdef CONFIG_PPC_PSERIES
+static int set_indicator_token = RTAS_UNKNOWN_SERVICE;
+#endif
static unsigned long in_xmon __read_mostly = 0;
static int xmon_on = IS_ENABLED(CONFIG_XMON_DEFAULT);
@@ -358,7 +361,6 @@ static inline void disable_surveillance(void)
#ifdef CONFIG_PPC_PSERIES
/* Since this can't be a module, args should end up below 4GB. */
static struct rtas_args args;
- int token;
/*
* At this point we have got all the cpus we can into
@@ -367,11 +369,11 @@ static inline void disable_surveillance(void)
* If we did try to take rtas.lock there would be a
* real possibility of deadlock.
*/
- token = rtas_token("set-indicator");
- if (token == RTAS_UNKNOWN_SERVICE)
+ if (set_indicator_token == RTAS_UNKNOWN_SERVICE)
return;
- rtas_call_unlocked(&args, token, 3, 1, NULL, SURVEILLANCE_TOKEN, 0, 0);
+ rtas_call_unlocked(&args, set_indicator_token, 3, 1, NULL,
+ SURVEILLANCE_TOKEN, 0, 0);
#endif /* CONFIG_PPC_PSERIES */
}
@@ -3688,6 +3690,14 @@ static void xmon_init(int enable)
__debugger_iabr_match = xmon_iabr_match;
__debugger_break_match = xmon_break_match;
__debugger_fault_handler = xmon_fault_handler;
+
+#ifdef CONFIG_PPC_PSERIES
+ /*
+ * Get the token here to avoid trying to get a lock
+ * during the crash, causing a deadlock.
+ */
+ set_indicator_token = rtas_token("set-indicator");
+#endif
} else {
__debugger = NULL;
__debugger_ipi = NULL;
--
2.19.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] powerpc/xmon: Fix invocation inside lock region
2018-11-08 17:12 [PATCH] powerpc/xmon: Fix invocation inside lock region Breno Leitao
@ 2018-11-09 16:22 ` Thiago Jung Bauermann
2018-12-07 13:07 ` Michael Ellerman
1 sibling, 0 replies; 3+ messages in thread
From: Thiago Jung Bauermann @ 2018-11-09 16:22 UTC (permalink / raw)
To: Breno Leitao; +Cc: linuxppc-dev
Breno Leitao <leitao@debian.org> writes:
> Currently xmon needs to get devtree_lock (through rtas_token()) during its
> invocation (at crash time). If there is a crash while devtree_lock is being
> held, then xmon tries to get the lock but spins forever and never get into
> the interactive debugger, as in the following case:
>
> int *ptr = NULL;
> raw_spin_lock_irqsave(&devtree_lock, flags);
> *ptr = 0xdeadbeef;
>
> This patch avoids calling rtas_token(), thus trying to get the same lock,
> at crash time. This new mechanism proposes getting the token at
> initialization time (xmon_init()) and just consuming it at crash time.
>
> This would allow xmon to be possible invoked independent of devtree_lock
> being held or not.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
I'm not familiar with xmon code but I was curious, and for what it's
worth:
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> arch/powerpc/xmon/xmon.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 36b8dc47a3c3..b566203d09c5 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -75,6 +75,9 @@ static int xmon_gate;
> #define xmon_owner 0
> #endif /* CONFIG_SMP */
>
> +#ifdef CONFIG_PPC_PSERIES
> +static int set_indicator_token = RTAS_UNKNOWN_SERVICE;
> +#endif
> static unsigned long in_xmon __read_mostly = 0;
> static int xmon_on = IS_ENABLED(CONFIG_XMON_DEFAULT);
>
> @@ -358,7 +361,6 @@ static inline void disable_surveillance(void)
> #ifdef CONFIG_PPC_PSERIES
> /* Since this can't be a module, args should end up below 4GB. */
> static struct rtas_args args;
> - int token;
>
> /*
> * At this point we have got all the cpus we can into
> @@ -367,11 +369,11 @@ static inline void disable_surveillance(void)
> * If we did try to take rtas.lock there would be a
> * real possibility of deadlock.
> */
> - token = rtas_token("set-indicator");
> - if (token == RTAS_UNKNOWN_SERVICE)
> + if (set_indicator_token == RTAS_UNKNOWN_SERVICE)
> return;
>
> - rtas_call_unlocked(&args, token, 3, 1, NULL, SURVEILLANCE_TOKEN, 0, 0);
> + rtas_call_unlocked(&args, set_indicator_token, 3, 1, NULL,
> + SURVEILLANCE_TOKEN, 0, 0);
>
> #endif /* CONFIG_PPC_PSERIES */
> }
> @@ -3688,6 +3690,14 @@ static void xmon_init(int enable)
> __debugger_iabr_match = xmon_iabr_match;
> __debugger_break_match = xmon_break_match;
> __debugger_fault_handler = xmon_fault_handler;
> +
> +#ifdef CONFIG_PPC_PSERIES
> + /*
> + * Get the token here to avoid trying to get a lock
> + * during the crash, causing a deadlock.
> + */
> + set_indicator_token = rtas_token("set-indicator");
> +#endif
> } else {
> __debugger = NULL;
> __debugger_ipi = NULL;
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: powerpc/xmon: Fix invocation inside lock region
2018-11-08 17:12 [PATCH] powerpc/xmon: Fix invocation inside lock region Breno Leitao
2018-11-09 16:22 ` Thiago Jung Bauermann
@ 2018-12-07 13:07 ` Michael Ellerman
1 sibling, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2018-12-07 13:07 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: Breno Leitao
On Thu, 2018-11-08 at 17:12:42 UTC, Breno Leitao wrote:
> Currently xmon needs to get devtree_lock (through rtas_token()) during its
> invocation (at crash time). If there is a crash while devtree_lock is being
> held, then xmon tries to get the lock but spins forever and never get into
> the interactive debugger, as in the following case:
>
> int *ptr = NULL;
> raw_spin_lock_irqsave(&devtree_lock, flags);
> *ptr = 0xdeadbeef;
>
> This patch avoids calling rtas_token(), thus trying to get the same lock,
> at crash time. This new mechanism proposes getting the token at
> initialization time (xmon_init()) and just consuming it at crash time.
>
> This would allow xmon to be possible invoked independent of devtree_lock
> being held or not.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/8d4a862276a9c30a269d368d324fb5
cheers
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-12-07 13:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-08 17:12 [PATCH] powerpc/xmon: Fix invocation inside lock region Breno Leitao
2018-11-09 16:22 ` Thiago Jung Bauermann
2018-12-07 13:07 ` 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).