WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] Re: [PATCH 2 of 4] xentrace: replace ASSERT with printk

To: George Dunlap <george.dunlap@xxxxxxxxxx>, Keir Fraser <keir@xxxxxxx>
Subject: Re: [Xen-devel] Re: [PATCH 2 of 4] xentrace: replace ASSERT with printk in __trace_var
From: Keir Fraser <keir.xen@xxxxxxxxx>
Date: Mon, 18 Jul 2011 12:12:24 +0100
Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Olaf Hering <olaf@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 18 Jul 2011 04:14:57 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=user-agent:date:subject:from:to:cc:message-id:thread-topic :thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; bh=Fn5gkFLhxFcoMTaD/ao78aAXEHc/Ol3IFWOdIdb8Mo4=; b=ojaqTrcoHNIMsttzyYZOrBZ2pf8Qti0HMhfe0zxM1bZuFWR6NOOfPR1gYw85yw387Y KDqLjZp0+zguSuUU8bX4q2WgU9CNA+lMR7DMAm6oa/Upa2yucx723GZPbUezzjhxFAtK orM8y8yuXIPzdNCHjoQ1tifQXzd107FTzU9bE=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1310985717.3401.11.camel@elijah>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcxFO5HlPulWFwQmdEK2choTwkFDjQ==
Thread-topic: [Xen-devel] Re: [PATCH 2 of 4] xentrace: replace ASSERT with printk in __trace_var
User-agent: Microsoft-Entourage/12.30.0.110427
On 18/07/2011 11:41, "George Dunlap" <george.dunlap@xxxxxxxxxx> wrote:

> On Sat, 2011-07-16 at 09:20 +0100, Keir Fraser wrote:
>> It's not like we have dodgy third-party drivers to worry about. Just fix the
>> code -- *which is in the tree!* -- that is making the bad calls! It was
>> correct to make this an assert/bug_on in the first place.
> 
> I think Olaf was objecting to a relatively minor fault like this causing
> a hypervisor crash (in debug builds only of course), and asked if it
> could be changed to a warning instead.
> 
> My personal preference would be to keep it an ASSERT, but I'm not really
> that fussed either way.  The main thing is to make sure that only debug
> builds may have significant negative effects, and I think this patch
> doesn't satisfy that (as if you add loglvl=all on a production system,
> you may get stuck sending significant output to the console).

It only has a negative effect if you have xentrace enabled. That's fine.

 -- Keir

>  -George
> 
>> 
>>  -- Keir
>> 
>> On 15/07/2011 17:31, "George Dunlap" <george.dunlap@xxxxxxxxxx> wrote:
>> 
>>> This seems likely to spam the console if there is a trace which violates
>>> this; and this may happen in production environments if the loglevel is
>>> increased.  I think putting in something to warn just once would be a
>>> better idea.
>>> 
>>>  -George
>>> 
>>> On Fri, 2011-07-15 at 16:38 +0100, Olaf Hering wrote:
>>>> # HG changeset patch
>>>> # User Olaf Hering <olaf@xxxxxxxxx>
>>>> # Date 1310741871 -7200
>>>> # Node ID e0ff4eea0432e0af3210e090a47414a0126e9904
>>>> # Parent  d0dcdddf5285eba0605a95dfda79b794803fa733
>>>> xentrace: replace ASSERT with printk in __trace_var
>>>> 
>>>> If trace_var gets called with large extra_data, do not crash the
>>>> hypervisor.
>>>> Instead print a warning and truncate the buffer.
>>>> 
>>>> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
>>>> 
>>>> diff -r d0dcdddf5285 -r e0ff4eea0432 xen/common/trace.c
>>>> --- a/xen/common/trace.c
>>>> +++ b/xen/common/trace.c
>>>> @@ -683,7 +683,10 @@ void __trace_var(u32 event, bool_t cycle
>>>>      if ( (extra % sizeof(u32)) != 0 )
>>>>          extra_word++;
>>>>      
>>>> -    ASSERT(extra_word <= TRACE_EXTRA_MAX);
>>>> +    if ( unlikely(extra_word > TRACE_EXTRA_MAX) )
>>>> +        printk(XENLOG_WARNING "xentrace: event %x extra_data %u too
>>>> large.\n",
>>>> +                event, extra);
>>>> +
>>>>      extra_word = min_t(int, extra_word, TRACE_EXTRA_MAX);
>>>>  
>>>>      /* Round size up to nearest word */
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>>> http://lists.xensource.com/xen-devel
>> 
>> 
> 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>