Skip to contentSkip to conversation

Xentrace: Avoid Crash on Debug Build Xen

PATCH In review Yosuke Iwamatsu 2 participants 3 messages Oct 5, 2007 – Oct 9, 2007

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 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

Xentrace: Remove assertions to avoid system crashes on debug build xen.

xen/common/trace.c +1 -5
diff -r 2d761ca771fb xen/common/trace.c
@@ -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;
         }
         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));
-
     if ( rec_size > bytes_to_wrap )
         insert_wrap_record(buf, rec_size);
 

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw

Hi there,

xen/common/trace.c Fri Oct 05 10:28:33 2007 +0900 renamed from xen/common/trace.c Thu Oct 04 17:58:16 2007 +0100 +1 -1
diff -r 2d761ca771fb xen/common/trace.c
@@ -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));
-

Quoted 4 lines

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

Quoted 7 lines

The attached patch removes assertions to avoid system crashes, and also fixes a mistake of an assignment statement.

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

Signature · 4 lines
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!
#2 lore raw

PATCH 1/1 Xentrace: Avoid Crash on Debug Build Xen

Quoted 20 lines · Mark Williamson
         }
         total_size += LOST_REC_SIZE;
     }
(quoted diff) +0 -4
-
-    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,
Quoted 3 lines

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

Quoted 3 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.

Quoted 14 lines

Cheers,
Mark

Quoted 7 lines

The attached patch removes assertions to avoid system crashes, and also fixes a mistake of an assignment statement.

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