Re: [Xen-devel] [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips

2017-11-13 Thread Julien Grall 

Hi Jan,

On 11/06/2017 11:09 AM, Jan Beulich wrote:

On 31.10.17 at 11:49,  wrote:

--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -44,7 +44,13 @@ static void check_lock(struct lock_debug *debug)
  if ( unlikely(debug->irq_safe != irq_safe) )
  {
  int seen = cmpxchg(>irq_safe, -1, irq_safe);
-BUG_ON(seen == !irq_safe);
+
+if ( seen == !irq_safe )
+{
+printk("CHECKLOCK FAILURE: prev irqsafe: %d, curr irqsafe %d\n",
+   seen, irq_safe);
+BUG();


This really should use XENLOG_ERR imo, so that the message won't
be lost if warnings are rate limited.


The patch was already merged. I guess a follow-up could be done for Xen 
4.10.


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/spinlock: Improve the output from check_lock() if it trips

2017-11-06 Thread Jan Beulich 
>>> On 31.10.17 at 11:49,  wrote:
> --- a/xen/common/spinlock.c
> +++ b/xen/common/spinlock.c
> @@ -44,7 +44,13 @@ static void check_lock(struct lock_debug *debug)
>  if ( unlikely(debug->irq_safe != irq_safe) )
>  {
>  int seen = cmpxchg(>irq_safe, -1, irq_safe);
> -BUG_ON(seen == !irq_safe);
> +
> +if ( seen == !irq_safe )
> +{
> +printk("CHECKLOCK FAILURE: prev irqsafe: %d, curr irqsafe %d\n",
> +   seen, irq_safe);
> +BUG();

This really should use XENLOG_ERR imo, so that the message won't
be lost if warnings are rate limited.

Jan


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

Re: [Xen-devel] [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips

2017-11-02 Thread Julien Grall 

Hi Andrew,

On 31/10/17 10:49, Andrew Cooper wrote:

If check_lock() triggers, a crash will occur.  Instead of simply identifying
"the irq context was different", indicate the expected and current irq
context.

Signed-off-by: Andrew Cooper 


Release-acked-by: Julien Grall 

Cheers,


---
CC: George Dunlap 
CC: Jan Beulich 
CC: Konrad Rzeszutek Wilk 
CC: Stefano Stabellini 
CC: Tim Deegan 
CC: Wei Liu 
CC: Julien Grall 

check_lock() only exists in debug builds, which makes this a low risk change
for 4.10.
---
  xen/common/spinlock.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 44b07b7..8f2ba08 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -44,7 +44,13 @@ static void check_lock(struct lock_debug *debug)
  if ( unlikely(debug->irq_safe != irq_safe) )
  {
  int seen = cmpxchg(>irq_safe, -1, irq_safe);
-BUG_ON(seen == !irq_safe);
+
+if ( seen == !irq_safe )
+{
+printk("CHECKLOCK FAILURE: prev irqsafe: %d, curr irqsafe %d\n",
+   seen, irq_safe);
+BUG();
+}
  }
  }
  



--
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/spinlock: Improve the output from check_lock() if it trips

2017-10-31 Thread George Dunlap 
On 10/31/2017 10:49 AM, Andrew Cooper wrote:
> If check_lock() triggers, a crash will occur.  Instead of simply identifying
> "the irq context was different", indicate the expected and current irq
> context.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: George Dunlap 

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

Re: [Xen-devel] [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips

2017-10-31 Thread Wei Liu 
On Tue, Oct 31, 2017 at 10:49:10AM +, Andrew Cooper wrote:
> If check_lock() triggers, a crash will occur.  Instead of simply identifying
> "the irq context was different", indicate the expected and current irq
> context.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Wei Liu 

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

[Xen-devel] [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips

2017-10-31 Thread Andrew Cooper 
If check_lock() triggers, a crash will occur.  Instead of simply identifying
"the irq context was different", indicate the expected and current irq
context.

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 

check_lock() only exists in debug builds, which makes this a low risk change
for 4.10.
---
 xen/common/spinlock.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 44b07b7..8f2ba08 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -44,7 +44,13 @@ static void check_lock(struct lock_debug *debug)
 if ( unlikely(debug->irq_safe != irq_safe) )
 {
 int seen = cmpxchg(>irq_safe, -1, irq_safe);
-BUG_ON(seen == !irq_safe);
+
+if ( seen == !irq_safe )
+{
+printk("CHECKLOCK FAILURE: prev irqsafe: %d, curr irqsafe %d\n",
+   seen, irq_safe);
+BUG();
+}
 }
 }
 
-- 
2.1.4


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