xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips
@ 2017-10-31 10:49 Andrew Cooper
  2017-10-31 10:52 ` Wei Liu
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Andrew Cooper @ 2017-10-31 10:49 UTC (permalink / raw)
  To: Xen-devel
  Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap,
	Andrew Cooper, Tim Deegan, Julien Grall, Jan Beulich

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 <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>

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(&debug->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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips
  2017-10-31 10:49 [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips Andrew Cooper
@ 2017-10-31 10:52 ` Wei Liu
  2017-10-31 10:53 ` George Dunlap
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Wei Liu @ 2017-10-31 10:52 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 10:49:10AM +0000, 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 <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] 6+ messages in thread

* Re: [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips
  2017-10-31 10:49 [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips Andrew Cooper
  2017-10-31 10:52 ` Wei Liu
@ 2017-10-31 10:53 ` George Dunlap
  2017-11-02 13:52 ` Julien Grall
  2017-11-06 11:09 ` Jan Beulich
  3 siblings, 0 replies; 6+ messages in thread
From: George Dunlap @ 2017-10-31 10:53 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 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 <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] 6+ messages in thread

* Re: [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips
  2017-10-31 10:49 [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips Andrew Cooper
  2017-10-31 10:52 ` Wei Liu
  2017-10-31 10:53 ` George Dunlap
@ 2017-11-02 13:52 ` Julien Grall
  2017-11-06 11:09 ` Jan Beulich
  3 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2017-11-02 13:52 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 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 <andrew.cooper3@citrix.com>

Release-acked-by: Julien Grall <julien.grall@linaro.org>

Cheers,

> ---
> 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>
> 
> 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(&debug->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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips
  2017-10-31 10:49 [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips Andrew Cooper
                   ` (2 preceding siblings ...)
  2017-11-02 13:52 ` Julien Grall
@ 2017-11-06 11:09 ` Jan Beulich
  2017-11-13 16:05   ` Julien Grall
  3 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2017-11-06 11:09 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Stefano Stabellini, Wei Liu, KonradRzeszutek Wilk, George Dunlap,
	Tim Deegan, Xen-devel, Julien Grall

>>> On 31.10.17 at 11:49, <andrew.cooper3@citrix.com> 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(&debug->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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips
  2017-11-06 11:09 ` Jan Beulich
@ 2017-11-13 16:05   ` Julien Grall
  0 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2017-11-13 16:05 UTC (permalink / raw)
  To: Jan Beulich, Andrew Cooper
  Cc: Stefano Stabellini, Wei Liu, KonradRzeszutek Wilk, George Dunlap,
	Tim Deegan, Xen-devel, Julien Grall

Hi Jan,

On 11/06/2017 11:09 AM, Jan Beulich wrote:
>>>> On 31.10.17 at 11:49, <andrew.cooper3@citrix.com> 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(&debug->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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-11-13 16:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-31 10:49 [PATCH for-4.10] common/spinlock: Improve the output from check_lock() if it trips Andrew Cooper
2017-10-31 10:52 ` Wei Liu
2017-10-31 10:53 ` George Dunlap
2017-11-02 13:52 ` Julien Grall
2017-11-06 11:09 ` Jan Beulich
2017-11-13 16:05   ` Julien Grall

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).