Mailing List Archive: [RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data.

[RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data.

Nov 29, 2006, 10:59 PM

Post #1 of 12 (392 views)

Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
---

xen/common/trace.c | 6 +++---
xen/include/public/trace.h | 2 +-
xen/include/xen/trace.h | 14 +++++++-------
3 files changed, 11 insertions(+), 11 deletions(-)

Index: xen-unstable.hg-mainline.xentrace/xen/common/trace.c
===================================================================
--- xen-unstable.hg-mainline.xentrace.orig/xen/common/trace.c
+++ xen-unstable.hg-mainline.xentrace/xen/common/trace.c
@@ -46,7 +46,7 @@ static int nr_recs;
static int t_buf_highwater;

/* Number of records lost due to per-CPU trace buffer being full. */
-static DEFINE_PER_CPU(unsigned long, lost_records);
+static DEFINE_PER_CPU(uint64_t, lost_records);

/* a flag recording whether initialization has been done */
/* or more properly, if the tbuf subsystem is enabled right now */
@@ -228,8 +228,8 @@ int tb_control(xen_sysctl_tbuf_op_t *tbc
* failure, otherwise 0. Failure occurs only if the trace buffers are not yet
* initialised.
*/
-void trace(u32 event, unsigned long d1, unsigned long d2,
- unsigned long d3, unsigned long d4, unsigned long d5)
+void trace(uint32_t event, uint64_t d1, uint64_t d2, uint64_t d3, uint64_t d4,
+ uint64_t d5)
{
struct t_buf *buf;
struct t_rec *rec;
Index: xen-unstable.hg-mainline.xentrace/xen/include/public/trace.h
===================================================================
--- xen-unstable.hg-mainline.xentrace.orig/xen/include/public/trace.h
+++ xen-unstable.hg-mainline.xentrace/xen/include/public/trace.h
@@ -76,7 +76,7 @@
struct t_rec {
uint64_t cycles; /* cycle counter timestamp */
uint32_t event; /* event ID */
- unsigned long data[5]; /* event data items */
+ uint64_t data[5]; /* event data items */
};

/*
Index: xen-unstable.hg-mainline.xentrace/xen/include/xen/trace.h
===================================================================
--- xen-unstable.hg-mainline.xentrace.orig/xen/include/xen/trace.h
+++ xen-unstable.hg-mainline.xentrace/xen/include/xen/trace.h
@@ -33,19 +33,19 @@ void init_trace_bufs(void);
/* used to retrieve the physical address of the trace buffers */
int tb_control(struct xen_sysctl_tbuf_op *tbc);

-void trace(u32 event, unsigned long d1, unsigned long d2,
- unsigned long d3, unsigned long d4, unsigned long d5);
+void trace(uint32_t event, uint64_t d1, uint64_t d2, uint64_t d3, uint64_t d4,
+ uint64_t d5);

/* Avoids troubling the caller with casting their arguments to a trace macro */
#define trace_do_casts(e,d1,d2,d3,d4,d5) \
do { \
if ( unlikely(tb_init_done) ) \
trace(e, \
- (unsigned long)d1, \
- (unsigned long)d2, \
- (unsigned long)d3, \
- (unsigned long)d4, \
- (unsigned long)d5); \
+ (uint64_t)d1, \
+ (uint64_t)d2, \
+ (uint64_t)d3, \
+ (uint64_t)d4, \
+ (uint64_t)d5); \
} while ( 0 )

/* Convenience macros for calling the trace function. */

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

Re: [RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data. [ In reply to ]

mark.williamson at cl

Nov 30, 2006, 6:02 AM

Post #2 of 12 (391 views)

Permalink

It's worth noting that this patch will (roughly) halve the number of trace
records. Nobody should probably be relying on trace buffer size anymore, so
this shouldn't be an issue.

No objections here.

Cheers,
Mark

On Thursday 30 November 2006 05:59, Tony Breeds wrote:
> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
> ---
>
> xen/common/trace.c | 6 +++---
> xen/include/public/trace.h | 2 +-
> xen/include/xen/trace.h | 14 +++++++-------
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> Index: xen-unstable.hg-mainline.xentrace/xen/common/trace.c
> ===================================================================
> --- xen-unstable.hg-mainline.xentrace.orig/xen/common/trace.c
> +++ xen-unstable.hg-mainline.xentrace/xen/common/trace.c
> @@ -46,7 +46,7 @@ static int nr_recs;
> static int t_buf_highwater;
>
> /* Number of records lost due to per-CPU trace buffer being full. */
> -static DEFINE_PER_CPU(unsigned long, lost_records);
> +static DEFINE_PER_CPU(uint64_t, lost_records);
>
> /* a flag recording whether initialization has been done */
> /* or more properly, if the tbuf subsystem is enabled right now */
> @@ -228,8 +228,8 @@ int tb_control(xen_sysctl_tbuf_op_t *tbc
> * failure, otherwise 0. Failure occurs only if the trace buffers are not
> yet * initialised.
> */
> -void trace(u32 event, unsigned long d1, unsigned long d2,
> - unsigned long d3, unsigned long d4, unsigned long d5)
> +void trace(uint32_t event, uint64_t d1, uint64_t d2, uint64_t d3, uint64_t
> d4, + uint64_t d5)
> {
> struct t_buf *buf;
> struct t_rec *rec;
> Index: xen-unstable.hg-mainline.xentrace/xen/include/public/trace.h
> ===================================================================
> --- xen-unstable.hg-mainline.xentrace.orig/xen/include/public/trace.h
> +++ xen-unstable.hg-mainline.xentrace/xen/include/public/trace.h
> @@ -76,7 +76,7 @@
> struct t_rec {
> uint64_t cycles; /* cycle counter timestamp */
> uint32_t event; /* event ID */
> - unsigned long data[5]; /* event data items */
> + uint64_t data[5]; /* event data items */
> };
>
> /*
> Index: xen-unstable.hg-mainline.xentrace/xen/include/xen/trace.h
> ===================================================================
> --- xen-unstable.hg-mainline.xentrace.orig/xen/include/xen/trace.h
> +++ xen-unstable.hg-mainline.xentrace/xen/include/xen/trace.h
> @@ -33,19 +33,19 @@ void init_trace_bufs(void);
> /* used to retrieve the physical address of the trace buffers */
> int tb_control(struct xen_sysctl_tbuf_op *tbc);
>
> -void trace(u32 event, unsigned long d1, unsigned long d2,
> - unsigned long d3, unsigned long d4, unsigned long d5);
> +void trace(uint32_t event, uint64_t d1, uint64_t d2, uint64_t d3, uint64_t
> d4, + uint64_t d5);
>
> /* Avoids troubling the caller with casting their arguments to a trace
> macro */ #define trace_do_casts(e,d1,d2,d3,d4,d5) \
> do { \
> if ( unlikely(tb_init_done) ) \
> trace(e, \
> - (unsigned long)d1, \
> - (unsigned long)d2, \
> - (unsigned long)d3, \
> - (unsigned long)d4, \
> - (unsigned long)d5); \
> + (uint64_t)d1, \
> + (uint64_t)d2, \
> + (uint64_t)d3, \
> + (uint64_t)d4, \
> + (uint64_t)d5); \
> } while ( 0 )
>
> /* Convenience macros for calling the trace function. */
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

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

Re: [RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data. [ In reply to ]

dunlapg at umich

Nov 30, 2006, 9:58 AM

Post #3 of 12 (398 views)

Permalink

Hmm... this has the unfortunate side-effect of doubling the size of
the trace, and effectively halving the effectiveness of the trace
buffer in avoiding drops. My moderate-length traces are already in
the gigabyte range, and I occasionally lose trace records even with a
buffer size of 256. It would be really nice if we could avoid that.

I happen to be using the VMENTER/VMEXIT tracing, which could be
consolidated into one record if we went to a 64-bit trace. Is anyone
else doing high-bandwidth tracing that this would affect in a
significantly negative way?

-George

On 11/30/06, Tony Breeds <tony@bakeyournoodle.com> wrote:
> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
> ---
>
> xen/common/trace.c | 6 +++---
> xen/include/public/trace.h | 2 +-
> xen/include/xen/trace.h | 14 +++++++-------
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> Index: xen-unstable.hg-mainline.xentrace/xen/common/trace.c
> ===================================================================
> --- xen-unstable.hg-mainline.xentrace.orig/xen/common/trace.c
> +++ xen-unstable.hg-mainline.xentrace/xen/common/trace.c
> @@ -46,7 +46,7 @@ static int nr_recs;
> static int t_buf_highwater;
>
> /* Number of records lost due to per-CPU trace buffer being full. */
> -static DEFINE_PER_CPU(unsigned long, lost_records);
> +static DEFINE_PER_CPU(uint64_t, lost_records);
>
> /* a flag recording whether initialization has been done */
> /* or more properly, if the tbuf subsystem is enabled right now */
> @@ -228,8 +228,8 @@ int tb_control(xen_sysctl_tbuf_op_t *tbc
> * failure, otherwise 0. Failure occurs only if the trace buffers are not yet
> * initialised.
> */
> -void trace(u32 event, unsigned long d1, unsigned long d2,
> - unsigned long d3, unsigned long d4, unsigned long d5)
> +void trace(uint32_t event, uint64_t d1, uint64_t d2, uint64_t d3, uint64_t d4,
> + uint64_t d5)
> {
> struct t_buf *buf;
> struct t_rec *rec;
> Index: xen-unstable.hg-mainline.xentrace/xen/include/public/trace.h
> ===================================================================
> --- xen-unstable.hg-mainline.xentrace.orig/xen/include/public/trace.h
> +++ xen-unstable.hg-mainline.xentrace/xen/include/public/trace.h
> @@ -76,7 +76,7 @@
> struct t_rec {
> uint64_t cycles; /* cycle counter timestamp */
> uint32_t event; /* event ID */
> - unsigned long data[5]; /* event data items */
> + uint64_t data[5]; /* event data items */
> };
>
> /*
> Index: xen-unstable.hg-mainline.xentrace/xen/include/xen/trace.h
> ===================================================================
> --- xen-unstable.hg-mainline.xentrace.orig/xen/include/xen/trace.h
> +++ xen-unstable.hg-mainline.xentrace/xen/include/xen/trace.h
> @@ -33,19 +33,19 @@ void init_trace_bufs(void);
> /* used to retrieve the physical address of the trace buffers */
> int tb_control(struct xen_sysctl_tbuf_op *tbc);
>
> -void trace(u32 event, unsigned long d1, unsigned long d2,
> - unsigned long d3, unsigned long d4, unsigned long d5);
> +void trace(uint32_t event, uint64_t d1, uint64_t d2, uint64_t d3, uint64_t d4,
> + uint64_t d5);
>
> /* Avoids troubling the caller with casting their arguments to a trace macro */
> #define trace_do_casts(e,d1,d2,d3,d4,d5) \
> do { \
> if ( unlikely(tb_init_done) ) \
> trace(e, \
> - (unsigned long)d1, \
> - (unsigned long)d2, \
> - (unsigned long)d3, \
> - (unsigned long)d4, \
> - (unsigned long)d5); \
> + (uint64_t)d1, \
> + (uint64_t)d2, \
> + (uint64_t)d3, \
> + (uint64_t)d4, \
> + (uint64_t)d5); \
> } while ( 0 )
>
> /* Convenience macros for calling the trace function. */
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

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

Re: [RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data. [ In reply to ]

keir at xensource

Nov 30, 2006, 10:03 AM

Post #4 of 12 (391 views)

Permalink

On 30/11/06 16:58, "George Dunlap" <dunlapg@umich.edu> wrote:

> Hmm... this has the unfortunate side-effect of doubling the size of
> the trace, and effectively halving the effectiveness of the trace
> buffer in avoiding drops. My moderate-length traces are already in
> the gigabyte range, and I occasionally lose trace records even with a
> buffer size of 256. It would be really nice if we could avoid that.
>
> I happen to be using the VMENTER/VMEXIT tracing, which could be
> consolidated into one record if we went to a 64-bit trace. Is anyone
> else doing high-bandwidth tracing that this would affect in a
> significantly negative way?

As we move increasingly towards x86/64 this is an issue that will need to be
addressed even if we leave the tracing fields as longs.

-- Keir

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

Re: [RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data. [ In reply to ]

mark.williamson at cl

Nov 30, 2006, 7:29 PM

Post #5 of 12 (396 views)

Permalink

I guess one possibility would be to continue using unsigned longs but store
the machine word size and endianness in a header in the trace file. This
gets us platform independence.

This avoids adding extra overhead on the fast path, the extra processing can
happen offline (and probably not at all in the common case that you're on the
same endianness / word size as the trace was collected on).

Another alternative would be to allow some combination of 32-bit or (fewer)
64-bit words in the record. This would let us keep the same record size, but
have a bit more flexibility.

Going the whole hog, we could even make the trace data opaque to trace.c -
have a char[] for the data, and deal with the semantics in terms
of "longs" "u64" etc in macros in the traced code, and in xentrace_format.

If we did this, the logical extension would be to have variable length trace
records with a fixed-size header giving the full length. I think this would
be a good direction to go in, and would ensure that we maximise use of the
trace buffer space. It shouldn't be that hard to modify the system to do
this - most of the work may even be in making it nice to use!

Cheers,
Mark

On Thursday 30 November 2006 17:03, Keir Fraser wrote:
> On 30/11/06 16:58, "George Dunlap" <dunlapg@umich.edu> wrote:
> > Hmm... this has the unfortunate side-effect of doubling the size of
> > the trace, and effectively halving the effectiveness of the trace
> > buffer in avoiding drops. My moderate-length traces are already in
> > the gigabyte range, and I occasionally lose trace records even with a
> > buffer size of 256. It would be really nice if we could avoid that.
> >
> > I happen to be using the VMENTER/VMEXIT tracing, which could be
> > consolidated into one record if we went to a 64-bit trace. Is anyone
> > else doing high-bandwidth tracing that this would affect in a
> > significantly negative way?
>
> As we move increasingly towards x86/64 this is an issue that will need to
> be addressed even if we leave the tracing fields as longs.
>
> -- Keir
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

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

Re: [RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data. [ In reply to ]

jimix at watson

Dec 1, 2006, 4:32 AM

Post #6 of 12 (394 views)

Permalink

I'll go one (huge) step further...
should we adopt LTTng? http://ltt.polymtl.ca/
The trace stanzas are well defined and comes with a bunch of
visualization and analysis tools.
I know a lot of people in at IBM have been using it for performance
studies of various operating systems and I have been asked how Xen
could support if for Xen but for domains as well.
(http://ltt.polymtl.ca/svn/ltt/branches/poly/doc/developer/xen-port.txt)

-JX

On Nov 30, 2006, at 9:29 PM, Mark Williamson wrote:

> I guess one possibility would be to continue using unsigned longs
> but store
> the machine word size and endianness in a header in the trace
> file. This
> gets us platform independence.
>
> This avoids adding extra overhead on the fast path, the extra
> processing can
> happen offline (and probably not at all in the common case that
> you're on the
> same endianness / word size as the trace was collected on).
>
> Another alternative would be to allow some combination of 32-bit or
> (fewer)
> 64-bit words in the record. This would let us keep the same record
> size, but
> have a bit more flexibility.
>
> Going the whole hog, we could even make the trace data opaque to
> trace.c -
> have a char[] for the data, and deal with the semantics in terms
> of "longs" "u64" etc in macros in the traced code, and in
> xentrace_format.
>
> If we did this, the logical extension would be to have variable
> length trace
> records with a fixed-size header giving the full length. I think
> this would
> be a good direction to go in, and would ensure that we maximise use
> of the
> trace buffer space. It shouldn't be that hard to modify the system
> to do
> this - most of the work may even be in making it nice to use!
>
> Cheers,
> Mark
>
> On Thursday 30 November 2006 17:03, Keir Fraser wrote:
>> On 30/11/06 16:58, "George Dunlap" <dunlapg@umich.edu> wrote:
>>> Hmm... this has the unfortunate side-effect of doubling the size of
>>> the trace, and effectively halving the effectiveness of the trace
>>> buffer in avoiding drops. My moderate-length traces are already in
>>> the gigabyte range, and I occasionally lose trace records even
>>> with a
>>> buffer size of 256. It would be really nice if we could avoid that.
>>>
>>> I happen to be using the VMENTER/VMEXIT tracing, which could be
>>> consolidated into one record if we went to a 64-bit trace. Is
>>> anyone
>>> else doing high-bandwidth tracing that this would affect in a
>>> significantly negative way?
>>
>> As we move increasingly towards x86/64 this is an issue that will
>> need to
>> be addressed even if we leave the tracing fields as longs.
>>
>> -- Keir
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
> --
> 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

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

Re: [RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data. [ In reply to ]

dunlapg at umich

Dec 1, 2006, 10:54 AM

Post #7 of 12 (396 views)

Permalink

Variable-length trace buffers would certainly be nice. Right now, for
some records, I'm squeezing bits into things, and just *one* more byte
would be great... but for other records, I'm storing 4 words of 0, and
there just doesn't seem to be any other useful information to add.

Keir, is the simpler, "one trace size fits all" method just because it
was easier to implement originally, or is the simplicity expected to
greatly reduce overhead and/or bugginess? If the former, then there
seems enough interest in making the tracing more flexible to be worth
changing; if the latter, then we should probably chose something and
live with it, or perhaps a compromise (i.e., two record sizes).

-George

On 11/30/06, Mark Williamson <mark.williamson@cl.cam.ac.uk> wrote:
> I guess one possibility would be to continue using unsigned longs but store
> the machine word size and endianness in a header in the trace file. This
> gets us platform independence.
>
> This avoids adding extra overhead on the fast path, the extra processing can
> happen offline (and probably not at all in the common case that you're on the
> same endianness / word size as the trace was collected on).
>
> Another alternative would be to allow some combination of 32-bit or (fewer)
> 64-bit words in the record. This would let us keep the same record size, but
> have a bit more flexibility.
>
> Going the whole hog, we could even make the trace data opaque to trace.c -
> have a char[] for the data, and deal with the semantics in terms
> of "longs" "u64" etc in macros in the traced code, and in xentrace_format.
>
> If we did this, the logical extension would be to have variable length trace
> records with a fixed-size header giving the full length. I think this would
> be a good direction to go in, and would ensure that we maximise use of the
> trace buffer space. It shouldn't be that hard to modify the system to do
> this - most of the work may even be in making it nice to use!
>
> Cheers,
> Mark
>
> On Thursday 30 November 2006 17:03, Keir Fraser wrote:
> > On 30/11/06 16:58, "George Dunlap" <dunlapg@umich.edu> wrote:
> > > Hmm... this has the unfortunate side-effect of doubling the size of
> > > the trace, and effectively halving the effectiveness of the trace
> > > buffer in avoiding drops. My moderate-length traces are already in
> > > the gigabyte range, and I occasionally lose trace records even with a
> > > buffer size of 256. It would be really nice if we could avoid that.
> > >
> > > I happen to be using the VMENTER/VMEXIT tracing, which could be
> > > consolidated into one record if we went to a 64-bit trace. Is anyone
> > > else doing high-bandwidth tracing that this would affect in a
> > > significantly negative way?
> >
> > As we move increasingly towards x86/64 this is an issue that will need to
> > be addressed even if we leave the tracing fields as longs.
> >
> > -- Keir
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
>
> --
> 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

Re: [RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data. [ In reply to ]

keir at xensource

Dec 1, 2006, 11:37 AM

Post #8 of 12 (392 views)

Permalink

On 1/12/06 5:54 pm, "George Dunlap" <dunlapg@umich.edu> wrote:

> Variable-length trace buffers would certainly be nice. Right now, for
> some records, I'm squeezing bits into things, and just *one* more byte
> would be great... but for other records, I'm storing 4 words of 0, and
> there just doesn't seem to be any other useful information to add.
>
> Keir, is the simpler, "one trace size fits all" method just because it
> was easier to implement originally, or is the simplicity expected to
> greatly reduce overhead and/or bugginess? If the former, then there
> seems enough interest in making the tracing more flexible to be worth
> changing; if the latter, then we should probably chose something and
> live with it, or perhaps a compromise (i.e., two record sizes).

There's no reason not to make the trace format more flexible. There's a
question about how you represent trace points in the Xen code though, when
the format is no longer a list of fixed size integers.

-- Keir

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

[RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data. [ In reply to ]

tony at bakeyournoodle

Dec 3, 2006, 8:47 PM

Post #9 of 12 (392 views)

Permalink

Re: [RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data. [ In reply to ]

mark.williamson at cl

Dec 4, 2006, 11:20 AM

Post #10 of 12 (400 views)

Permalink

> Variable-length trace buffers would certainly be nice. Right now, for
> some records, I'm squeezing bits into things, and just *one* more byte
> would be great... but for other records, I'm storing 4 words of 0, and
> there just doesn't seem to be any other useful information to add.

Yep, this is really something we should look about improving now.

> Keir, is the simpler, "one trace size fits all" method just because it
> was easier to implement originally, or is the simplicity expected to
> greatly reduce overhead and/or bugginess?

It's just there because it was simpler to implement. I'm confident that we
can come up with a more flexible format that won't have a detrimental effect
on performance / stability.

> If the former, then there
> seems enough interest in making the tracing more flexible to be worth
> changing; if the latter, then we should probably chose something and
> live with it, or perhaps a compromise (i.e., two record sizes).

Two record sizes might be an interesting compromise, but I suggest we go the
whole hog and spec a design for fully variable data lengths with a fixed
length header.

Cheers,
Mark

> -George
>
> On 11/30/06, Mark Williamson <mark.williamson@cl.cam.ac.uk> wrote:
> > I guess one possibility would be to continue using unsigned longs but
> > store the machine word size and endianness in a header in the trace file.
> > This gets us platform independence.
> >
> > This avoids adding extra overhead on the fast path, the extra processing
> > can happen offline (and probably not at all in the common case that
> > you're on the same endianness / word size as the trace was collected on).
> >
> > Another alternative would be to allow some combination of 32-bit or
> > (fewer) 64-bit words in the record. This would let us keep the same
> > record size, but have a bit more flexibility.
> >
> > Going the whole hog, we could even make the trace data opaque to trace.c
> > - have a char[] for the data, and deal with the semantics in terms of
> > "longs" "u64" etc in macros in the traced code, and in xentrace_format.
> >
> > If we did this, the logical extension would be to have variable length
> > trace records with a fixed-size header giving the full length. I think
> > this would be a good direction to go in, and would ensure that we
> > maximise use of the trace buffer space. It shouldn't be that hard to
> > modify the system to do this - most of the work may even be in making it
> > nice to use!
> >
> > Cheers,
> > Mark
> >
> > On Thursday 30 November 2006 17:03, Keir Fraser wrote:
> > > On 30/11/06 16:58, "George Dunlap" <dunlapg@umich.edu> wrote:
> > > > Hmm... this has the unfortunate side-effect of doubling the size of
> > > > the trace, and effectively halving the effectiveness of the trace
> > > > buffer in avoiding drops. My moderate-length traces are already in
> > > > the gigabyte range, and I occasionally lose trace records even with a
> > > > buffer size of 256. It would be really nice if we could avoid that.
> > > >
> > > > I happen to be using the VMENTER/VMEXIT tracing, which could be
> > > > consolidated into one record if we went to a 64-bit trace. Is anyone
> > > > else doing high-bandwidth tracing that this would affect in a
> > > > significantly negative way?
> > >
> > > As we move increasingly towards x86/64 this is an issue that will need
> > > to be addressed even if we leave the tracing fields as longs.
> > >
> > > -- Keir
> > >
> > >
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@lists.xensource.com
> > > http://lists.xensource.com/xen-devel
> >
> > --
> > 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!

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

Re: [RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data. [ In reply to ]

mark.williamson at cl

Dec 5, 2006, 9:54 AM

Post #11 of 12 (397 views)

Permalink

> There's no reason not to make the trace format more flexible. There's a
> question about how you represent trace points in the Xen code though, when
> the format is no longer a list of fixed size integers.

I can see two main possibilities. One involving a variadic function and one
involving mega macros of doom.

One possibility would be a trace() function taking a variable number of
arguments, i.e.

void trace(type, unsigned char data1, unsigned char data2, ... etc)

And a set of arch-defined macros (or at least bitness / endian defined
macros). Eg. on x86 we might have:

#define TRACE_U16(d) ((unsigned char)(d & 255)), ((unsigned char)(d >> 8))

We'd need to verify whether the extra processing had a measurable performance
impact, however.

Another alternative would be to make the array of trace buffers globally
accessible and then use a set of macros for the trace() instead of an inline
function. The macros could then look something like (pseudocode):

struct trace_record {
u32 type;
u32 data_len;
char data[]
};

char *trace_buffer[NR_CPUS]

#define open_trace(type) \
do { \
disable local irqs \
struct trace_record *record = \
&trace_buffer[cpu][producer_idx]; \
record->type = (u32)type \
record->data_len = 0;

#define trace_u16(data) *(u16 *)record->data[record->data_len] = data \
record->data_len += sizeof(u16)

... etc for different data types, with appropriate variations if necessary for
different platforms ...

#define close_trace() \
inc producer counter by sizeof(struct \
trace_record) + record->data_len for userspace \
to see \
re-enable local irqs \
} while(0)

Things become unhappy here because there'd need to be some kind of bounds
checking in here to determine whether we need to wrap to the beginning of the
trace buffer again. The alternatives as I see them would be either:

a) include code in each data macro to check if we'd reached the end of the
buffer and wrap the data appropriately, or
b) include code that'll simply copy everything we've built so far to the
beginning of the trace buffer and start again.

Either way is going to be ugly and unpleasant. Also, we have the problem of
not knowing whether we're going to wrap OR run out of space until we're part
way through the trace record, although in this instance, I guess we could
just change to create a "missed data" record.

I think the first approach (variadic function) above is probably nicer. We
can always make a few macros to make common cases (e.g. recording a type and
a single u64 of data) less verbose.

Any thoughts?

Cheers,
Mark

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

Re: [RFC][PATCH] 1/3] [XEN] Use explicit bit sized fields for exported xentrace data. [ In reply to ]

gdunlap at xensource

Dec 5, 2006, 1:05 PM

Post #12 of 12 (394 views)

Permalink

I see two other options:
* Pre-allocate a block of data and fill it in
* Allocate a struct on the stack, and copy it all at once.

In the first case you'd do something like the following:
struct {
[trace layout]
} *trec;

trec=trace_var(TRC_TYPE, sizeof(*trec), [maybe some other info]);

/* Fill in trec->* */

The second case looks similar:
struct {
[trace layout]
} trec;

/* Fill in trec.* */

trace_var(TRC_TYPE, &trec, sizeof(trec), [maybe other info]);

The second case involves an extra copy, but that shouldn't be a big
deal. It has the advantage of being self-contained, and the trace
code can make the record "wrap around" transparently.

The first means no copying, but it also means no "wrap around"; if
there's not enough room at the end of a buffer, the space would just
have to be left empty. That's not probably such a big deal, though.
The bigger problem comes if several "open" trace records happen at
once. It's technically possible that the trace buffer will wrap
around before a function is done writing to its original buffer.

In both of these cases, the common "TRACE_nD" macros can be left, I
think. We might want to add "TRACE_nDL" for 64-bit values, and then
let those who need more flexible trace structures call trace_var()
directly.

This way of doing things also has the advantage that the trace record
can be defined in a public header somewhere, and used by user-space
analysis tools as well as the hypervisor tracing code.

Thoughts?

-George

On 12/5/06, Mark Williamson <mark.williamson@cl.cam.ac.uk> wrote:
> > There's no reason not to make the trace format more flexible. There's a
> > question about how you represent trace points in the Xen code though, when
> > the format is no longer a list of fixed size integers.
>
> I can see two main possibilities. One involving a variadic function and one
> involving mega macros of doom.
>
> One possibility would be a trace() function taking a variable number of
> arguments, i.e.
>
> void trace(type, unsigned char data1, unsigned char data2, ... etc)
>
> And a set of arch-defined macros (or at least bitness / endian defined
> macros). Eg. on x86 we might have:
>
> #define TRACE_U16(d) ((unsigned char)(d & 255)), ((unsigned char)(d >> 8))
>
> We'd need to verify whether the extra processing had a measurable performance
> impact, however.
>
> Another alternative would be to make the array of trace buffers globally
> accessible and then use a set of macros for the trace() instead of an inline
> function. The macros could then look something like (pseudocode):
>
> struct trace_record {
> u32 type;
> u32 data_len;
> char data[]
> };
>
> char *trace_buffer[NR_CPUS]
>
> #define open_trace(type) \
> do { \
> disable local irqs \
> struct trace_record *record = \
> &trace_buffer[cpu][producer_idx]; \
> record->type = (u32)type \
> record->data_len = 0;
>
> #define trace_u16(data) *(u16 *)record->data[record->data_len] = data \
> record->data_len += sizeof(u16)
>
> ... etc for different data types, with appropriate variations if necessary for
> different platforms ...
>
> #define close_trace() \
> inc producer counter by sizeof(struct \
> trace_record) + record->data_len for userspace \
> to see \
> re-enable local irqs \
> } while(0)
>
>
> Things become unhappy here because there'd need to be some kind of bounds
> checking in here to determine whether we need to wrap to the beginning of the
> trace buffer again. The alternatives as I see them would be either:
>
> a) include code in each data macro to check if we'd reached the end of the
> buffer and wrap the data appropriately, or
> b) include code that'll simply copy everything we've built so far to the
> beginning of the trace buffer and start again.
>
> Either way is going to be ugly and unpleasant. Also, we have the problem of
> not knowing whether we're going to wrap OR run out of space until we're part
> way through the trace record, although in this instance, I guess we could
> just change to create a "missed data" record.
>
> I think the first approach (variadic function) above is probably nicer. We
> can always make a few macros to make common cases (e.g. recording a type and
> a single u64 of data) less verbose.
>
> Any thoughts?
>
> Cheers,
> Mark
>
> --
> 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