* 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