Re: [Xen-devel] [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls

[Julien Grall]

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 
---
CC: George Dunlap 
CC: Jan Beulich 
CC: Konrad Rzeszutek Wilk 
CC: Stefano Stabellini 
CC: Tim Deegan 
CC: Wei Liu 
CC: Julien Grall 

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 

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

Re: [Xen-devel] [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls

[Wei Liu]

On Tue, Oct 31, 2017 at 05:18:52PM +, 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 

Reviewed-by: Wei Liu 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls

[George Dunlap]

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 

Reviewed-by: George Dunlap 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH for-4.10] common/multicall: Increase debugability for bad hypercalls

[Andrew Cooper]

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 
---
CC: George Dunlap 
CC: Jan Beulich 
CC: Konrad Rzeszutek Wilk 
CC: Stefano Stabellini 
CC: Tim Deegan 
CC: Wei Liu 
CC: Julien Grall 

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