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

[Keir Fraser <keir.xen@xxxxxxxxx>]

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