Xen devel - Oct 2007 - [PATCH] Xentrace: Avoid Crash on Debug Build Xen

Hi there,

Current xen-unstable (debug build) crashes when using xentrace command,
due to assertion failures. It seems assertion failures occur whenever
there exist lost records, and I don''t see much reason to do this.
(Could anyone explain about these ASSERT lines ?)
The attached patch removes assertions to avoid system crashes,
and also fixes a mistake of an assignment statement.

Regards,
-------------------
Yosuke Iwamatsu
        NEC Corporation




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

Hi there,

diff -r 2d761ca771fb xen/common/trace.c
--- a/xen/common/trace.c        Thu Oct 04 17:58:16 2007 +0100
+++ b/xen/common/trace.c        Fri Oct 05 10:28:33 2007 +0900
@@ -435,12 +435,10 @@ void __trace_var(u32 event, int cycles, 
         {
             bytes_to_wrap -= LOST_REC_SIZE;
             if ( bytes_to_wrap == 0 )
-                bytes_to_wrap == data_size;
+                bytes_to_wrap = data_size;

Good catch!

         }
         total_size += LOST_REC_SIZE;
     }
-
-    ASSERT(bytes_to_wrap == calc_bytes_to_wrap(buf));
 
     if ( rec_size > bytes_to_wrap )
     {
@@ -484,8 +482,6 @@ void __trace_var(u32 event, int cycles, 
         insert_lost_records(buf);
     }
 
-    ASSERT(bytes_to_wrap == calc_bytes_to_wrap(buf));
-
> Current xen-unstable (debug build) crashes when using xentrace command,
> due to assertion failures. It seems assertion failures occur whenever
> there exist lost records, and I don''t see much reason to do this.
> (Could anyone explain about these ASSERT lines ?)
These asserts appear to have originally been checking that bytes_to_wrap is 
being calculated correctly...  It looks to me that the first assert 
isoutdated though, because it clearly will fail if a record is lost.

Perhaps the second one ought to succed though, since the value of buf->prod 
will have been appropriately updated by insert_lost_records()?

Cheers,
Mark
> The attached patch removes assertions to avoid system crashes,
> and also fixes a mistake of an assignment statement.
>
> Regards,
> -------------------
> Yosuke Iwamatsu
>         NEC Corporation


-- 
Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!

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

Mark Williamson wrote:
>          }
>          total_size += LOST_REC_SIZE;
>      }
> -
> -    ASSERT(bytes_to_wrap == calc_bytes_to_wrap(buf));
>  
>      if ( rec_size > bytes_to_wrap )
>      {
> @@ -484,8 +482,6 @@ void __trace_var(u32 event, int cycles, 
>          insert_lost_records(buf);
>      }
>  
> -    ASSERT(bytes_to_wrap == calc_bytes_to_wrap(buf));
> -
> 
>> Current xen-unstable (debug build) crashes when using xentrace command,
>> due to assertion failures. It seems assertion failures occur whenever
>> there exist lost records, and I don''t see much reason to do
this.
>> (Could anyone explain about these ASSERT lines ?)
> 
> These asserts appear to have originally been checking that bytes_to_wrap is
> being calculated correctly...  It looks to me that the first assert 
> isoutdated though, because it clearly will fail if a record is lost.
> 
> Perhaps the second one ought to succed though, since the value of
buf->prod
> will have been appropriately updated by insert_lost_records()?
Indeed the second assert seems to always succeed.
> 
> Cheers,
> Mark
> 
>> The attached patch removes assertions to avoid system crashes,
>> and also fixes a mistake of an assignment statement.
>>
>> Regards,
>> -------------------
>> Yosuke Iwamatsu
>>         NEC Corporation
> 
> 
> 


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