Re: [Xen-devel] [RFC][PATCH] 2/3] [TOOLS][XENTRACE] Update tools to write data to disk in a known endian'ness.

[Mark Williamson <mark.williamson@xxxxxxxxxxxx>]

> Use network byte order when writing data to disk, making the data portable
> to any machine.

Thanks for splitting this from the formatting changes - it's a big help.

No objections to this here, seems like a nice improvement.  I did wonder if 
perhaps some sort of trace file header would be useful, giving metadata.  For 
instance, a record of what dom0 / Xen builds were being used, and type of 
machine might be useful.  A future extensible format would be a plus too, I 
guess.  None of this is strictly necessary, it just might help with 
organising trace files.

Cheers,
Mark

> Signed-off-by: Tony Breeds <tony@xxxxxxxxxxxxxxxxxx>
> ---
>
>  tools/xentrace/xentrace.c      |   47
> ++++++++++++++++++++++++++++++++++------- tools/xentrace/xentrace_format | 
>   7 +++---
>  2 files changed, 44 insertions(+), 10 deletions(-)
>
> Index: xen-unstable.hg-mainline.xentrace/tools/xentrace/xentrace.c
> ===================================================================
> --- xen-unstable.hg-mainline.xentrace.orig/tools/xentrace/xentrace.c
> +++ xen-unstable.hg-mainline.xentrace/tools/xentrace/xentrace.c
> @@ -28,6 +28,20 @@
>
>  #include <xenctrl.h>
>
> +#include <arpa/inet.h> /* hton*(), ntoh*() */
> +#include <endian.h>
> +
> +/* There is no 64-bit htonll, so create one */
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> +#define        htonll(x)   ( (((uint64_t)htonl(x)) << 32) + htonl(x >> 32)
> ) +#define        ntohll(x)   ( (((uint64_t)ntohl(x)) << 32) + ntohl(x >>
> 32) ) +#else
> +#define        htonll(x)   (x)
> +#define        ntohll(x)   (x)
> +#endif
> +
> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> +
>  #define PERROR(_m, _a...)                                       \
>  do {                                                            \
>      int __saved_errno = errno;                                  \
> @@ -90,12 +104,30 @@ struct timespec millis_to_timespec(unsig
>   * Outputs the trace record to a filestream, prepending the CPU ID of the
>   * source trace buffer.
>   */
> -void write_rec(unsigned int cpu, struct t_rec *rec, FILE *out)
> +void write_rec(uint32_t cpu, struct t_rec *rec, FILE *out)
>  {
>      size_t written = 0;
> -    written += fwrite(&cpu, sizeof(cpu), 1, out);
> -    written += fwrite(rec, sizeof(*rec), 1, out);
> -    if ( written != 2 )
> +    int i;
> +    /* Place network byte order representation in temp vars, rather than
> +     * write back into kernel/xen memory */
> +    uint64_t tmp64;
> +    uint32_t tmp32;
> +
> +    tmp32 = htonl(cpu);
> +    written += fwrite(&tmp32, sizeof(tmp32), 1, out);
> +
> +    tmp64 = htonll(rec->cycles);
> +    written += fwrite(&tmp64, sizeof(tmp64), 1, out);
> +
> +    tmp32 = htonl(rec->event);
> +    written += fwrite(&tmp32, sizeof(tmp32), 1, out);
> +
> +    for ( i=0; i<ARRAY_SIZE(rec->data); i++ ) {
> +        tmp64 = htonl(rec->data[i]);
> +        written += fwrite(&tmp64, sizeof(tmp64), 1, out);
> +    }
> +
> +    if ( written != 8 )
>      {
>          PERROR("Failed to write trace record");
>          exit(EXIT_FAILURE);
> @@ -147,6 +179,7 @@ struct t_buf *map_tbufs(unsigned long tb
>          exit(EXIT_FAILURE);
>      }
>
> +    /* On PPC (At least) the DOMID arg is ignored in dom0 */
>      tbufs_mapped = xc_map_foreign_range(xc_handle, DOMID_XEN,
>                                          size * num, PROT_READ |
> PROT_WRITE, tbufs_mfn);
> @@ -253,7 +286,7 @@ struct t_rec **init_rec_ptrs(struct t_bu
>  /**
>   * get_num_cpus - get the number of logical CPUs
>   */
> -unsigned int get_num_cpus(void)
> +uint32_t get_num_cpus(void)
>  {
>      xc_physinfo_t physinfo;
>      int xc_handle = xc_interface_open();
> @@ -282,14 +315,14 @@ unsigned int get_num_cpus(void)
>   */
>  int monitor_tbufs(FILE *logfile)
>  {
> -    int i;
> +    uint32_t i;
>
>      void *tbufs_mapped;          /* pointer to where the tbufs are mapped 
>   */ struct t_buf **meta;         /* pointers to the trace buffer metadata 
>   */ struct t_rec **data;         /* pointers to the trace buffer data
> areas * where they are mapped into user space.   */ unsigned long
> tbufs_mfn;     /* mfn of the tbufs                         */ -    unsigned
> int  num;           /* number of trace buffers / logical CPUS   */ +   
> uint32_t num;                /* number of trace buffers / logical CPUS   */
> unsigned long size;          /* size of a single trace buffer            */
>
>      int size_in_recs;
> Index: xen-unstable.hg-mainline.xentrace/tools/xentrace/xentrace_format
> ===================================================================
> --- xen-unstable.hg-mainline.xentrace.orig/tools/xentrace/xentrace_format
> +++ xen-unstable.hg-mainline.xentrace/tools/xentrace/xentrace_format
> @@ -84,10 +84,11 @@ interrupted = 0
>  defs = read_defs(arg[0])
>
>  # structure of trace record + prepended CPU id (as output by xentrace):
> -# CPU(I) TSC(Q) EVENT(L) D1(L) D2(L) D3(L) D4(L) D5(L)
> +# CPU(L) TSC(Q) EVENT(L) D1(Q) D2(Q) D3(Q) D4(Q) D5(Q)
>  # read CPU id separately to avoid structure packing problems on 64-bit
> arch. -CPUREC = "I"
> -TRCREC = "QLLLLLL"
> +# Force network byte order.
> +CPUREC = "!L"
> +TRCREC = "!QLQQQQQ"
>
>  last_tsc = [0]
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel