David Vrabel
2012-May-31 11:16 UTC
[PATCH 00 of 10] xenalyze: build, hypercall tracing and plugins (v2)
A collection of fixes for xenalyze improving the build, adding support for the TRC_PV_HYPERCALL_V2 records and adding a simple plugin system. Changes in v2: - updates for changes in PV_HYPERCALL_V2 format. - removed some decoding of events which don''t exist. - include updated trace.h. David
Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- .hgignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.hgignore b/.hgignore new file mode 100644 --- /dev/null +++ b/.hgignore @@ -0,0 +1,3 @@ +.*\.o +xenalyze +dump-raw
David Vrabel
2012-May-31 11:16 UTC
[PATCH 02 of 10] xenalyze: automatically generate dependencies
Use gcc''s -MD and -MP options to automatically generate dependencies. Signed-off-by: David Vrabel <david.vrabel@citrix.com> diff --git a/.hgignore b/.hgignore --- a/.hgignore +++ b/.hgignore @@ -1,3 +1,4 @@ +.*\.d .*\.o xenalyze dump-raw diff --git a/Makefile b/Makefile --- a/Makefile +++ b/Makefile @@ -1,5 +1,3 @@ -CC = gcc - CFLAGS += -g -O2 CFLAGS += -fno-strict-aliasing CFLAGS += -std=gnu99 @@ -7,22 +5,35 @@ CFLAGS += -Wall -Wstrict-prototypes CFLAGS += -Wno-unused-value CFLAGS += -Wdeclaration-after-statement -CFLAGS += -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE +CFLAGS += -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE CFLAGS += -mno-tls-direct-seg-refs -CFLAGS += -Werror +CFLAGS += -Werror -BIN = xenalyze dump-raw +BIN := xenalyze dump-raw -HDRS = trace.h analyze.h mread.h +xenalyze_OBJS := \ + mread.o \ + xenalyze.o + +dump-raw_OBJS := \ + dump-raw.o all: $(BIN) +xenalyze: $(xenalyze_OBJS) + $(CC) $(LDFLAGS) -o $@ $^ + +dump-raw: $(dump-raw_OBJS) + $(CC) $(LDFLAGS) -o $@ $^ + +%.o: %.c + $(CC) $(CFLAGS) -MD -MP -c -o $@ $< + +all_objs := $(foreach b,$(BIN),$($(b)_OBJS)) +all_deps := $(all_objs:.o=.d) + .PHONY: clean clean: - $(RM) *.a *.so *.o *.rpm $(BIN) $(LIBBIN) + $(RM) $(BIN) $(all_objs) $(all_deps) -%: %.c $(HDRS) Makefile - $(CC) $(CFLAGS) -o $@ $< - -xenalyze: xenalyze.o mread.o - $(CC) $(CFLAGS) -o $@ $^ +-include $(all_deps)
David Vrabel
2012-May-31 11:16 UTC
[PATCH 03 of 10] xenalyze: remove decode of unused events
The PV_UPDATE_VA_MAPPING event is not present in xen-unstable, nor is
it present in any of the 3.2, 3.3, 3.4 or 4.1 XenServer trees I looked
at so I can only assume this was an event that never made it upstream.
Remove the code as the event ID is now used for PV_HYPERCALL_V2.
Similarly, some of the the HW_IRQ_* events are also not used anywhere
I could find.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
diff --git a/xenalyze.c b/xenalyze.c
--- a/xenalyze.c
+++ b/xenalyze.c
@@ -1531,7 +1531,6 @@ enum {
PV_GDT_LDT_MAPPING_FAULT,
PV_PTWR_EMULATION,
PV_PTWR_EMULATION_PAE,
- PV_UPDATE_VA_MAPPING,
PV_MAX
};
@@ -6514,44 +6513,6 @@ void pv_ptwr_emulation_process(struct re
}
}
-void pv_update_va_mapping_process(struct record_info *ri, struct pv_data *pv) {
- union pv_event pevt = { .event = ri->event };
- union {
- /* gpl2 is deprecated */
- struct {
- unsigned long long val;
- unsigned int va, flags;
- } x32;
- struct {
- unsigned long long val;
- unsigned long long va, flags;
- } x64;
- } *r = (typeof(r))ri->d;
- struct {
- unsigned long long val, va, flags;
- } e;
-
- if ( pevt.x64 )
- {
- e.val = r->x64.val;
- e.va = r->x64.va;
- e.flags = r->x64.flags;
- }
- else
- {
- e.val = r->x32.val;
- e.va = r->x32.va;
- e.flags = r->x32.flags;
- }
-
- if ( opt.dump_all )
- {
- printf(" %s update_va_mapping l1e %llx va %llx flags %llx\n",
- ri->dump_header,
- e.val, e.va, e.flags);
- }
-}
-
void pv_generic_process(struct record_info *ri, struct pv_data *pv) {
union pv_event pevt = { .event = ri->event };
if ( opt.dump_all ) {
@@ -6638,9 +6599,6 @@ void pv_process(struct pcpu_info *p)
case PV_PTWR_EMULATION_PAE:
pv_ptwr_emulation_process(ri, pv);
break;
- case PV_UPDATE_VA_MAPPING:
- pv_update_va_mapping_process(ri, pv);
- break;
case PV_PAGE_FAULT:
//pv_pf_process(ri, pv);
//break;
@@ -7893,149 +7851,6 @@ void irq_process(struct pcpu_info *p) {
}
break;
}
- case TRC_HW_IRQ_MSI_WRITE:
- {
- struct {
- unsigned address_lo, address_hi;
- unsigned data;
- unsigned irq:16, pos:16;
- uint8_t func, slot, bus, type;
- unsigned mask_base;
- } *r = (typeof(r))ri->d;
-
- if ( opt.dump_all )
- {
- printf(" %s irq_msi_write irq %x t %x base %x addr %x %x data
%x pci %02x:%02x.%x %x\n",
- ri->dump_header,
- r->irq,
- r->type,
- r->mask_base,
- r->address_hi, r->address_lo,
- r->data,
- r->bus, r->slot, r->func, r->pos);
- }
- break;
- }
- case TRC_HW_IRQ_IOMMU_AMD_IRE:
- {
- struct {
- uint16_t bdf, id;
- int offset;
- uint8_t dest_mode, dev_mode, vector, dest;
- } *r = (typeof(r))ri->d;
-
- if ( opt.dump_all )
- {
- printf(" %s irq_iommu_ire bdf %x id %x offset %x dest_mode %x
dev_mode %x vec %x dest %x\n",
- ri->dump_header,
- r->bdf, r->id,
- r->offset,
- r->dest_mode, r->dev_mode,
- r->vector, r->dest);
- }
- break;
- }
- case TRC_HW_IRQ_MAP_PIRQ_MSI:
- {
- struct {
- unsigned domain:16,
- pirq:16,
- irq:16,
- bus:16,
- devfn:16,
- entry_nr:16;
- } *r = (typeof(r))ri->d;
-
- if ( r->irq < MAX_IRQ )
- {
- struct irq_desc *irq=irq_table+r->irq;
-
- if ( irq->dev )
- {
- fprintf(warn, "Strange, irq %d already has dev
%02x:%x.%x!\n",
- r->irq, irq->dev->bus,
- irq->dev->devfn>>4,
- irq->dev->devfn&3);
- }
- else
- {
- struct pci_dev *pdev = pdev_find(r->bus, r->devfn);
-
- irq->dev=pdev;
- irq->type=IRQ_MSI;
- }
- }
-
- if ( opt.dump_all )
- {
- printf(" %s irq_map_pirq_msi d%d pirq %x(%d) irq %x bus %x
devfn %x entry %x\n",
- ri->dump_header,
- r->domain,
- r->pirq,
- r->pirq,
- r->irq,
- r->bus,
- r->devfn,
- r->entry_nr);
- }
- break;
- }
- case TRC_HW_IRQ_MAP_PIRQ_GSI:
- {
- struct {
- unsigned domain, pirq, irq;
- } *r = (typeof(r))ri->d;
-
- if ( opt.dump_all )
- {
- printf(" %s irq_map_pirq_gsi d%d pirq %x(%d) irq %x\n",
- ri->dump_header,
- r->domain,
- r->pirq,
- r->pirq,
- r->irq);
- }
- break;
- }
- case TRC_HW_IRQ_MSI_SET_AFFINITY:
- {
- struct {
- unsigned irq, apic_id, vector;
- } *r = (typeof(r))ri->d;
-
- if ( opt.dump_all )
- {
- printf(" %s irq_msi_set_affinity irq %x apicid %x vec
%x\n",
- ri->dump_header,
- r->irq,
- r->apic_id,
- r->vector);
- }
- break;
- }
- case TRC_HW_IRQ_SET_DESC_AFFINITY:
- {
- struct {
- unsigned line:16, irq:16;
- char fname[24]; /* Extra 7 words; 6 words * 4 = 24 */
- } *r = (typeof(r))ri->d;
- char fname[25];
- int i;
-
- for(i=0; i<24; i++)
- fname[i]=r->fname[i];
- fname[i]=0;
-
- if ( opt.dump_all )
- {
- printf(" %s irq_set_desc_affinity irq %x %s:%d\n",
- ri->dump_header,
- r->irq,
- fname,
- r->line);
- }
- break;
- }
case TRC_HW_IRQ_CLEAR_VECTOR:
case TRC_HW_IRQ_MOVE_FINISH :
default:
David Vrabel
2012-May-31 11:16 UTC
[PATCH 04 of 10] xenalyze: update trace.h to match xen-unstable
Update trace.h to the version in xen-unstable (plus my PV_HYPERCALL_*
patches).
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
diff --git a/trace.h b/trace.h
--- a/trace.h
+++ b/trace.h
@@ -57,6 +57,7 @@
#define TRC_SCHED_CLASS 0x00022000 /* Scheduler-specific */
#define TRC_SCHED_VERBOSE 0x00028000 /* More inclusive scheduling */
+/* Trace classes for Hardware */
#define TRC_HW_PM 0x00801000 /* Power management traces */
#define TRC_HW_IRQ 0x00802000 /* Traces relating to the handling of
IRQs */
@@ -93,20 +94,51 @@
#define TRC_MEM_POD_ZERO_RECLAIM (TRC_MEM + 17)
#define TRC_MEM_POD_SUPERPAGE_SPLINTER (TRC_MEM + 18)
+#define TRC_PV_ENTRY 0x00201000 /* Hypervisor entry points for PV guests. */
+#define TRC_PV_SUBCALL 0x00202000 /* Sub-call in a multicall hypercall */
-#define TRC_PV_HYPERCALL (TRC_PV + 1)
-#define TRC_PV_TRAP (TRC_PV + 3)
-#define TRC_PV_PAGE_FAULT (TRC_PV + 4)
-#define TRC_PV_FORCED_INVALID_OP (TRC_PV + 5)
-#define TRC_PV_EMULATE_PRIVOP (TRC_PV + 6)
-#define TRC_PV_EMULATE_4GB (TRC_PV + 7)
-#define TRC_PV_MATH_STATE_RESTORE (TRC_PV + 8)
-#define TRC_PV_PAGING_FIXUP (TRC_PV + 9)
-#define TRC_PV_GDT_LDT_MAPPING_FAULT (TRC_PV + 10)
-#define TRC_PV_PTWR_EMULATION (TRC_PV + 11)
-#define TRC_PV_PTWR_EMULATION_PAE (TRC_PV + 12)
- /* Indicates that addresses in trace record are 64 bits */
-#define TRC_64_FLAG (0x100)
+#define TRC_PV_HYPERCALL (TRC_PV_ENTRY + 1)
+#define TRC_PV_TRAP (TRC_PV_ENTRY + 3)
+#define TRC_PV_PAGE_FAULT (TRC_PV_ENTRY + 4)
+#define TRC_PV_FORCED_INVALID_OP (TRC_PV_ENTRY + 5)
+#define TRC_PV_EMULATE_PRIVOP (TRC_PV_ENTRY + 6)
+#define TRC_PV_EMULATE_4GB (TRC_PV_ENTRY + 7)
+#define TRC_PV_MATH_STATE_RESTORE (TRC_PV_ENTRY + 8)
+#define TRC_PV_PAGING_FIXUP (TRC_PV_ENTRY + 9)
+#define TRC_PV_GDT_LDT_MAPPING_FAULT (TRC_PV_ENTRY + 10)
+#define TRC_PV_PTWR_EMULATION (TRC_PV_ENTRY + 11)
+#define TRC_PV_PTWR_EMULATION_PAE (TRC_PV_ENTRY + 12)
+#define TRC_PV_HYPERCALL_V2 (TRC_PV_ENTRY + 13)
+#define TRC_PV_HYPERCALL_SUBCALL (TRC_PV_SUBCALL + 14)
+
+/*
+ * TRC_PV_HYPERCALL_V2 format
+ *
+ * Only some of the hypercall argument are recorded. Bit fields A0 to
+ * A5 in the first extra word are set if the argument is present and
+ * the arguments themselves are packed sequentially in the following
+ * words.
+ *
+ * The TRC_64_FLAG bit is not set for these events (even if there are
+ * 64-bit arguments in the record).
+ *
+ * Word
+ * 0 bit 31 30|29 28|27 26|25 24|23 22|21 20|19 ... 0
+ * A5 |A4 |A3 |A2 |A1 |A0 |Hypercall op
+ * 1 First 32 bit (or low word of first 64 bit) arg in record
+ * 2 Second 32 bit (or high word of first 64 bit) arg in record
+ * ...
+ *
+ * A0-A5 bitfield values:
+ *
+ * 00b Argument not present
+ * 01b 32-bit argument present
+ * 10b 64-bit argument present
+ * 11b Reserved
+ */
+#define TRC_PV_HYPERCALL_V2_ARG_32(i) (0x1 << (20 + 2*(i)))
+#define TRC_PV_HYPERCALL_V2_ARG_64(i) (0x2 << (20 + 2*(i)))
+#define TRC_PV_HYPERCALL_V2_ARG_MASK (0xfff00000)
#define TRC_SHADOW_NOT_SHADOW (TRC_SHADOW + 1)
#define TRC_SHADOW_FAST_PROPAGATE (TRC_SHADOW + 2)
@@ -125,6 +157,7 @@
#define TRC_SHADOW_RESYNC_ONLY (TRC_SHADOW + 15)
/* trace events per subclass */
+#define TRC_HVM_NESTEDFLAG (0x400)
#define TRC_HVM_VMENTRY (TRC_HVM_ENTRYEXIT + 0x01)
#define TRC_HVM_VMEXIT (TRC_HVM_ENTRYEXIT + 0x02)
#define TRC_HVM_VMEXIT64 (TRC_HVM_ENTRYEXIT + TRC_64_FLAG + 0x02)
@@ -165,6 +198,7 @@
#define TRC_HVM_REALMODE_EMULATE (TRC_HVM_HANDLER + 0x22)
#define TRC_HVM_TRAP (TRC_HVM_HANDLER + 0x23)
#define TRC_HVM_TRAP_DEBUG (TRC_HVM_HANDLER + 0x24)
+#define TRC_HVM_VLAPIC (TRC_HVM_HANDLER + 0x25)
#define TRC_HVM_IOPORT_WRITE (TRC_HVM_HANDLER + 0x216)
#define TRC_HVM_IOMEM_WRITE (TRC_HVM_HANDLER + 0x217)
@@ -172,8 +206,9 @@
/* trace events for per class */
#define TRC_PM_FREQ_CHANGE (TRC_HW_PM + 0x01)
#define TRC_PM_IDLE_ENTRY (TRC_HW_PM + 0x02)
-#define TRC_PM_IDLE_EXIT (TRC_HW_PM + 0x03)
+#define TRC_PM_IDLE_EXIT (TRC_HW_PM + 0x03)
+/* Trace events for IRQs */
#define TRC_HW_IRQ_MOVE_CLEANUP_DELAY (TRC_HW_IRQ + 0x1)
#define TRC_HW_IRQ_MOVE_CLEANUP (TRC_HW_IRQ + 0x2)
#define TRC_HW_IRQ_BIND_VECTOR (TRC_HW_IRQ + 0x3)
@@ -182,12 +217,15 @@
#define TRC_HW_IRQ_ASSIGN_VECTOR (TRC_HW_IRQ + 0x6)
#define TRC_HW_IRQ_UNMAPPED_VECTOR (TRC_HW_IRQ + 0x7)
#define TRC_HW_IRQ_HANDLED (TRC_HW_IRQ + 0x8)
-#define TRC_HW_IRQ_MSI_WRITE (TRC_HW_IRQ + 0x9)
-#define TRC_HW_IRQ_MAP_PIRQ_MSI (TRC_HW_IRQ + 0xa)
-#define TRC_HW_IRQ_MAP_PIRQ_GSI (TRC_HW_IRQ + 0xb)
-#define TRC_HW_IRQ_MSI_SET_AFFINITY (TRC_HW_IRQ + 0x10)
-#define TRC_HW_IRQ_SET_DESC_AFFINITY (TRC_HW_IRQ + 0x11)
-#define TRC_HW_IRQ_IOMMU_AMD_IRE (TRC_HW_IRQ + 0x12)
+
+/*
+ * Event Flags
+ *
+ * Some events (e.g, TRC_PV_TRAP and TRC_HVM_IOMEM_READ) have multiple
+ * record formats. These event flags distinguish between the
+ * different formats.
+ */
+#define TRC_64_FLAG 0x100 /* Addresses are 64 bits (instead of 32 bits) */
/* This structure represents a single trace buffer record. */
struct t_rec {
@@ -205,6 +243,34 @@ struct t_rec {
} u;
};
+/*
+ * This structure contains the metadata for a single trace buffer. The head
+ * field, indexes into an array of struct t_rec''s.
+ */
+struct t_buf {
+ /* Assume the data buffer size is X. X is generally not a power of 2.
+ * CONS and PROD are incremented modulo (2*X):
+ * 0 <= cons < 2*X
+ * 0 <= prod < 2*X
+ * This is done because addition modulo X breaks at 2^32 when X is not a
+ * power of 2:
+ * (((2^32 - 1) % X) + 1) % X != (2^32) % X
+ */
+ uint32_t cons; /* Offset of next item to be consumed by control tools. */
+ uint32_t prod; /* Offset of next item to be produced by Xen. */
+ /* Records follow immediately after the meta-data header. */
+};
+
+/* Structure used to pass MFNs to the trace buffers back to trace consumers.
+ * Offset is an offset into the mapped structure where the mfn list will be
held.
+ * MFNs will be at ((unsigned long *)(t_info))+(t_info->cpu_offset[cpu]).
+ */
+struct t_info {
+ uint16_t tbuf_size; /* Size in pages of each trace buffer */
+ uint16_t mfn_offset[]; /* Offset within t_info structure of the page list
per cpu */
+ /* MFN lists immediately after the header */
+};
+
#endif /* __XEN_PUBLIC_TRACE_H__ */
/*
David Vrabel
2012-May-31 11:16 UTC
[PATCH 05 of 10] xenalyze: correctly display of count of HW events
HW events where being shown under "(null)" in the summary.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
diff --git a/xenalyze.c b/xenalyze.c
--- a/xenalyze.c
+++ b/xenalyze.c
@@ -1805,7 +1805,8 @@ enum {
TOPLEVEL_MEM,
TOPLEVEL_PV,
TOPLEVEL_SHADOW,
- TOPLEVEL_MAX=12
+ TOPLEVEL_HW,
+ TOPLEVEL_MAX=TOPLEVEL_HW+1,
};
char * toplevel_name[TOPLEVEL_MAX] = {
@@ -1816,6 +1817,7 @@ char * toplevel_name[TOPLEVEL_MAX] = {
[TOPLEVEL_MEM]="mem",
[TOPLEVEL_PV]="pv",
[TOPLEVEL_SHADOW]="shadow",
+ [TOPLEVEL_HW]="hw",
};
struct trace_volume {
@@ -8595,7 +8597,6 @@ void process_cpu_change(struct pcpu_info
}
}
-#define TOPLEVEL_MAX 16
struct tl_assert_mask {
unsigned p_current:1,
not_idle_domain:1;
David Vrabel
2012-May-31 11:16 UTC
[PATCH 06 of 10] xenalyze: move struct record_info into a header
Split out struct record_info onto the analyze.h so it can be used
outside of xenalyze.c.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
diff --git a/analyze.h b/analyze.h
--- a/analyze.h
+++ b/analyze.h
@@ -1,5 +1,8 @@
#ifndef __ANALYZE_H
# define __ANALYZE_H
+
+#include <stdint.h>
+
#define TRC_GEN_MAIN 0
#define TRC_SCHED_MAIN 1
#define TRC_DOM0OP_MAIN 2
@@ -47,4 +50,56 @@ enum {
};
#define TRC_HVM_OP_DESTROY_PROC (TRC_HVM_HANDLER + 0x100)
+
+typedef unsigned long long tsc_t;
+
+/* -- on-disk trace buffer definitions -- */
+struct trace_record {
+ union {
+ struct {
+ unsigned event:28,
+ extra_words:3,
+ cycle_flag:1;
+ union {
+ struct {
+ uint32_t tsc_lo, tsc_hi;
+ uint32_t data[7];
+ } tsc;
+ struct {
+ uint32_t data[7];
+ } notsc;
+ } u;
+ };
+ uint32_t raw[8];
+ };
+};
+
+/* -- General info about a current record -- */
+struct time_struct {
+ unsigned long long time;
+ unsigned int s, ns;
+};
+
+#define DUMP_HEADER_MAX 256
+
+struct record_info {
+ int cpu;
+ tsc_t tsc;
+ union {
+ unsigned event;
+ struct {
+ unsigned minor:12,
+ sub:4,
+ main:12,
+ unused:4;
+ } evt;
+ };
+ int extra_words;
+ int size;
+ uint32_t *d;
+ char dump_header[DUMP_HEADER_MAX];
+ struct time_struct t;
+ struct trace_record rec;
+};
+
#endif
diff --git a/xenalyze.c b/xenalyze.c
--- a/xenalyze.c
+++ b/xenalyze.c
@@ -40,8 +40,6 @@
struct mread_ctrl;
-typedef unsigned long long tsc_t;
-
#define DEFAULT_CPU_HZ 2400000000LL
#define ADDR_SPACE_BITS 48
#define DEFAULT_SAMPLE_SIZE 10240
@@ -260,57 +258,8 @@ struct {
.interval = { .msec = DEFAULT_INTERVAL_LENGTH },
};
-/* -- on-disk trace buffer definitions -- */
-struct trace_record {
- union {
- struct {
- unsigned event:28,
- extra_words:3,
- cycle_flag:1;
- union {
- struct {
- uint32_t tsc_lo, tsc_hi;
- uint32_t data[7];
- } tsc;
- struct {
- uint32_t data[7];
- } notsc;
- } u;
- };
- uint32_t raw[8];
- };
-};
-
FILE *warn = NULL;
-/* -- General info about a current record -- */
-struct time_struct {
- unsigned long long time;
- unsigned int s, ns;
-};
-
-#define DUMP_HEADER_MAX 256
-
-struct record_info {
- int cpu;
- tsc_t tsc;
- union {
- unsigned event;
- struct {
- unsigned minor:12,
- sub:4,
- main:12,
- unused:4;
- } evt;
- };
- int extra_words;
- int size;
- uint32_t *d;
- char dump_header[DUMP_HEADER_MAX];
- struct time_struct t;
- struct trace_record rec;
-};
-
/* -- Summary data -- */
struct cycle_framework {
tsc_t first_tsc, last_tsc, total_cycles;
David Vrabel
2012-May-31 11:16 UTC
[PATCH 07 of 10] xenalyze: decode PV_HYPERCALL_V2 records
Newer version of Xen produce TRC_PV_HYPERCALL_V2 records instead of
the older TRC_PV_HYPERCALL format. This updated format doesn''t
included the IP but it does include select hypercall arguments.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
diff --git a/pv.h b/pv.h
new file mode 100644
--- /dev/null
+++ b/pv.h
@@ -0,0 +1,41 @@
+/*
+ * PV event decoding.
+ *
+ * Copyright (C) 2012 Citrix Systems R&D Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#ifndef __PV_H
+
+#include "analyze.h"
+#include "trace.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define ARG_MISSING 0x0
+#define ARG_32BIT 0x1
+#define ARG_64BIT 0x2
+
+#define MMU_UPDATE_PREEMPTED (~(~0U>>1))
+
+static inline uint32_t pv_hypercall_op(const struct record_info *ri)
+{
+ return ri->d[0] & ~TRC_PV_HYPERCALL_V2_ARG_MASK;
+}
+
+static inline int pv_hypercall_arg_present(const struct record_info *ri, int
arg)
+{
+ return (ri->d[0] >> (20 + 2*arg)) & 0x3;
+}
+
+uint64_t pv_hypercall_arg(const struct record_info *ri, int i);
+
+#ifdef __cplusplus
+} /* extern "C" */
+#endif
+
+#endif
diff --git a/xenalyze.c b/xenalyze.c
--- a/xenalyze.c
+++ b/xenalyze.c
@@ -32,6 +32,7 @@
#include "trace.h"
#include "analyze.h"
#include "mread.h"
+#include "pv.h"
#include <errno.h>
#include <strings.h>
#include <string.h>
@@ -1480,6 +1481,7 @@ enum {
PV_GDT_LDT_MAPPING_FAULT,
PV_PTWR_EMULATION,
PV_PTWR_EMULATION_PAE,
+ PV_HYPERCALL_V2 = 13,
PV_MAX
};
@@ -6518,6 +6520,96 @@ void pv_summary(struct pv_data *pv) {
}
}
+uint64_t pv_hypercall_arg(const struct record_info *ri, int arg)
+{
+ int i, word;
+
+ for (i = 0, word = 1; i < 6 && word < ri->extra_words;
i++) {
+ int present = pv_hypercall_arg_present(ri, i);
+
+ /* Is this the argument we''re looking for? */
+ if (i == arg) {
+ switch (present) {
+ case ARG_MISSING:
+ return 0;
+ case ARG_32BIT:
+ return ri->d[word];
+ case ARG_64BIT:
+ return (uint64_t)(ri->d[word + 1]) | ri->d[word];
+ }
+ }
+
+ /* Skip over any words for this argument. */
+ word += present;
+ }
+
+ return 0;
+}
+
+static const char *grant_table_op_cmd_to_str(uint32_t cmd)
+{
+ const char *cmd_str[] = {
+ "map_grant_ref", "unmap_grant_ref",
"setup_table", "dump_table",
+ "transfer", "copy", "query_size",
"unmap_and_replace",
+ "set_version", "get_status_frames",
"get_version", "swap_grant_ref",
+ };
+ static char buf[32];
+
+ if (cmd <= 11)
+ return cmd_str[cmd];
+
+ snprintf(buf, sizeof(buf), "unknown (%d)", cmd);
+ return buf;
+}
+
+void pv_hypercall_v2_process(struct record_info *ri, struct pv_data *pv)
+{
+ int op = pv_hypercall_op(ri);
+
+ if(opt.summary_info) {
+ if(op < PV_HYPERCALL_MAX)
+ pv->hypercall_count[op]++;
+ }
+
+ if(opt.dump_all) {
+ if(op < HYPERCALL_MAX)
+ printf(" %s hypercall %2x (%s)",
+ ri->dump_header, op, hypercall_name[op]);
+ else
+ printf(" %s hypercall %2x",
+ ri->dump_header, op);
+ switch(op) {
+ case HYPERCALL_mmu_update:
+ {
+ uint32_t count = pv_hypercall_arg(ri, 1);
+ printf(" %d updates%s", count &
~MMU_UPDATE_PREEMPTED,
+ (count & MMU_UPDATE_PREEMPTED) ? "
(preempted)" : "");
+ }
+ break;
+ case HYPERCALL_multicall:
+ {
+ uint32_t calls = pv_hypercall_arg(ri, 1);
+ printf(" %d calls", calls);
+ }
+ break;
+ case HYPERCALL_grant_table_op:
+ {
+ uint32_t cmd = pv_hypercall_arg(ri, 0);
+ uint32_t count = pv_hypercall_arg(ri, 2);
+ printf(" %s %d ops", grant_table_op_cmd_to_str(cmd),
count);
+ }
+ break;
+ case HYPERCALL_mmuext_op:
+ {
+ uint32_t count = pv_hypercall_arg(ri, 1);
+ printf(" %d ops", count);
+ }
+ break;
+ }
+ printf("\n");
+ }
+}
+
void pv_process(struct pcpu_info *p)
{
struct record_info *ri = &p->ri;
@@ -6550,9 +6642,9 @@ void pv_process(struct pcpu_info *p)
case PV_PTWR_EMULATION_PAE:
pv_ptwr_emulation_process(ri, pv);
break;
- case PV_PAGE_FAULT:
- //pv_pf_process(ri, pv);
- //break;
+ case PV_HYPERCALL_V2:
+ pv_hypercall_v2_process(ri, pv);
+ break;
default:
pv_generic_process(ri, pv);
break;
David Vrabel
2012-May-31 11:16 UTC
[PATCH 08 of 10] xenalyze: decode PV_HYPERCALL_SUBCALL events
Decode the PV_HYPERCALL_SUBCALL events which are used for calls within
a multicall hypercall. They are indented so its easier to see which
multicall they were part of.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
diff --git a/xenalyze.c b/xenalyze.c
--- a/xenalyze.c
+++ b/xenalyze.c
@@ -1482,6 +1482,7 @@ enum {
PV_PTWR_EMULATION,
PV_PTWR_EMULATION_PAE,
PV_HYPERCALL_V2 = 13,
+ PV_HYPERCALL_SUBCALL = 14,
PV_MAX
};
@@ -6562,7 +6563,8 @@ static const char *grant_table_op_cmd_to
return buf;
}
-void pv_hypercall_v2_process(struct record_info *ri, struct pv_data *pv)
+void pv_hypercall_v2_process(struct record_info *ri, struct pv_data *pv,
+ const char *indent)
{
int op = pv_hypercall_op(ri);
@@ -6573,11 +6575,11 @@ void pv_hypercall_v2_process(struct reco
if(opt.dump_all) {
if(op < HYPERCALL_MAX)
- printf(" %s hypercall %2x (%s)",
- ri->dump_header, op, hypercall_name[op]);
+ printf(" %s%s hypercall %2x (%s)",
+ ri->dump_header, indent, op, hypercall_name[op]);
else
- printf(" %s hypercall %2x",
- ri->dump_header, op);
+ printf(" %s%s hypercall %2x",
+ ri->dump_header, indent, op);
switch(op) {
case HYPERCALL_mmu_update:
{
@@ -6643,7 +6645,10 @@ void pv_process(struct pcpu_info *p)
pv_ptwr_emulation_process(ri, pv);
break;
case PV_HYPERCALL_V2:
- pv_hypercall_v2_process(ri, pv);
+ pv_hypercall_v2_process(ri, pv, "");
+ break;
+ case PV_HYPERCALL_SUBCALL:
+ pv_hypercall_v2_process(ri, pv, " ");
break;
default:
pv_generic_process(ri, pv);
David Vrabel
2012-May-31 11:16 UTC
[PATCH 09 of 10] xenalyze: add a basic plugin infrastructure
Allow xenalyze to be include (at build time) plugins that can do
per-record actions and a summary. These plugins can be in C or C++.
The plugins entry points are in struct plugin and pointers to all the
plugins linked in xenalyze are placed in a "plugin" section so
plugin_init() can find them all.
A new command line option (-p, --plugin=PLUGIN) is added to enable one
or more plugins.
A sample plugin (skeleton) is included (mostly because at least one
plugin must be present for the build to work).
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
Makefile | 10 +++---
analyze.h | 55 ++++++++++++++++++++++++++++++++++
plugin.cc | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++
plugin.h | 48 +++++++++++++++++++++++++++++
plugin.hh | 42 ++++++++++++++++++++++++++
plugins/skeleton.cc | 31 +++++++++++++++++++
xenalyze.c | 72 +++++++++++++-------------------------------
7 files changed, 287 insertions(+), 55 deletions(-)
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,4 @@
-CFLAGS += -g -O2
+CFLAGS += -g -O2 -I.
CFLAGS += -fno-strict-aliasing
CFLAGS += -std=gnu99
CFLAGS += -Wall -Wstrict-prototypes
@@ -9,11 +9,15 @@ CFLAGS += -D_LARGEFILE_SOURCE -D_LARGEFI
CFLAGS += -mno-tls-direct-seg-refs
CFLAGS += -Werror
+CXXFLAGS := -g -O2 -I. -Wall -Werror -std=c++0x
+
BIN := xenalyze dump-raw
xenalyze_OBJS := \
mread.o \
- xenalyze.o
+ plugin.o \
+ xenalyze.o \
+ plugins/skeleton.o
dump-raw_OBJS := \
dump-raw.o
@@ -21,7 +25,7 @@ dump-raw_OBJS := \
all: $(BIN)
xenalyze: $(xenalyze_OBJS)
- $(CC) $(LDFLAGS) -o $@ $^
+ $(CXX) $(LDFLAGS) -o $@ $^
dump-raw: $(dump-raw_OBJS)
$(CC) $(LDFLAGS) -o $@ $^
@@ -29,6 +33,9 @@ dump-raw: $(dump-raw_OBJS)
%.o: %.c
$(CC) $(CFLAGS) -MD -MP -c -o $@ $<
+%.o: %.cc
+ $(CXX) $(CXXFLAGS) -MD -MP -c -o $@ $<
+
all_objs := $(foreach b,$(BIN),$($(b)_OBJS))
all_deps := $(all_objs:.o=.d)
diff --git a/plugin.cc b/plugin.cc
new file mode 100644
--- /dev/null
+++ b/plugin.cc
@@ -0,0 +1,84 @@
+/*
+ * Xenalyze plugin infrastructure.
+ *
+ * Copyright (C) 2012, Citrix Systems R&D Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#include <stdio.h>
+#include <string.h>
+#include <list>
+
+#include "plugin.hh"
+
+typedef std::list<struct plugin *> plugin_list;
+
+static plugin_list available;
+static plugin_list enabled;
+
+bool plugin_enable(const char *name)
+{
+ for (auto p = available.begin(); p != available.end(); p++) {
+ struct plugin *plugin = *p;
+ if (strcmp(plugin->name, name) == 0) {
+ enabled.push_back(plugin);
+ if (plugin->enable) {
+ plugin->enable(plugin);
+ }
+ return true;
+ }
+ }
+ return false;
+}
+
+void plugin_process(const struct record_info *ri)
+{
+ for (auto p = enabled.begin(); p != enabled.end(); p++) {
+ struct plugin *plugin = *p;
+ if (plugin->process) {
+ plugin->process(plugin, ri);
+ }
+ }
+}
+
+void plugin_summary(void)
+{
+ for (auto p = enabled.begin(); p != enabled.end(); p++) {
+ struct plugin *plugin = *p;
+ if (plugin->summary) {
+ printf("Summary for %s plugin:\n", plugin->name);
+ plugin->summary(plugin);
+ }
+ }
+}
+
+static void plugin_add(struct plugin *plugin)
+{
+ available.push_back(plugin);
+}
+
+void plugin_init(void)
+{
+ extern struct plugin *__start_plugin;
+ extern struct plugin *__stop_plugin;
+ struct plugin **p;
+
+ for (p = &__start_plugin; p < &__stop_plugin; p++) {
+ plugin_add(*p);
+ }
+}
+
+void plugin_process_wrapper(struct plugin *plugin, const struct record_info
*ri)
+{
+ xenalyze_plugin *p = static_cast<xenalyze_plugin*>(plugin->data);
+ p->process(ri);
+}
+
+void plugin_summary_wrapper(struct plugin *plugin)
+{
+ xenalyze_plugin *p = static_cast<xenalyze_plugin*>(plugin->data);
+ p->summary();
+ delete p;
+}
diff --git a/plugin.h b/plugin.h
new file mode 100644
--- /dev/null
+++ b/plugin.h
@@ -0,0 +1,47 @@
+/*
+ * Xenalyze plugin C API.
+ *
+ * Copyright (C) 2012, Citrix Systems R&D Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#ifndef PLUGIN_H
+#define PLUGIN_H
+
+#include <stdbool.h>
+
+#include "analyze.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct plugin;
+
+typedef void (*plugin_enable_f)(struct plugin *plugin);
+typedef void (*plugin_process_f)(struct plugin *plugin, const struct
record_info *ri);
+typedef void (*plugin_summary_f)(struct plugin *plugin);
+
+struct plugin {
+ const char *name;
+ plugin_enable_f enable;
+ plugin_process_f process;
+ plugin_summary_f summary;
+ void *data;
+};
+
+#define DEFINE_PLUGIN(p) \
+ struct plugin *__plugin_ ## p __attribute__((section("plugin")))
= &p
+
+void plugin_init(void);
+bool plugin_enable(const char *name);
+void plugin_process(const struct record_info *ri);
+void plugin_summary(void);
+
+#ifdef __cplusplus
+} /* extern "C" */
+#endif
+
+#endif /* #ifndef PLUGIN_H */
diff --git a/plugin.hh b/plugin.hh
new file mode 100644
--- /dev/null
+++ b/plugin.hh
@@ -0,0 +1,42 @@
+/*
+ * Xenalyze plugin C++ API.
+ *
+ * Copyright (C) 2012, Citrix Systems R&D Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#ifndef PLUGIN_HH
+#define PLUGIN_HH
+
+#include "plugin.h"
+
+class xenalyze_plugin {
+public:
+ virtual ~xenalyze_plugin() {}
+
+ virtual void process(const struct record_info *ri) = 0;
+ virtual void summary() = 0;
+};
+
+#define DEFINE_CXX_PLUGIN(name, cls) \
+ static void __plugin_ ## cls ## _enable(struct plugin *plugin) \
+ { \
+ plugin->data = new cls(); \
+ } \
+ \
+ static struct plugin __plugin ## cls = { \
+ name, \
+ __plugin_ ## cls ## _enable, \
+ plugin_process_wrapper, \
+ plugin_summary_wrapper, \
+ }; \
+ DEFINE_PLUGIN(__plugin ## cls)
+
+extern "C" {
+void plugin_process_wrapper(struct plugin *plugin, const struct record_info
*ri);
+void plugin_summary_wrapper(struct plugin *plugin);
+}
+
+#endif /* #ifndef PLUGIN_HH */
diff --git a/plugins/skeleton.cc b/plugins/skeleton.cc
new file mode 100644
--- /dev/null
+++ b/plugins/skeleton.cc
@@ -0,0 +1,31 @@
+/*
+ * Skeleton xenalyze plugin.
+ *
+ * Copyright (C) 2012, Citrix Systems R&D Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#include "plugin.hh"
+
+class skeleton_plugin : xenalyze_plugin {
+public:
+ skeleton_plugin() {}
+ ~skeleton_plugin() {}
+
+ void process(const struct record_info *ri);
+ void summary(void);
+};
+
+void skeleton_plugin::process(const struct record_info *ri)
+{
+ /* Put per-trace record stuff here. */
+}
+
+void skeleton_plugin::summary(void)
+{
+ /* Print a summary of the results (if applicable). */
+}
+
+DEFINE_CXX_PLUGIN("skeleton", skeleton_plugin);
diff --git a/xenalyze.c b/xenalyze.c
--- a/xenalyze.c
+++ b/xenalyze.c
@@ -32,6 +32,7 @@
#include "trace.h"
#include "analyze.h"
#include "mread.h"
+#include "plugin.h"
#include "pv.h"
#include <errno.h>
#include <strings.h>
@@ -8763,6 +8764,8 @@ void process_record(struct pcpu_info *p)
default:
process_generic(ri);
}
+
+ plugin_process(ri);
}
UPDATE_VOLUME(p, toplevel[toplevel], ri->size);
@@ -9346,6 +9349,7 @@ enum {
OPT_DUMP_ALL=''a'',
OPT_INTERVAL_LENGTH=''i'',
OPT_SUMMARY=''s'',
+ OPT_PLUGIN=''p'',
};
enum {
@@ -9816,6 +9820,15 @@ error_t cmd_parser(int key, char *arg, s
opt.tsc_loop_fatal = 1;
break;
+ case OPT_PLUGIN:
+ if (plugin_enable(arg)) {
+ G.output_defined = 1;
+ } else {
+ fprintf(stderr, "ERROR: No such plugin `%s''.\n",
arg);
+ exit(1);
+ }
+ break;
+
case ARGP_KEY_ARG:
{
/* FIXME - strcpy */
@@ -10108,6 +10121,10 @@ const struct argp_option cmd_opts[] = {
.arg = "errlevel",
.doc = "Sets tolerance for errors found in the file. Default is 3;
max is 6.", },
+ { .name = "plugin",
+ .key = OPT_PLUGIN,
+ .arg = "PLUGIN",
+ .doc = "Enable a decoder or summary plugin.", },
{ 0 },
};
@@ -10127,6 +10144,8 @@ int main(int argc, char *argv[]) {
/* Start with warn at stderr. */
warn = stderr;
+ plugin_init();
+
argp_parse(&parser_def, argc, argv, 0, NULL, NULL);
if (G.trace_file == NULL)
@@ -10163,6 +10182,8 @@ int main(int argc, char *argv[]) {
if(opt.summary)
summary();
+ plugin_summary();
+
if(opt.report_pcpu)
report_pcpu();
The batch-size plugin produces a histogram of the different counts in
some of the hypercalls (multical and mmu_update). This can be useful
to see if different guest kernels are less efficient at batching
hypercalls.
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,7 @@ xenalyze_OBJS := \
mread.o \
plugin.o \
xenalyze.o \
+ plugins/batch-size.o \
plugins/skeleton.o
dump-raw_OBJS := \
diff --git a/plugins/batch-size.cc b/plugins/batch-size.cc
new file mode 100644
--- /dev/null
+++ b/plugins/batch-size.cc
@@ -0,0 +1,73 @@
+/*
+ * Hypercall batch size xenalyze plugin.
+ *
+ * Copyright (C) 2012, Citrix Systems R&D Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This plugin produces a histogram of the batch sizes of some of the
+ * hypercalls. Specifically:
+ *
+ * - number of calls in a multicall.
+ * - number of updates in a mmu_update.
+ */
+#include <stdio.h>
+#include <map>
+
+#include "plugin.hh"
+#include "pv.h"
+#include "trace.h"
+
+class batch_size_plugin : xenalyze_plugin {
+public:
+ batch_size_plugin() {}
+ ~batch_size_plugin() {}
+
+ void process(const struct record_info *ri);
+ void summary();
+
+private:
+ std::map<int, int> multicall;
+ std::map<int, int> mmu_update;
+};
+
+void batch_size_plugin::process(const struct record_info *ri)
+{
+ uint32_t count;
+
+ if (ri->event == TRC_PV_HYPERCALL_V2) {
+ switch (pv_hypercall_op(ri)) {
+ case 1: /* mmu_update */
+ count = pv_hypercall_arg(ri, 1) & ~MMU_UPDATE_PREEMPTED;
+ mmu_update[count]++;
+ break;
+ case 13: /* multicall */
+ count = pv_hypercall_arg(ri, 1);
+ multicall[count]++;
+ break;
+ }
+ }
+}
+
+static void summarize(const char *name, std::map<int, int> &call)
+{
+ int calls = 0, batches = 0;
+
+ printf(" %s:\n Size Count\n", name);
+ for (auto i = call.begin(); i != call.end(); i++) {
+ printf(" %-4d %d\n", i->first, i->second);
+ calls += i->first * i->second;
+ batches += i->second;
+ }
+ printf(" Average %.1f\n", (double)calls / (double)batches);
+}
+
+void batch_size_plugin::summary()
+{
+ summarize("multicall", multicall);
+ summarize("mmu_update", mmu_update);
+}
+
+DEFINE_CXX_PLUGIN("batch-size", batch_size_plugin);
George Dunlap
2012-Jun-06 17:00 UTC
Re: [PATCH 02 of 10] xenalyze: automatically generate dependencies
On Thu, May 31, 2012 at 12:16 PM, David Vrabel <david.vrabel@citrix.com> wrote:> Use gcc''s -MD and -MP options to automatically generate dependencies. > > Signed-off-by: David Vrabel <david.vrabel@citrix.com>Acked-by: George Dunlap <george.dunlap@eu.citrix.com>> > diff --git a/.hgignore b/.hgignore > --- a/.hgignore > +++ b/.hgignore > @@ -1,3 +1,4 @@ > +.*\.d > .*\.o > xenalyze > dump-raw > diff --git a/Makefile b/Makefile > --- a/Makefile > +++ b/Makefile > @@ -1,5 +1,3 @@ > -CC = gcc > - > CFLAGS += -g -O2 > CFLAGS += -fno-strict-aliasing > CFLAGS += -std=gnu99 > @@ -7,22 +5,35 @@ CFLAGS += -Wall -Wstrict-prototypes > CFLAGS += -Wno-unused-value > CFLAGS += -Wdeclaration-after-statement > > -CFLAGS += -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > +CFLAGS += -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > CFLAGS += -mno-tls-direct-seg-refs > -CFLAGS += -Werror > +CFLAGS += -Werror > > -BIN = xenalyze dump-raw > +BIN := xenalyze dump-raw > > -HDRS = trace.h analyze.h mread.h > +xenalyze_OBJS := \ > + mread.o \ > + xenalyze.o > + > +dump-raw_OBJS := \ > + dump-raw.o > > all: $(BIN) > > +xenalyze: $(xenalyze_OBJS) > + $(CC) $(LDFLAGS) -o $@ $^ > + > +dump-raw: $(dump-raw_OBJS) > + $(CC) $(LDFLAGS) -o $@ $^ > + > +%.o: %.c > + $(CC) $(CFLAGS) -MD -MP -c -o $@ $< > + > +all_objs := $(foreach b,$(BIN),$($(b)_OBJS)) > +all_deps := $(all_objs:.o=.d) > + > .PHONY: clean > clean: > - $(RM) *.a *.so *.o *.rpm $(BIN) $(LIBBIN) > + $(RM) $(BIN) $(all_objs) $(all_deps) > > -%: %.c $(HDRS) Makefile > - $(CC) $(CFLAGS) -o $@ $< > - > -xenalyze: xenalyze.o mread.o > - $(CC) $(CFLAGS) -o $@ $^ > +-include $(all_deps) > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
George Dunlap
2012-Jun-06 17:03 UTC
Re: [PATCH 03 of 10] xenalyze: remove decode of unused events
On Thu, May 31, 2012 at 12:16 PM, David Vrabel <david.vrabel@citrix.com> wrote:> The PV_UPDATE_VA_MAPPING event is not present in xen-unstable, nor is > it present in any of the 3.2, 3.3, 3.4 or 4.1 XenServer trees I looked > at so I can only assume this was an event that never made it upstream. > > Remove the code as the event ID is now used for PV_HYPERCALL_V2. > > Similarly, some of the the HW_IRQ_* events are also not used anywhere > I could find.Certainly makes sense to remove these, but I think the PV_UPDATE_VA_MAPPING at least is probably useful, if I can find the patch for it. (The HW_IRQ events were for tracking down that IOMMU bug reported by SolarFlare last summer.) I''ll make two changesets which remove these individually, so it''s easy to revert if we decide to bring them back. -George> > Signed-off-by: David Vrabel <david.vrabel@citrix.com> > --- > > diff --git a/xenalyze.c b/xenalyze.c > --- a/xenalyze.c > +++ b/xenalyze.c > @@ -1531,7 +1531,6 @@ enum { > PV_GDT_LDT_MAPPING_FAULT, > PV_PTWR_EMULATION, > PV_PTWR_EMULATION_PAE, > - PV_UPDATE_VA_MAPPING, > PV_MAX > }; > > @@ -6514,44 +6513,6 @@ void pv_ptwr_emulation_process(struct re > } > } > > -void pv_update_va_mapping_process(struct record_info *ri, struct pv_data *pv) { > - union pv_event pevt = { .event = ri->event }; > - union { > - /* gpl2 is deprecated */ > - struct { > - unsigned long long val; > - unsigned int va, flags; > - } x32; > - struct { > - unsigned long long val; > - unsigned long long va, flags; > - } x64; > - } *r = (typeof(r))ri->d; > - struct { > - unsigned long long val, va, flags; > - } e; > - > - if ( pevt.x64 ) > - { > - e.val = r->x64.val; > - e.va = r->x64.va; > - e.flags = r->x64.flags; > - } > - else > - { > - e.val = r->x32.val; > - e.va = r->x32.va; > - e.flags = r->x32.flags; > - } > - > - if ( opt.dump_all ) > - { > - printf(" %s update_va_mapping l1e %llx va %llx flags %llx\n", > - ri->dump_header, > - e.val, e.va, e.flags); > - } > -} > - > void pv_generic_process(struct record_info *ri, struct pv_data *pv) { > union pv_event pevt = { .event = ri->event }; > if ( opt.dump_all ) { > @@ -6638,9 +6599,6 @@ void pv_process(struct pcpu_info *p) > case PV_PTWR_EMULATION_PAE: > pv_ptwr_emulation_process(ri, pv); > break; > - case PV_UPDATE_VA_MAPPING: > - pv_update_va_mapping_process(ri, pv); > - break; > case PV_PAGE_FAULT: > //pv_pf_process(ri, pv); > //break; > @@ -7893,149 +7851,6 @@ void irq_process(struct pcpu_info *p) { > } > break; > } > - case TRC_HW_IRQ_MSI_WRITE: > - { > - struct { > - unsigned address_lo, address_hi; > - unsigned data; > - unsigned irq:16, pos:16; > - uint8_t func, slot, bus, type; > - unsigned mask_base; > - } *r = (typeof(r))ri->d; > - > - if ( opt.dump_all ) > - { > - printf(" %s irq_msi_write irq %x t %x base %x addr %x %x data %x pci %02x:%02x.%x %x\n", > - ri->dump_header, > - r->irq, > - r->type, > - r->mask_base, > - r->address_hi, r->address_lo, > - r->data, > - r->bus, r->slot, r->func, r->pos); > - } > - break; > - } > - case TRC_HW_IRQ_IOMMU_AMD_IRE: > - { > - struct { > - uint16_t bdf, id; > - int offset; > - uint8_t dest_mode, dev_mode, vector, dest; > - } *r = (typeof(r))ri->d; > - > - if ( opt.dump_all ) > - { > - printf(" %s irq_iommu_ire bdf %x id %x offset %x dest_mode %x dev_mode %x vec %x dest %x\n", > - ri->dump_header, > - r->bdf, r->id, > - r->offset, > - r->dest_mode, r->dev_mode, > - r->vector, r->dest); > - } > - break; > - } > - case TRC_HW_IRQ_MAP_PIRQ_MSI: > - { > - struct { > - unsigned domain:16, > - pirq:16, > - irq:16, > - bus:16, > - devfn:16, > - entry_nr:16; > - } *r = (typeof(r))ri->d; > - > - if ( r->irq < MAX_IRQ ) > - { > - struct irq_desc *irq=irq_table+r->irq; > - > - if ( irq->dev ) > - { > - fprintf(warn, "Strange, irq %d already has dev %02x:%x.%x!\n", > - r->irq, irq->dev->bus, > - irq->dev->devfn>>4, > - irq->dev->devfn&3); > - } > - else > - { > - struct pci_dev *pdev = pdev_find(r->bus, r->devfn); > - > - irq->dev=pdev; > - irq->type=IRQ_MSI; > - } > - } > - > - if ( opt.dump_all ) > - { > - printf(" %s irq_map_pirq_msi d%d pirq %x(%d) irq %x bus %x devfn %x entry %x\n", > - ri->dump_header, > - r->domain, > - r->pirq, > - r->pirq, > - r->irq, > - r->bus, > - r->devfn, > - r->entry_nr); > - } > - break; > - } > - case TRC_HW_IRQ_MAP_PIRQ_GSI: > - { > - struct { > - unsigned domain, pirq, irq; > - } *r = (typeof(r))ri->d; > - > - if ( opt.dump_all ) > - { > - printf(" %s irq_map_pirq_gsi d%d pirq %x(%d) irq %x\n", > - ri->dump_header, > - r->domain, > - r->pirq, > - r->pirq, > - r->irq); > - } > - break; > - } > - case TRC_HW_IRQ_MSI_SET_AFFINITY: > - { > - struct { > - unsigned irq, apic_id, vector; > - } *r = (typeof(r))ri->d; > - > - if ( opt.dump_all ) > - { > - printf(" %s irq_msi_set_affinity irq %x apicid %x vec %x\n", > - ri->dump_header, > - r->irq, > - r->apic_id, > - r->vector); > - } > - break; > - } > - case TRC_HW_IRQ_SET_DESC_AFFINITY: > - { > - struct { > - unsigned line:16, irq:16; > - char fname[24]; /* Extra 7 words; 6 words * 4 = 24 */ > - } *r = (typeof(r))ri->d; > - char fname[25]; > - int i; > - > - for(i=0; i<24; i++) > - fname[i]=r->fname[i]; > - fname[i]=0; > - > - if ( opt.dump_all ) > - { > - printf(" %s irq_set_desc_affinity irq %x %s:%d\n", > - ri->dump_header, > - r->irq, > - fname, > - r->line); > - } > - break; > - } > case TRC_HW_IRQ_CLEAR_VECTOR: > case TRC_HW_IRQ_MOVE_FINISH : > default: > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
George Dunlap
2012-Jun-07 10:14 UTC
Re: [PATCH 04 of 10] xenalyze: update trace.h to match xen-unstable
On 31/05/12 12:16, David Vrabel wrote:> Update trace.h to the version in xen-unstable (plus my PV_HYPERCALL_* > patches). > > Signed-off-by: David Vrabel<david.vrabel@citrix.com>Acked-by: George Dunlap <george.dunlap@eu.citrix.com>> --- > > diff --git a/trace.h b/trace.h > --- a/trace.h > +++ b/trace.h > @@ -57,6 +57,7 @@ > #define TRC_SCHED_CLASS 0x00022000 /* Scheduler-specific */ > #define TRC_SCHED_VERBOSE 0x00028000 /* More inclusive scheduling */ > > +/* Trace classes for Hardware */ > #define TRC_HW_PM 0x00801000 /* Power management traces */ > #define TRC_HW_IRQ 0x00802000 /* Traces relating to the handling of IRQs */ > > @@ -93,20 +94,51 @@ > #define TRC_MEM_POD_ZERO_RECLAIM (TRC_MEM + 17) > #define TRC_MEM_POD_SUPERPAGE_SPLINTER (TRC_MEM + 18) > > +#define TRC_PV_ENTRY 0x00201000 /* Hypervisor entry points for PV guests. */ > +#define TRC_PV_SUBCALL 0x00202000 /* Sub-call in a multicall hypercall */ > > -#define TRC_PV_HYPERCALL (TRC_PV + 1) > -#define TRC_PV_TRAP (TRC_PV + 3) > -#define TRC_PV_PAGE_FAULT (TRC_PV + 4) > -#define TRC_PV_FORCED_INVALID_OP (TRC_PV + 5) > -#define TRC_PV_EMULATE_PRIVOP (TRC_PV + 6) > -#define TRC_PV_EMULATE_4GB (TRC_PV + 7) > -#define TRC_PV_MATH_STATE_RESTORE (TRC_PV + 8) > -#define TRC_PV_PAGING_FIXUP (TRC_PV + 9) > -#define TRC_PV_GDT_LDT_MAPPING_FAULT (TRC_PV + 10) > -#define TRC_PV_PTWR_EMULATION (TRC_PV + 11) > -#define TRC_PV_PTWR_EMULATION_PAE (TRC_PV + 12) > - /* Indicates that addresses in trace record are 64 bits */ > -#define TRC_64_FLAG (0x100) > +#define TRC_PV_HYPERCALL (TRC_PV_ENTRY + 1) > +#define TRC_PV_TRAP (TRC_PV_ENTRY + 3) > +#define TRC_PV_PAGE_FAULT (TRC_PV_ENTRY + 4) > +#define TRC_PV_FORCED_INVALID_OP (TRC_PV_ENTRY + 5) > +#define TRC_PV_EMULATE_PRIVOP (TRC_PV_ENTRY + 6) > +#define TRC_PV_EMULATE_4GB (TRC_PV_ENTRY + 7) > +#define TRC_PV_MATH_STATE_RESTORE (TRC_PV_ENTRY + 8) > +#define TRC_PV_PAGING_FIXUP (TRC_PV_ENTRY + 9) > +#define TRC_PV_GDT_LDT_MAPPING_FAULT (TRC_PV_ENTRY + 10) > +#define TRC_PV_PTWR_EMULATION (TRC_PV_ENTRY + 11) > +#define TRC_PV_PTWR_EMULATION_PAE (TRC_PV_ENTRY + 12) > +#define TRC_PV_HYPERCALL_V2 (TRC_PV_ENTRY + 13) > +#define TRC_PV_HYPERCALL_SUBCALL (TRC_PV_SUBCALL + 14) > + > +/* > + * TRC_PV_HYPERCALL_V2 format > + * > + * Only some of the hypercall argument are recorded. Bit fields A0 to > + * A5 in the first extra word are set if the argument is present and > + * the arguments themselves are packed sequentially in the following > + * words. > + * > + * The TRC_64_FLAG bit is not set for these events (even if there are > + * 64-bit arguments in the record). > + * > + * Word > + * 0 bit 31 30|29 28|27 26|25 24|23 22|21 20|19 ... 0 > + * A5 |A4 |A3 |A2 |A1 |A0 |Hypercall op > + * 1 First 32 bit (or low word of first 64 bit) arg in record > + * 2 Second 32 bit (or high word of first 64 bit) arg in record > + * ... > + * > + * A0-A5 bitfield values: > + * > + * 00b Argument not present > + * 01b 32-bit argument present > + * 10b 64-bit argument present > + * 11b Reserved > + */ > +#define TRC_PV_HYPERCALL_V2_ARG_32(i) (0x1<< (20 + 2*(i))) > +#define TRC_PV_HYPERCALL_V2_ARG_64(i) (0x2<< (20 + 2*(i))) > +#define TRC_PV_HYPERCALL_V2_ARG_MASK (0xfff00000) > > #define TRC_SHADOW_NOT_SHADOW (TRC_SHADOW + 1) > #define TRC_SHADOW_FAST_PROPAGATE (TRC_SHADOW + 2) > @@ -125,6 +157,7 @@ > #define TRC_SHADOW_RESYNC_ONLY (TRC_SHADOW + 15) > > /* trace events per subclass */ > +#define TRC_HVM_NESTEDFLAG (0x400) > #define TRC_HVM_VMENTRY (TRC_HVM_ENTRYEXIT + 0x01) > #define TRC_HVM_VMEXIT (TRC_HVM_ENTRYEXIT + 0x02) > #define TRC_HVM_VMEXIT64 (TRC_HVM_ENTRYEXIT + TRC_64_FLAG + 0x02) > @@ -165,6 +198,7 @@ > #define TRC_HVM_REALMODE_EMULATE (TRC_HVM_HANDLER + 0x22) > #define TRC_HVM_TRAP (TRC_HVM_HANDLER + 0x23) > #define TRC_HVM_TRAP_DEBUG (TRC_HVM_HANDLER + 0x24) > +#define TRC_HVM_VLAPIC (TRC_HVM_HANDLER + 0x25) > > #define TRC_HVM_IOPORT_WRITE (TRC_HVM_HANDLER + 0x216) > #define TRC_HVM_IOMEM_WRITE (TRC_HVM_HANDLER + 0x217) > @@ -172,8 +206,9 @@ > /* trace events for per class */ > #define TRC_PM_FREQ_CHANGE (TRC_HW_PM + 0x01) > #define TRC_PM_IDLE_ENTRY (TRC_HW_PM + 0x02) > -#define TRC_PM_IDLE_EXIT (TRC_HW_PM + 0x03) > +#define TRC_PM_IDLE_EXIT (TRC_HW_PM + 0x03) > > +/* Trace events for IRQs */ > #define TRC_HW_IRQ_MOVE_CLEANUP_DELAY (TRC_HW_IRQ + 0x1) > #define TRC_HW_IRQ_MOVE_CLEANUP (TRC_HW_IRQ + 0x2) > #define TRC_HW_IRQ_BIND_VECTOR (TRC_HW_IRQ + 0x3) > @@ -182,12 +217,15 @@ > #define TRC_HW_IRQ_ASSIGN_VECTOR (TRC_HW_IRQ + 0x6) > #define TRC_HW_IRQ_UNMAPPED_VECTOR (TRC_HW_IRQ + 0x7) > #define TRC_HW_IRQ_HANDLED (TRC_HW_IRQ + 0x8) > -#define TRC_HW_IRQ_MSI_WRITE (TRC_HW_IRQ + 0x9) > -#define TRC_HW_IRQ_MAP_PIRQ_MSI (TRC_HW_IRQ + 0xa) > -#define TRC_HW_IRQ_MAP_PIRQ_GSI (TRC_HW_IRQ + 0xb) > -#define TRC_HW_IRQ_MSI_SET_AFFINITY (TRC_HW_IRQ + 0x10) > -#define TRC_HW_IRQ_SET_DESC_AFFINITY (TRC_HW_IRQ + 0x11) > -#define TRC_HW_IRQ_IOMMU_AMD_IRE (TRC_HW_IRQ + 0x12) > + > +/* > + * Event Flags > + * > + * Some events (e.g, TRC_PV_TRAP and TRC_HVM_IOMEM_READ) have multiple > + * record formats. These event flags distinguish between the > + * different formats. > + */ > +#define TRC_64_FLAG 0x100 /* Addresses are 64 bits (instead of 32 bits) */ > > /* This structure represents a single trace buffer record. */ > struct t_rec { > @@ -205,6 +243,34 @@ struct t_rec { > } u; > }; > > +/* > + * This structure contains the metadata for a single trace buffer. The head > + * field, indexes into an array of struct t_rec''s. > + */ > +struct t_buf { > + /* Assume the data buffer size is X. X is generally not a power of 2. > + * CONS and PROD are incremented modulo (2*X): > + * 0<= cons< 2*X > + * 0<= prod< 2*X > + * This is done because addition modulo X breaks at 2^32 when X is not a > + * power of 2: > + * (((2^32 - 1) % X) + 1) % X != (2^32) % X > + */ > + uint32_t cons; /* Offset of next item to be consumed by control tools. */ > + uint32_t prod; /* Offset of next item to be produced by Xen. */ > + /* Records follow immediately after the meta-data header. */ > +}; > + > +/* Structure used to pass MFNs to the trace buffers back to trace consumers. > + * Offset is an offset into the mapped structure where the mfn list will be held. > + * MFNs will be at ((unsigned long *)(t_info))+(t_info->cpu_offset[cpu]). > + */ > +struct t_info { > + uint16_t tbuf_size; /* Size in pages of each trace buffer */ > + uint16_t mfn_offset[]; /* Offset within t_info structure of the page list per cpu */ > + /* MFN lists immediately after the header */ > +}; > + > #endif /* __XEN_PUBLIC_TRACE_H__ */ > > /*
George Dunlap
2012-Jun-07 10:15 UTC
Re: [PATCH 04 of 10] xenalyze: update trace.h to match xen-unstable
On 31/05/12 12:16, David Vrabel wrote:> Update trace.h to the version in xen-unstable (plus my PV_HYPERCALL_* > patches). > > Signed-off-by: David Vrabel<david.vrabel@citrix.com>Acked-by: George Dunlap <george.dunlap@eu.citrix.com>> --- > > diff --git a/trace.h b/trace.h > --- a/trace.h > +++ b/trace.h > @@ -57,6 +57,7 @@ > #define TRC_SCHED_CLASS 0x00022000 /* Scheduler-specific */ > #define TRC_SCHED_VERBOSE 0x00028000 /* More inclusive scheduling */ > > +/* Trace classes for Hardware */ > #define TRC_HW_PM 0x00801000 /* Power management traces */ > #define TRC_HW_IRQ 0x00802000 /* Traces relating to the handling of IRQs */ > > @@ -93,20 +94,51 @@ > #define TRC_MEM_POD_ZERO_RECLAIM (TRC_MEM + 17) > #define TRC_MEM_POD_SUPERPAGE_SPLINTER (TRC_MEM + 18) > > +#define TRC_PV_ENTRY 0x00201000 /* Hypervisor entry points for PV guests. */ > +#define TRC_PV_SUBCALL 0x00202000 /* Sub-call in a multicall hypercall */ > > -#define TRC_PV_HYPERCALL (TRC_PV + 1) > -#define TRC_PV_TRAP (TRC_PV + 3) > -#define TRC_PV_PAGE_FAULT (TRC_PV + 4) > -#define TRC_PV_FORCED_INVALID_OP (TRC_PV + 5) > -#define TRC_PV_EMULATE_PRIVOP (TRC_PV + 6) > -#define TRC_PV_EMULATE_4GB (TRC_PV + 7) > -#define TRC_PV_MATH_STATE_RESTORE (TRC_PV + 8) > -#define TRC_PV_PAGING_FIXUP (TRC_PV + 9) > -#define TRC_PV_GDT_LDT_MAPPING_FAULT (TRC_PV + 10) > -#define TRC_PV_PTWR_EMULATION (TRC_PV + 11) > -#define TRC_PV_PTWR_EMULATION_PAE (TRC_PV + 12) > - /* Indicates that addresses in trace record are 64 bits */ > -#define TRC_64_FLAG (0x100) > +#define TRC_PV_HYPERCALL (TRC_PV_ENTRY + 1) > +#define TRC_PV_TRAP (TRC_PV_ENTRY + 3) > +#define TRC_PV_PAGE_FAULT (TRC_PV_ENTRY + 4) > +#define TRC_PV_FORCED_INVALID_OP (TRC_PV_ENTRY + 5) > +#define TRC_PV_EMULATE_PRIVOP (TRC_PV_ENTRY + 6) > +#define TRC_PV_EMULATE_4GB (TRC_PV_ENTRY + 7) > +#define TRC_PV_MATH_STATE_RESTORE (TRC_PV_ENTRY + 8) > +#define TRC_PV_PAGING_FIXUP (TRC_PV_ENTRY + 9) > +#define TRC_PV_GDT_LDT_MAPPING_FAULT (TRC_PV_ENTRY + 10) > +#define TRC_PV_PTWR_EMULATION (TRC_PV_ENTRY + 11) > +#define TRC_PV_PTWR_EMULATION_PAE (TRC_PV_ENTRY + 12) > +#define TRC_PV_HYPERCALL_V2 (TRC_PV_ENTRY + 13) > +#define TRC_PV_HYPERCALL_SUBCALL (TRC_PV_SUBCALL + 14) > + > +/* > + * TRC_PV_HYPERCALL_V2 format > + * > + * Only some of the hypercall argument are recorded. Bit fields A0 to > + * A5 in the first extra word are set if the argument is present and > + * the arguments themselves are packed sequentially in the following > + * words. > + * > + * The TRC_64_FLAG bit is not set for these events (even if there are > + * 64-bit arguments in the record). > + * > + * Word > + * 0 bit 31 30|29 28|27 26|25 24|23 22|21 20|19 ... 0 > + * A5 |A4 |A3 |A2 |A1 |A0 |Hypercall op > + * 1 First 32 bit (or low word of first 64 bit) arg in record > + * 2 Second 32 bit (or high word of first 64 bit) arg in record > + * ... > + * > + * A0-A5 bitfield values: > + * > + * 00b Argument not present > + * 01b 32-bit argument present > + * 10b 64-bit argument present > + * 11b Reserved > + */ > +#define TRC_PV_HYPERCALL_V2_ARG_32(i) (0x1<< (20 + 2*(i))) > +#define TRC_PV_HYPERCALL_V2_ARG_64(i) (0x2<< (20 + 2*(i))) > +#define TRC_PV_HYPERCALL_V2_ARG_MASK (0xfff00000) > > #define TRC_SHADOW_NOT_SHADOW (TRC_SHADOW + 1) > #define TRC_SHADOW_FAST_PROPAGATE (TRC_SHADOW + 2) > @@ -125,6 +157,7 @@ > #define TRC_SHADOW_RESYNC_ONLY (TRC_SHADOW + 15) > > /* trace events per subclass */ > +#define TRC_HVM_NESTEDFLAG (0x400) > #define TRC_HVM_VMENTRY (TRC_HVM_ENTRYEXIT + 0x01) > #define TRC_HVM_VMEXIT (TRC_HVM_ENTRYEXIT + 0x02) > #define TRC_HVM_VMEXIT64 (TRC_HVM_ENTRYEXIT + TRC_64_FLAG + 0x02) > @@ -165,6 +198,7 @@ > #define TRC_HVM_REALMODE_EMULATE (TRC_HVM_HANDLER + 0x22) > #define TRC_HVM_TRAP (TRC_HVM_HANDLER + 0x23) > #define TRC_HVM_TRAP_DEBUG (TRC_HVM_HANDLER + 0x24) > +#define TRC_HVM_VLAPIC (TRC_HVM_HANDLER + 0x25) > > #define TRC_HVM_IOPORT_WRITE (TRC_HVM_HANDLER + 0x216) > #define TRC_HVM_IOMEM_WRITE (TRC_HVM_HANDLER + 0x217) > @@ -172,8 +206,9 @@ > /* trace events for per class */ > #define TRC_PM_FREQ_CHANGE (TRC_HW_PM + 0x01) > #define TRC_PM_IDLE_ENTRY (TRC_HW_PM + 0x02) > -#define TRC_PM_IDLE_EXIT (TRC_HW_PM + 0x03) > +#define TRC_PM_IDLE_EXIT (TRC_HW_PM + 0x03) > > +/* Trace events for IRQs */ > #define TRC_HW_IRQ_MOVE_CLEANUP_DELAY (TRC_HW_IRQ + 0x1) > #define TRC_HW_IRQ_MOVE_CLEANUP (TRC_HW_IRQ + 0x2) > #define TRC_HW_IRQ_BIND_VECTOR (TRC_HW_IRQ + 0x3) > @@ -182,12 +217,15 @@ > #define TRC_HW_IRQ_ASSIGN_VECTOR (TRC_HW_IRQ + 0x6) > #define TRC_HW_IRQ_UNMAPPED_VECTOR (TRC_HW_IRQ + 0x7) > #define TRC_HW_IRQ_HANDLED (TRC_HW_IRQ + 0x8) > -#define TRC_HW_IRQ_MSI_WRITE (TRC_HW_IRQ + 0x9) > -#define TRC_HW_IRQ_MAP_PIRQ_MSI (TRC_HW_IRQ + 0xa) > -#define TRC_HW_IRQ_MAP_PIRQ_GSI (TRC_HW_IRQ + 0xb) > -#define TRC_HW_IRQ_MSI_SET_AFFINITY (TRC_HW_IRQ + 0x10) > -#define TRC_HW_IRQ_SET_DESC_AFFINITY (TRC_HW_IRQ + 0x11) > -#define TRC_HW_IRQ_IOMMU_AMD_IRE (TRC_HW_IRQ + 0x12) > + > +/* > + * Event Flags > + * > + * Some events (e.g, TRC_PV_TRAP and TRC_HVM_IOMEM_READ) have multiple > + * record formats. These event flags distinguish between the > + * different formats. > + */ > +#define TRC_64_FLAG 0x100 /* Addresses are 64 bits (instead of 32 bits) */ > > /* This structure represents a single trace buffer record. */ > struct t_rec { > @@ -205,6 +243,34 @@ struct t_rec { > } u; > }; > > +/* > + * This structure contains the metadata for a single trace buffer. The head > + * field, indexes into an array of struct t_rec''s. > + */ > +struct t_buf { > + /* Assume the data buffer size is X. X is generally not a power of 2. > + * CONS and PROD are incremented modulo (2*X): > + * 0<= cons< 2*X > + * 0<= prod< 2*X > + * This is done because addition modulo X breaks at 2^32 when X is not a > + * power of 2: > + * (((2^32 - 1) % X) + 1) % X != (2^32) % X > + */ > + uint32_t cons; /* Offset of next item to be consumed by control tools. */ > + uint32_t prod; /* Offset of next item to be produced by Xen. */ > + /* Records follow immediately after the meta-data header. */ > +}; > + > +/* Structure used to pass MFNs to the trace buffers back to trace consumers. > + * Offset is an offset into the mapped structure where the mfn list will be held. > + * MFNs will be at ((unsigned long *)(t_info))+(t_info->cpu_offset[cpu]). > + */ > +struct t_info { > + uint16_t tbuf_size; /* Size in pages of each trace buffer */ > + uint16_t mfn_offset[]; /* Offset within t_info structure of the page list per cpu */ > + /* MFN lists immediately after the header */ > +}; > + > #endif /* __XEN_PUBLIC_TRACE_H__ */ > > /*
George Dunlap
2012-Jun-07 10:16 UTC
Re: [PATCH 05 of 10] xenalyze: correctly display of count of HW events
On 31/05/12 12:16, David Vrabel wrote:> HW events where being shown under "(null)" in the summary. > > Signed-off-by: David Vrabel<david.vrabel@citrix.com>Acked-by: George Dunlap <george.dunlap@eu.citrix.com>> > diff --git a/xenalyze.c b/xenalyze.c > --- a/xenalyze.c > +++ b/xenalyze.c > @@ -1805,7 +1805,8 @@ enum { > TOPLEVEL_MEM, > TOPLEVEL_PV, > TOPLEVEL_SHADOW, > - TOPLEVEL_MAX=12 > + TOPLEVEL_HW, > + TOPLEVEL_MAX=TOPLEVEL_HW+1, > }; > > char * toplevel_name[TOPLEVEL_MAX] = { > @@ -1816,6 +1817,7 @@ char * toplevel_name[TOPLEVEL_MAX] = { > [TOPLEVEL_MEM]="mem", > [TOPLEVEL_PV]="pv", > [TOPLEVEL_SHADOW]="shadow", > + [TOPLEVEL_HW]="hw", > }; > > struct trace_volume { > @@ -8595,7 +8597,6 @@ void process_cpu_change(struct pcpu_info > } > } > > -#define TOPLEVEL_MAX 16 > struct tl_assert_mask { > unsigned p_current:1, > not_idle_domain:1;
George Dunlap
2012-Jun-07 11:05 UTC
Re: [PATCH 09 of 10] xenalyze: add a basic plugin infrastructure
On 31/05/12 12:16, David Vrabel wrote:> Allow xenalyze to be include (at build time) plugins that can do > per-record actions and a summary. These plugins can be in C or C++. > > The plugins entry points are in struct plugin and pointers to all the > plugins linked in xenalyze are placed in a "plugin" section so > plugin_init() can find them all. > > A new command line option (-p, --plugin=PLUGIN) is added to enable one > or more plugins. > > A sample plugin (skeleton) is included (mostly because at least one > plugin must be present for the build to work). > > Signed-off-by: David Vrabel<david.vrabel@citrix.com>So what''s the main motivation of having this plugin infrastructure? The one plugin example you have ("batch-size", patch 10) seems simple enough that it should be fairly straightforward to just add as an option, with not much more boilerplate than C++ already requires. Looks like potential advantages may include: * Ability to use C++ language (for those who care for such things) * Ability to use STL for complex data structures * Ability to add an option like the "batch-size" plugin in a concise, self-contained way Potential disadvantages include: * An extra O(N) loop on the hot path (where N = # of enabled plugins) * For each enabled plugin, an extra full function call on the hot path; and a C++ function at that, which (my prejudice tells me) is likely to be more wasteful time and space-wise than a C function. Did I miss anything? Obviosuly thanks for being conscious of this cost by, for example, having a separate list for "enabled" vs "available". I guess I want to be convinced that allowing this kind of plug-in is really worth it, and won''t cost too much; especially when something like "batch-size" could be implemented in a pretty straightforward way without needing a plugin. I suppose that if there''s a plugin that turns out to be useful, but is expensive when run as a plug-in, we could always pull it into the main file as an optimization. Also, could you do a simple test to measure the overhead of having no plugins? Just finding a 700MB-ish file you have lying around and running "xenalyze -s" with and with this patch (and with this patch and -p skeleton or -p batch-size) would be pretty informative, and shouldn''t take too long. Thanks, -George> --- > Makefile | 10 +++--- > analyze.h | 55 ++++++++++++++++++++++++++++++++++ > plugin.cc | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > plugin.h | 48 +++++++++++++++++++++++++++++ > plugin.hh | 42 ++++++++++++++++++++++++++ > plugins/skeleton.cc | 31 +++++++++++++++++++ > xenalyze.c | 72 +++++++++++++------------------------------- > 7 files changed, 287 insertions(+), 55 deletions(-) > > diff --git a/Makefile b/Makefile > --- a/Makefile > +++ b/Makefile > @@ -1,4 +1,4 @@ > -CFLAGS += -g -O2 > +CFLAGS += -g -O2 -I. > CFLAGS += -fno-strict-aliasing > CFLAGS += -std=gnu99 > CFLAGS += -Wall -Wstrict-prototypes > @@ -9,11 +9,15 @@ CFLAGS += -D_LARGEFILE_SOURCE -D_LARGEFI > CFLAGS += -mno-tls-direct-seg-refs > CFLAGS += -Werror > > +CXXFLAGS := -g -O2 -I. -Wall -Werror -std=c++0x > + > BIN := xenalyze dump-raw > > xenalyze_OBJS := \ > mread.o \ > - xenalyze.o > + plugin.o \ > + xenalyze.o \ > + plugins/skeleton.o > > dump-raw_OBJS := \ > dump-raw.o > @@ -21,7 +25,7 @@ dump-raw_OBJS := \ > all: $(BIN) > > xenalyze: $(xenalyze_OBJS) > - $(CC) $(LDFLAGS) -o $@ $^ > + $(CXX) $(LDFLAGS) -o $@ $^ > > dump-raw: $(dump-raw_OBJS) > $(CC) $(LDFLAGS) -o $@ $^ > @@ -29,6 +33,9 @@ dump-raw: $(dump-raw_OBJS) > %.o: %.c > $(CC) $(CFLAGS) -MD -MP -c -o $@ $< > > +%.o: %.cc > + $(CXX) $(CXXFLAGS) -MD -MP -c -o $@ $< > + > all_objs := $(foreach b,$(BIN),$($(b)_OBJS)) > all_deps := $(all_objs:.o=.d) > > diff --git a/plugin.cc b/plugin.cc > new file mode 100644 > --- /dev/null > +++ b/plugin.cc > @@ -0,0 +1,84 @@ > +/* > + * Xenalyze plugin infrastructure. > + * > + * Copyright (C) 2012, Citrix Systems R&D Ltd > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + */ > +#include<stdio.h> > +#include<string.h> > +#include<list> > + > +#include "plugin.hh" > + > +typedef std::list<struct plugin *> plugin_list; > + > +static plugin_list available; > +static plugin_list enabled; > + > +bool plugin_enable(const char *name) > +{ > + for (auto p = available.begin(); p != available.end(); p++) { > + struct plugin *plugin = *p; > + if (strcmp(plugin->name, name) == 0) { > + enabled.push_back(plugin); > + if (plugin->enable) { > + plugin->enable(plugin); > + } > + return true; > + } > + } > + return false; > +} > + > +void plugin_process(const struct record_info *ri) > +{ > + for (auto p = enabled.begin(); p != enabled.end(); p++) { > + struct plugin *plugin = *p; > + if (plugin->process) { > + plugin->process(plugin, ri); > + } > + } > +} > + > +void plugin_summary(void) > +{ > + for (auto p = enabled.begin(); p != enabled.end(); p++) { > + struct plugin *plugin = *p; > + if (plugin->summary) { > + printf("Summary for %s plugin:\n", plugin->name); > + plugin->summary(plugin); > + } > + } > +} > + > +static void plugin_add(struct plugin *plugin) > +{ > + available.push_back(plugin); > +} > + > +void plugin_init(void) > +{ > + extern struct plugin *__start_plugin; > + extern struct plugin *__stop_plugin; > + struct plugin **p; > + > + for (p =&__start_plugin; p< &__stop_plugin; p++) { > + plugin_add(*p); > + } > +} > + > +void plugin_process_wrapper(struct plugin *plugin, const struct record_info *ri) > +{ > + xenalyze_plugin *p = static_cast<xenalyze_plugin*>(plugin->data); > + p->process(ri); > +} > + > +void plugin_summary_wrapper(struct plugin *plugin) > +{ > + xenalyze_plugin *p = static_cast<xenalyze_plugin*>(plugin->data); > + p->summary(); > + delete p; > +} > diff --git a/plugin.h b/plugin.h > new file mode 100644 > --- /dev/null > +++ b/plugin.h > @@ -0,0 +1,47 @@ > +/* > + * Xenalyze plugin C API. > + * > + * Copyright (C) 2012, Citrix Systems R&D Ltd > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + */ > +#ifndef PLUGIN_H > +#define PLUGIN_H > + > +#include<stdbool.h> > + > +#include "analyze.h" > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +struct plugin; > + > +typedef void (*plugin_enable_f)(struct plugin *plugin); > +typedef void (*plugin_process_f)(struct plugin *plugin, const struct record_info *ri); > +typedef void (*plugin_summary_f)(struct plugin *plugin); > + > +struct plugin { > + const char *name; > + plugin_enable_f enable; > + plugin_process_f process; > + plugin_summary_f summary; > + void *data; > +}; > + > +#define DEFINE_PLUGIN(p) \ > + struct plugin *__plugin_ ## p __attribute__((section("plugin"))) =&p > + > +void plugin_init(void); > +bool plugin_enable(const char *name); > +void plugin_process(const struct record_info *ri); > +void plugin_summary(void); > + > +#ifdef __cplusplus > +} /* extern "C" */ > +#endif > + > +#endif /* #ifndef PLUGIN_H */ > diff --git a/plugin.hh b/plugin.hh > new file mode 100644 > --- /dev/null > +++ b/plugin.hh > @@ -0,0 +1,42 @@ > +/* > + * Xenalyze plugin C++ API. > + * > + * Copyright (C) 2012, Citrix Systems R&D Ltd > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + */ > +#ifndef PLUGIN_HH > +#define PLUGIN_HH > + > +#include "plugin.h" > + > +class xenalyze_plugin { > +public: > + virtual ~xenalyze_plugin() {} > + > + virtual void process(const struct record_info *ri) = 0; > + virtual void summary() = 0; > +}; > + > +#define DEFINE_CXX_PLUGIN(name, cls) \ > + static void __plugin_ ## cls ## _enable(struct plugin *plugin) \ > + { \ > + plugin->data = new cls(); \ > + } \ > + \ > + static struct plugin __plugin ## cls = { \ > + name, \ > + __plugin_ ## cls ## _enable, \ > + plugin_process_wrapper, \ > + plugin_summary_wrapper, \ > + }; \ > + DEFINE_PLUGIN(__plugin ## cls) > + > +extern "C" { > +void plugin_process_wrapper(struct plugin *plugin, const struct record_info *ri); > +void plugin_summary_wrapper(struct plugin *plugin); > +} > + > +#endif /* #ifndef PLUGIN_HH */ > diff --git a/plugins/skeleton.cc b/plugins/skeleton.cc > new file mode 100644 > --- /dev/null > +++ b/plugins/skeleton.cc > @@ -0,0 +1,31 @@ > +/* > + * Skeleton xenalyze plugin. > + * > + * Copyright (C) 2012, Citrix Systems R&D Ltd > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + */ > +#include "plugin.hh" > + > +class skeleton_plugin : xenalyze_plugin { > +public: > + skeleton_plugin() {} > + ~skeleton_plugin() {} > + > + void process(const struct record_info *ri); > + void summary(void); > +}; > + > +void skeleton_plugin::process(const struct record_info *ri) > +{ > + /* Put per-trace record stuff here. */ > +} > + > +void skeleton_plugin::summary(void) > +{ > + /* Print a summary of the results (if applicable). */ > +} > + > +DEFINE_CXX_PLUGIN("skeleton", skeleton_plugin); > diff --git a/xenalyze.c b/xenalyze.c > --- a/xenalyze.c > +++ b/xenalyze.c > @@ -32,6 +32,7 @@ > #include "trace.h" > #include "analyze.h" > #include "mread.h" > +#include "plugin.h" > #include "pv.h" > #include<errno.h> > #include<strings.h> > @@ -8763,6 +8764,8 @@ void process_record(struct pcpu_info *p) > default: > process_generic(ri); > } > + > + plugin_process(ri); > } > > UPDATE_VOLUME(p, toplevel[toplevel], ri->size); > @@ -9346,6 +9349,7 @@ enum { > OPT_DUMP_ALL=''a'', > OPT_INTERVAL_LENGTH=''i'', > OPT_SUMMARY=''s'', > + OPT_PLUGIN=''p'', > }; > > enum { > @@ -9816,6 +9820,15 @@ error_t cmd_parser(int key, char *arg, s > opt.tsc_loop_fatal = 1; > break; > > + case OPT_PLUGIN: > + if (plugin_enable(arg)) { > + G.output_defined = 1; > + } else { > + fprintf(stderr, "ERROR: No such plugin `%s''.\n", arg); > + exit(1); > + } > + break; > + > case ARGP_KEY_ARG: > { > /* FIXME - strcpy */ > @@ -10108,6 +10121,10 @@ const struct argp_option cmd_opts[] = { > .arg = "errlevel", > .doc = "Sets tolerance for errors found in the file. Default is 3; max is 6.", }, > > + { .name = "plugin", > + .key = OPT_PLUGIN, > + .arg = "PLUGIN", > + .doc = "Enable a decoder or summary plugin.", }, > > { 0 }, > }; > @@ -10127,6 +10144,8 @@ int main(int argc, char *argv[]) { > /* Start with warn at stderr. */ > warn = stderr; > > + plugin_init(); > + > argp_parse(&parser_def, argc, argv, 0, NULL, NULL); > > if (G.trace_file == NULL) > @@ -10163,6 +10182,8 @@ int main(int argc, char *argv[]) { > if(opt.summary) > summary(); > > + plugin_summary(); > + > if(opt.report_pcpu) > report_pcpu(); >
George Dunlap
2012-Jun-07 11:11 UTC
Re: [PATCH 06 of 10] xenalyze: move struct record_info into a header
On 31/05/12 12:16, David Vrabel wrote:> Split out struct record_info onto the analyze.h so it can be used > outside of xenalyze.c.I assume this is mainly to support the plugin infrastructure? -George> > Signed-off-by: David Vrabel<david.vrabel@citrix.com> > --- > > diff --git a/analyze.h b/analyze.h > --- a/analyze.h > +++ b/analyze.h > @@ -1,5 +1,8 @@ > #ifndef __ANALYZE_H > # define __ANALYZE_H > + > +#include<stdint.h> > + > #define TRC_GEN_MAIN 0 > #define TRC_SCHED_MAIN 1 > #define TRC_DOM0OP_MAIN 2 > @@ -47,4 +50,56 @@ enum { > }; > > #define TRC_HVM_OP_DESTROY_PROC (TRC_HVM_HANDLER + 0x100) > + > +typedef unsigned long long tsc_t; > + > +/* -- on-disk trace buffer definitions -- */ > +struct trace_record { > + union { > + struct { > + unsigned event:28, > + extra_words:3, > + cycle_flag:1; > + union { > + struct { > + uint32_t tsc_lo, tsc_hi; > + uint32_t data[7]; > + } tsc; > + struct { > + uint32_t data[7]; > + } notsc; > + } u; > + }; > + uint32_t raw[8]; > + }; > +}; > + > +/* -- General info about a current record -- */ > +struct time_struct { > + unsigned long long time; > + unsigned int s, ns; > +}; > + > +#define DUMP_HEADER_MAX 256 > + > +struct record_info { > + int cpu; > + tsc_t tsc; > + union { > + unsigned event; > + struct { > + unsigned minor:12, > + sub:4, > + main:12, > + unused:4; > + } evt; > + }; > + int extra_words; > + int size; > + uint32_t *d; > + char dump_header[DUMP_HEADER_MAX]; > + struct time_struct t; > + struct trace_record rec; > +}; > + > #endif > diff --git a/xenalyze.c b/xenalyze.c > --- a/xenalyze.c > +++ b/xenalyze.c > @@ -40,8 +40,6 @@ > struct mread_ctrl; > > > -typedef unsigned long long tsc_t; > - > #define DEFAULT_CPU_HZ 2400000000LL > #define ADDR_SPACE_BITS 48 > #define DEFAULT_SAMPLE_SIZE 10240 > @@ -260,57 +258,8 @@ struct { > .interval = { .msec = DEFAULT_INTERVAL_LENGTH }, > }; > > -/* -- on-disk trace buffer definitions -- */ > -struct trace_record { > - union { > - struct { > - unsigned event:28, > - extra_words:3, > - cycle_flag:1; > - union { > - struct { > - uint32_t tsc_lo, tsc_hi; > - uint32_t data[7]; > - } tsc; > - struct { > - uint32_t data[7]; > - } notsc; > - } u; > - }; > - uint32_t raw[8]; > - }; > -}; > - > FILE *warn = NULL; > > -/* -- General info about a current record -- */ > -struct time_struct { > - unsigned long long time; > - unsigned int s, ns; > -}; > - > -#define DUMP_HEADER_MAX 256 > - > -struct record_info { > - int cpu; > - tsc_t tsc; > - union { > - unsigned event; > - struct { > - unsigned minor:12, > - sub:4, > - main:12, > - unused:4; > - } evt; > - }; > - int extra_words; > - int size; > - uint32_t *d; > - char dump_header[DUMP_HEADER_MAX]; > - struct time_struct t; > - struct trace_record rec; > -}; > - > /* -- Summary data -- */ > struct cycle_framework { > tsc_t first_tsc, last_tsc, total_cycles;
David Vrabel
2012-Jun-07 11:31 UTC
Re: [PATCH 06 of 10] xenalyze: move struct record_info into a header
On 07/06/12 12:11, George Dunlap wrote:> On 31/05/12 12:16, David Vrabel wrote: >> Split out struct record_info onto the analyze.h so it can be used >> outside of xenalyze.c. > I assume this is mainly to support the plugin infrastructure?Yes, but it will also make it easier to split up the enormous xenalyze.c file in the future. David
George Dunlap
2012-Jun-07 11:35 UTC
Re: [PATCH 07 of 10] xenalyze: decode PV_HYPERCALL_V2 records
On 31/05/12 12:16, David Vrabel wrote:> Newer version of Xen produce TRC_PV_HYPERCALL_V2 records instead of > the older TRC_PV_HYPERCALL format. This updated format doesn''t > included the IP but it does include select hypercall arguments. > > Signed-off-by: David Vrabel<david.vrabel@citrix.com> > > diff --git a/pv.h b/pv.h > new file mode 100644 > --- /dev/null > +++ b/pv.hWhy does this need its own file?> +static const char *grant_table_op_cmd_to_str(uint32_t cmd)Hmm -- this is a different style to the other lists of this type. I guess I like having it in a function.> +{ > + const char *cmd_str[] = { > + "map_grant_ref", "unmap_grant_ref", "setup_table", "dump_table", > + "transfer", "copy", "query_size", "unmap_and_replace", > + "set_version", "get_status_frames", "get_version", "swap_grant_ref", > + };I''m a bit wary of having stuff just in a big list like this -- it seems like it makes it harder to double-check that you''ve gotten the right match-up. I''d prefer it look like hvm_event_handler_name[], where the number is annotated with a comment from time to time.> + static char buf[32]; > + > + if (cmd<= 11) > + return cmd_str[cmd];Instead of hardcoding the number of elements, could you use some calculation involving sizeof() to get that automatically? In any case, it should be "cmd < N", rather than "cmd <= N-1" (where N is the number of elements in the array).> + switch(op) { > + case HYPERCALL_mmu_update: > + {I''m not a fan of indenting a brace within a case statement -- I think this is emacs'' default C mode, but I prefer it the other way. (Not sure which config option sets this, though.) Other than that, looks good. -George
David Vrabel
2012-Jun-07 15:20 UTC
Re: [PATCH 07 of 10] xenalyze: decode PV_HYPERCALL_V2 records
On 07/06/12 12:35, George Dunlap wrote:> On 31/05/12 12:16, David Vrabel wrote: >> Newer version of Xen produce TRC_PV_HYPERCALL_V2 records instead of >> the older TRC_PV_HYPERCALL format. This updated format doesn''t >> included the IP but it does include select hypercall arguments. >> >> Signed-off-by: David Vrabel<david.vrabel@citrix.com> >> >> diff --git a/pv.h b/pv.h >> new file mode 100644 >> --- /dev/null >> +++ b/pv.h > Why does this need its own file?I would like to see Xenalyze split into more, smaller files each with related functionality. This is a start.>> +static const char *grant_table_op_cmd_to_str(uint32_t cmd) > Hmm -- this is a different style to the other lists of this type. I > guess I like having it in a function. >> +{ >> + const char *cmd_str[] = { >> + "map_grant_ref", "unmap_grant_ref", "setup_table", "dump_table", >> + "transfer", "copy", "query_size", "unmap_and_replace", >> + "set_version", "get_status_frames", "get_version", "swap_grant_ref", >> + }; > I''m a bit wary of having stuff just in a big list like this -- it seems > like it makes it harder to double-check that you''ve gotten the right > match-up. I''d prefer it look like hvm_event_handler_name[], where the > number is annotated with a comment from time to time.Ok.>> + static char buf[32]; >> + >> + if (cmd<= 11) >> + return cmd_str[cmd]; > Instead of hardcoding the number of elements, could you use some > calculation involving sizeof() to get that automatically? In any case, > it should be "cmd < N", rather than "cmd <= N-1" (where N is the number > of elements in the array).Ok.>> + switch(op) { >> + case HYPERCALL_mmu_update: >> + { > I''m not a fan of indenting a brace within a case statement -- I think > this is emacs'' default C mode, but I prefer it the other way. (Not > sure which config option sets this, though.)Ok. Also, this also doesn''t add the event name so (null) is printed in the summary. I''ll fix this up as well. David
David Vrabel
2012-Jun-07 15:26 UTC
Re: [PATCH 09 of 10] xenalyze: add a basic plugin infrastructure
On 07/06/12 12:05, George Dunlap wrote:> On 31/05/12 12:16, David Vrabel wrote: >> Allow xenalyze to be include (at build time) plugins that can do >> per-record actions and a summary. These plugins can be in C or C++. >> >> The plugins entry points are in struct plugin and pointers to all the >> plugins linked in xenalyze are placed in a "plugin" section so >> plugin_init() can find them all. >> >> A new command line option (-p, --plugin=PLUGIN) is added to enable one >> or more plugins. >> >> A sample plugin (skeleton) is included (mostly because at least one >> plugin must be present for the build to work). >> >> Signed-off-by: David Vrabel<david.vrabel@citrix.com> > So what''s the main motivation of having this plugin infrastructure? The > one plugin example you have ("batch-size", patch 10) seems simple enough > that it should be fairly straightforward to just add as an option, with > not much more boilerplate than C++ already requires. > > Looks like potential advantages may include: > * Ability to use C++ language (for those who care for such things) > * Ability to use STL for complex data structures > * Ability to add an option like the "batch-size" plugin in a concise, > self-contained wayThese are the main reasons. The last is the most important.> Potential disadvantages include: > * An extra O(N) loop on the hot path (where N = # of enabled plugins) > * For each enabled plugin, an extra full function call on the hot path; > and a C++ function at that, which (my prejudice tells me) is likely to > be more wasteful time and space-wise than a C function.I''d be surprised if these had any practical performance penalty but I''ll collect some measurements. David
George Dunlap
2012-Jun-07 16:02 UTC
Re: [PATCH 09 of 10] xenalyze: add a basic plugin infrastructure
On Thu, Jun 7, 2012 at 4:26 PM, David Vrabel <david.vrabel@citrix.com> wrote:> On 07/06/12 12:05, George Dunlap wrote: >> On 31/05/12 12:16, David Vrabel wrote: >>> Allow xenalyze to be include (at build time) plugins that can do >>> per-record actions and a summary. These plugins can be in C or C++. >>> >>> The plugins entry points are in struct plugin and pointers to all the >>> plugins linked in xenalyze are placed in a "plugin" section so >>> plugin_init() can find them all. >>> >>> A new command line option (-p, --plugin=PLUGIN) is added to enable one >>> or more plugins. >>> >>> A sample plugin (skeleton) is included (mostly because at least one >>> plugin must be present for the build to work). >>> >>> Signed-off-by: David Vrabel<david.vrabel@citrix.com> >> So what''s the main motivation of having this plugin infrastructure? The >> one plugin example you have ("batch-size", patch 10) seems simple enough >> that it should be fairly straightforward to just add as an option, with >> not much more boilerplate than C++ already requires. >> >> Looks like potential advantages may include: >> * Ability to use C++ language (for those who care for such things) >> * Ability to use STL for complex data structures >> * Ability to add an option like the "batch-size" plugin in a concise, >> self-contained way > > These are the main reasons. The last is the most important. > >> Potential disadvantages include: >> * An extra O(N) loop on the hot path (where N = # of enabled plugins) >> * For each enabled plugin, an extra full function call on the hot path; >> and a C++ function at that, which (my prejudice tells me) is likely to >> be more wasteful time and space-wise than a C function. > > I''d be surprised if these had any practical performance penalty but I''ll > collect some measurements.Thanks. I was going to mention but forgot: about 6 months ago I did a week''s worth of work optimizing xenalyze. At the beginning of the week, it was taking almost 2 minutes to do a "summary" analysis on a 700MiB file; after my optimizations, it took 10s. In that case things as simple as doing an extraneous integer division per record had a measurable impact. So I''m not just being paranoid. Going from "I can wait 10s for this to complete" to "I''m going to switch to a different task" can have a pretty big impact on my working effectiveness, at least. :-) I also have a set of patches that speed up "xenalyze -a", but that''s a lot more tricky and invasive, as with a 700MiB file you''re actually generating and writing to disk more than 1GiB of data. The solution there is mainly to use bespoke printing routines, rather than the libc ones. Using a plugin infrastructure and going to more indirection / abstraction seems like it would be going in the opposite direction. :-) -George