* [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls
@ 2017-10-31 17:18 Andrew Cooper
2017-10-31 17:25 ` George Dunlap
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Andrew Cooper @ 2017-10-31 17:18 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Tim Deegan, Julien Grall,
Jan Beulich
While investigating an issue (in a new codepath I'd introduced, as it turns
out), leaving interrupts disabled manifested as a subsequent op in the
multicall failing a check_lock() test.
The codepath would have hit the ASSERT_NOT_IN_ATOMIC on the return-to-guest
path, had it not hit the check_lock() first.
Call ASSERT_NOT_IN_ATOMIC() after each operation in the multicall, to make
failures more obvious.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Julien Grall <julien.grall@arm.com>
As with the related check_lock() patch, this only affects debug builds, so is
a very low risk change for 4.10
---
xen/common/multicall.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/xen/common/multicall.c b/xen/common/multicall.c
index c7af4e0..d98e59d 100644
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -66,6 +66,13 @@ do_multicall(
disp = arch_do_multicall_call(mcs);
+ /*
+ * In the unlikley event that a hypercall has left interrupts,
+ * spinlocks, or other things in a bad way, continuting the multicall
+ * will typically lead to far more subtle issues to debug.
+ */
+ ASSERT_NOT_IN_ATOMIC();
+
#ifndef NDEBUG
{
/*
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls
2017-10-31 17:18 [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls Andrew Cooper
@ 2017-10-31 17:25 ` George Dunlap
2017-10-31 17:27 ` Wei Liu
2017-11-02 14:03 ` Julien Grall
2 siblings, 0 replies; 4+ messages in thread
From: George Dunlap @ 2017-10-31 17:25 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Tim Deegan, Julien Grall, Jan Beulich
On 10/31/2017 05:18 PM, Andrew Cooper wrote:
> While investigating an issue (in a new codepath I'd introduced, as it turns
> out), leaving interrupts disabled manifested as a subsequent op in the
> multicall failing a check_lock() test.
>
> The codepath would have hit the ASSERT_NOT_IN_ATOMIC on the return-to-guest
> path, had it not hit the check_lock() first.
>
> Call ASSERT_NOT_IN_ATOMIC() after each operation in the multicall, to make
> failures more obvious.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls
2017-10-31 17:18 [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls Andrew Cooper
2017-10-31 17:25 ` George Dunlap
@ 2017-10-31 17:27 ` Wei Liu
2017-11-02 14:03 ` Julien Grall
2 siblings, 0 replies; 4+ messages in thread
From: Wei Liu @ 2017-10-31 17:27 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Tim Deegan, Xen-devel, Julien Grall, Jan Beulich
On Tue, Oct 31, 2017 at 05:18:52PM +0000, Andrew Cooper wrote:
> While investigating an issue (in a new codepath I'd introduced, as it turns
> out), leaving interrupts disabled manifested as a subsequent op in the
> multicall failing a check_lock() test.
>
> The codepath would have hit the ASSERT_NOT_IN_ATOMIC on the return-to-guest
> path, had it not hit the check_lock() first.
>
> Call ASSERT_NOT_IN_ATOMIC() after each operation in the multicall, to make
> failures more obvious.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls
2017-10-31 17:18 [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls Andrew Cooper
2017-10-31 17:25 ` George Dunlap
2017-10-31 17:27 ` Wei Liu
@ 2017-11-02 14:03 ` Julien Grall
2 siblings, 0 replies; 4+ messages in thread
From: Julien Grall @ 2017-11-02 14:03 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Tim Deegan, Julien Grall, Jan Beulich
Hi Andrew,
On 31/10/17 17:18, Andrew Cooper wrote:
> While investigating an issue (in a new codepath I'd introduced, as it turns
> out), leaving interrupts disabled manifested as a subsequent op in the
> multicall failing a check_lock() test.
>
> The codepath would have hit the ASSERT_NOT_IN_ATOMIC on the return-to-guest
> path, had it not hit the check_lock() first.
>
> Call ASSERT_NOT_IN_ATOMIC() after each operation in the multicall, to make
> failures more obvious.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: George Dunlap <George.Dunlap@eu.citrix.com>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Tim Deegan <tim@xen.org>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Julien Grall <julien.grall@arm.com>
>
> As with the related check_lock() patch, this only affects debug builds, so is
> a very low risk change for 4.10
Release-acked-by: Julien Grall <julien.grall@linaro.org>
With a couple of typos below.
> ---
> xen/common/multicall.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/xen/common/multicall.c b/xen/common/multicall.c
> index c7af4e0..d98e59d 100644
> --- a/xen/common/multicall.c
> +++ b/xen/common/multicall.c
> @@ -66,6 +66,13 @@ do_multicall(
>
> disp = arch_do_multicall_call(mcs);
>
> + /*
> + * In the unlikley event that a hypercall has left interrupts,
s/unlikley/unlikely/
> + * spinlocks, or other things in a bad way, continuting the multicall
s/continuting/continuing/
> + * will typically lead to far more subtle issues to debug.
> + */
> + ASSERT_NOT_IN_ATOMIC();
> +
> #ifndef NDEBUG
> {
> /*
>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-02 14:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-31 17:18 [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls Andrew Cooper
2017-10-31 17:25 ` George Dunlap
2017-10-31 17:27 ` Wei Liu
2017-11-02 14:03 ` Julien Grall
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.