John Levon
2006-May-15 16:19 UTC
[Xen-devel] Re: [Xen-changelog] Fix xentrace to initialise the trace buffers if they are not set up.
On Sun, May 14, 2006 at 08:56:08PM +0000, Atsushi SAKAI wrote:> Fix xentrace to initialise the trace buffers if they are not set up. > Signed-off-by: Atsushi SAKAI <sakaia@jp.fujitsu.com>> +#include "xc_private.h"Please don''t do this. It''s called "xc_private.h" for a reason.> + int ret; > + dom0_op_t op; /* dom0 op we''ll build */ > int xc_handle = xc_interface_open(); /* for accessing control interface */ > - > - if (xc_tbuf_get_size(xc_handle, &size32) != 0) > - goto fail; > - *size = size32; > - > - if (xc_tbuf_get_mfn(xc_handle, mfn) != 0) > - goto fail; > + unsigned int tbsize; > + > + enable_tracing_or_die(xc_handle); > + > + if (xc_tbuf_get_size(xc_handle, &tbsize) != 0) { > + perror("Failure to get tbuf info from Xen. Guess size is 0?"); > + exit(1); > + } > + else > + printf("Current tbuf size: 0x%x\n", tbsize); > + > + > + op.cmd = DOM0_TBUFCONTROL; > + op.interface_version = DOM0_INTERFACE_VERSION; > + op.u.tbufcontrol.op = DOM0_TBUF_GET_INFO; > + > + ret = do_dom0_op(xc_handle, &op);In particular, don''t do this. Only libbxc/xc_tbuf.c can include xc_private.h. Why have you re-introduced this? Can you fix it back up please to use get_size/get_mfn()? thanks, john _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2006-May-15 16:30 UTC
Re: [Xen-devel] Re: [Xen-changelog] Fix xentrace to initialise the trace buffers if they are not set up.
On 15 May 2006, at 17:19, John Levon wrote:> In particular, don''t do this. Only libbxc/xc_tbuf.c can include > xc_private.h. Why have you re-introduced this? Can you fix it back up > please to use get_size/get_mfn()?I guess he was cribbing from xenmon/xenbaked.c which still includes xc_private.h. Your cleanup patch only touched that file very superficially. I think there is some infrastructure common to both xentrace and xenmon that needs moving into xc_tbuf.c. That will remove duplicated code and mean that only libxenctrl needs to include xc_private.h and do grubby stuff with dom0 ops. I look forward to patches from someone. :-) --- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2006-May-15 16:39 UTC
Re: [Xen-devel] Re: [Xen-changelog] Fix xentrace to initialise the trace buffers if they are not set up.
On Mon, May 15, 2006 at 05:30:37PM +0100, Keir Fraser wrote:> I guess he was cribbing from xenmon/xenbaked.c which still includes > xc_private.h.Ah, yes.> I think there is some infrastructure common to both xentrace and xenmon > that needs moving into xc_tbuf.c. That will remove duplicated code andThis particular bit is actually already there, it''s just another simple cleanup.> I look forward to patches from someone. :-)I''ve been distracted by other things but I hope to find some time to continue on these cleanups soon. regards, john _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2006-May-16 00:04 UTC
[Xen-devel] [PATCH] Re: Fix xentrace to initialise the trace buffers if they are not set up.
On Mon, May 15, 2006 at 05:30:37PM +0100, Keir Fraser wrote:> I think there is some infrastructure common to both xentrace and xenmon > that needs moving into xc_tbuf.c. That will remove duplicated code andHere''s a patch that does that. BTW, there seems to be little point to ''setsize'' and ''tbctl''. They have to be invoked in a particular order. We could cut even more code here if we got rid of them. Is there a reason they exist? Next on the list is xsd_kva and its friends, followed by adding a proper evtchn interface instead of explicit ioctl()s. thanks, john PS next person to misuse xc_private.h gets a rap on the knuckles from me now. # HG changeset patch # User john.levon@sun.com # Node ID 627b79e3f0223d703f2e56857069227f93a6702a # Parent dc213d745642690b4bbc34af951e57f0d04c2d04 Use common code for enabling tracing via xenmon and xentrace, also fixing the last two remaining xc_private.h users outside of libxc. Signed-off-by: John Levon <john.levon@sun.com> diff -r dc213d745642 -r 627b79e3f022 tools/libxc/xc_tbuf.c --- a/tools/libxc/xc_tbuf.c Mon May 15 16:32:09 2006 +0100 +++ b/tools/libxc/xc_tbuf.c Mon May 15 08:16:01 2006 -0700 @@ -16,7 +16,7 @@ #include "xc_private.h" -int xc_tbuf_enable(int xc_handle, int enable) +static int tbuf_enable(int xc_handle, int enable) { DECLARE_DOM0_OP; @@ -30,7 +30,7 @@ int xc_tbuf_enable(int xc_handle, int en return xc_dom0_op(xc_handle, &op); } -int xc_tbuf_set_size(int xc_handle, uint32_t size) +int xc_tbuf_set_size(int xc_handle, unsigned long size) { DECLARE_DOM0_OP; @@ -42,7 +42,7 @@ int xc_tbuf_set_size(int xc_handle, uint return xc_dom0_op(xc_handle, &op); } -int xc_tbuf_get_size(int xc_handle, uint32_t *size) +int xc_tbuf_get_size(int xc_handle, unsigned long *size) { int rc; DECLARE_DOM0_OP; @@ -57,10 +57,17 @@ int xc_tbuf_get_size(int xc_handle, uint return rc; } -int xc_tbuf_get_mfn(int xc_handle, unsigned long *mfn) +int xc_tbuf_enable(int xc_handle, size_t cnt, unsigned long *mfn, + unsigned long *size) { + DECLARE_DOM0_OP; int rc; - DECLARE_DOM0_OP; + + if ( xc_tbuf_set_size(xc_handle, cnt) != 0 ) + return -1; + + if ( tbuf_enable(xc_handle, 1) != 0 ) + return -1; op.cmd = DOM0_TBUFCONTROL; op.interface_version = DOM0_INTERFACE_VERSION; @@ -68,8 +75,17 @@ int xc_tbuf_get_mfn(int xc_handle, unsig rc = xc_dom0_op(xc_handle, &op); if ( rc == 0 ) - *mfn = op.u.tbufcontrol.buffer_mfn; - return rc; + { + *size = op.u.tbufcontrol.size; + *mfn = op.u.tbufcontrol.buffer_mfn; + } + + return 0; +} + +int xc_tbuf_disable(int xc_handle) +{ + return tbuf_enable(xc_handle, 0); } int xc_tbuf_set_cpu_mask(int xc_handle, uint32_t mask) @@ -95,3 +111,4 @@ int xc_tbuf_set_evt_mask(int xc_handle, return do_dom0_op(xc_handle, &op); } + diff -r dc213d745642 -r 627b79e3f022 tools/libxc/xenctrl.h --- a/tools/libxc/xenctrl.h Mon May 15 16:32:09 2006 +0100 +++ b/tools/libxc/xenctrl.h Mon May 15 08:16:01 2006 -0700 @@ -529,15 +529,23 @@ long xc_get_tot_pages(int xc_handle, uin */ /** - * This function enables or disables tracing. Trace buffer memory must - * be already allocated by setting the size to a non-zero value, otherwise - * tracing cannot be enabled. - * - * @parm xc_handle a handle to an open hypervisor interface - * @parm enable the desired action, 1 for enable, 0 for disable - * @return 0 on success, -1 on failure. - */ -int xc_tbuf_enable(int xc_handle, int enable); + * xc_tbuf_enable - enable tracing buffers + * + * @parm xc_handle a handle to an open hypervisor interface + * @parm cnt size of tracing buffers to create (in pages) + * @parm mfn location to store mfn of the trace buffers to + * @parm size location to store the size (in bytes) of a trace buffer to + * + * Gets the machine address of the trace pointer area and the size of the + * per CPU buffers. + */ +int xc_tbuf_enable(int xc_handle, size_t cnt, unsigned long *mfn, + unsigned long *size); + +/* + * Disable tracing buffers. + */ +int xc_tbuf_disable(int xc_handle); /** * This function sets the size of the trace buffers. Setting the size @@ -549,7 +557,7 @@ int xc_tbuf_enable(int xc_handle, int en * @parm size the size in pages per cpu for the trace buffers * @return 0 on success, -1 on failure. */ -int xc_tbuf_set_size(int xc_handle, uint32_t size); +int xc_tbuf_set_size(int xc_handle, unsigned long size); /** * This function retrieves the current size of the trace buffers. @@ -559,16 +567,7 @@ int xc_tbuf_set_size(int xc_handle, uint * @parm size will contain the size in bytes for the trace buffers * @return 0 on success, -1 on failure. */ -int xc_tbuf_get_size(int xc_handle, uint32_t *size); - -/** - * This function retrieves the machine frame of the trace buffer. - - * @parm xc_handle a handle to an open hypervisor interface - * @parm mfn will contain the machine frame of the buffer. - * @return 0 on success, -1 on failure. - */ -int xc_tbuf_get_mfn(int xc_handle, unsigned long *mfn); +int xc_tbuf_get_size(int xc_handle, unsigned long *size); int xc_tbuf_set_cpu_mask(int xc_handle, uint32_t mask); diff -r dc213d745642 -r 627b79e3f022 tools/xenmon/xenbaked.c --- a/tools/xenmon/xenbaked.c Mon May 15 16:32:09 2006 +0100 +++ b/tools/xenmon/xenbaked.c Mon May 15 08:16:01 2006 -0700 @@ -35,6 +35,7 @@ #include <sys/mman.h> #include <sys/stat.h> #include <sys/types.h> +#include <sys/ioctl.h> #include <fcntl.h> #include <unistd.h> #include <errno.h> @@ -46,7 +47,14 @@ #include <sys/select.h> #include <xen/linux/evtchn.h> -#include "xc_private.h" +#define PERROR(_m, _a...) \ +do { \ + int __saved_errno = errno; \ + fprintf(stderr, "ERROR: " _m " (%d = %s)\n" , ## _a , \ + __saved_errno, strerror(__saved_errno)); \ + errno = __saved_errno; \ +} while (0) + typedef struct { int counter; } atomic_t; #define _atomic_read(v) ((v).counter) @@ -326,77 +334,32 @@ void wait_for_event(void) } } -void enable_tracing_or_die(int xc_handle) -{ - int enable = 1; - int tbsize = DEFAULT_TBUF_SIZE; - - if (xc_tbuf_enable(xc_handle, enable) != 0) { - if (xc_tbuf_set_size(xc_handle, tbsize) != 0) { - perror("set_size Hypercall failure"); - exit(1); - } - printf("Set default trace buffer allocation (%d pages)\n", tbsize); - if (xc_tbuf_enable(xc_handle, enable) != 0) { - perror("Could not enable trace buffers\n"); - exit(1); - } - } - else - printf("Tracing enabled\n"); -} - -void disable_tracing(void) -{ - int enable = 0; - int xc_handle = xc_interface_open(); - - xc_tbuf_enable(xc_handle, enable); - xc_interface_close(xc_handle); -} - - -/** - * get_tbufs - get pointer to and size of the trace buffers - * @mfn: location to store mfn of the trace buffers to - * @size: location to store the size of a trace buffer to - * - * Gets the machine address of the trace pointer area and the size of the - * per CPU buffers. - */ -void get_tbufs(unsigned long *mfn, unsigned long *size) -{ +static void get_tbufs(unsigned long *mfn, unsigned long *size) +{ + int xc_handle = xc_interface_open(); int ret; - dom0_op_t op; /* dom0 op we''ll build */ - int xc_handle = xc_interface_open(); /* for accessing control interface */ - unsigned int tbsize; - - enable_tracing_or_die(xc_handle); - - if (xc_tbuf_get_size(xc_handle, &tbsize) != 0) { - perror("Failure to get tbuf info from Xen. Guess size is 0?"); - exit(1); - } - else - printf("Current tbuf size: 0x%x\n", tbsize); - - - op.cmd = DOM0_TBUFCONTROL; - op.interface_version = DOM0_INTERFACE_VERSION; - op.u.tbufcontrol.op = DOM0_TBUF_GET_INFO; - - ret = do_dom0_op(xc_handle, &op); - - xc_interface_close(xc_handle); + + if ( xc_handle < 0 ) + { + exit(EXIT_FAILURE); + } + + ret = xc_tbuf_enable(xc_handle, DEFAULT_TBUF_SIZE, mfn, size); if ( ret != 0 ) { - PERROR("Failure to get trace buffer pointer from Xen"); - exit(EXIT_FAILURE); - } - - *mfn = op.u.tbufcontrol.buffer_mfn; - *size = op.u.tbufcontrol.size; + perror("Couldn''t enable trace buffers"); + exit(1); + } + + xc_interface_close(xc_handle); +} + +void disable_tracing(void) +{ + int xc_handle = xc_interface_open(); + xc_tbuf_disable(xc_handle); + xc_interface_close(xc_handle); } /** diff -r dc213d745642 -r 627b79e3f022 tools/xentrace/xentrace.c --- a/tools/xentrace/xentrace.c Mon May 15 16:32:09 2006 +0100 +++ b/tools/xentrace/xentrace.c Mon May 15 08:16:01 2006 -0700 @@ -28,8 +28,6 @@ #include <xenctrl.h> -#include "xc_private.h" - #define PERROR(_m, _a...) \ do { \ int __saved_errno = errno; \ @@ -103,67 +101,25 @@ void write_rec(unsigned int cpu, struct } } -void enable_tracing_or_die(int xc_handle) -{ - int enable = 1; - int tbsize = DEFAULT_TBUF_SIZE; - - if (xc_tbuf_enable(xc_handle, enable) != 0) { - if (xc_tbuf_set_size(xc_handle, tbsize) != 0) { - perror("set_size Hypercall failure"); - exit(1); - } - printf("Set default trace buffer allocation (%d pages)\n", tbsize); - if (xc_tbuf_enable(xc_handle, enable) != 0) { - perror("Could not enable trace buffers\n"); - exit(1); - } - } - else - printf("Tracing enabled\n"); -} - -/** - * get_tbufs - get pointer to and size of the trace buffers - * @mfn: location to store mfn of the trace buffers to - * @size: location to store the size of a trace buffer to - * - * Gets the machine address of the trace pointer area and the size of the - * per CPU buffers. - */ -void get_tbufs(unsigned long *mfn, unsigned long *size) -{ +static void get_tbufs(unsigned long *mfn, unsigned long *size) +{ + int xc_handle = xc_interface_open(); int ret; - dom0_op_t op; /* dom0 op we''ll build */ - int xc_handle = xc_interface_open(); /* for accessing control interface */ - unsigned int tbsize; - - enable_tracing_or_die(xc_handle); - - if (xc_tbuf_get_size(xc_handle, &tbsize) != 0) { - perror("Failure to get tbuf info from Xen. Guess size is 0?"); - exit(1); - } - else - printf("Current tbuf size: 0x%x\n", tbsize); - - - op.cmd = DOM0_TBUFCONTROL; - op.interface_version = DOM0_INTERFACE_VERSION; - op.u.tbufcontrol.op = DOM0_TBUF_GET_INFO; - - ret = do_dom0_op(xc_handle, &op); + + if ( xc_handle < 0 ) + { + exit(EXIT_FAILURE); + } + + ret = xc_tbuf_enable(xc_handle, DEFAULT_TBUF_SIZE, mfn, size); + + if ( ret != 0 ) + { + perror("Couldn''t enable trace buffers"); + exit(1); + } xc_interface_close(xc_handle); - - if ( ret != 0 ) - { - PERROR("Failure to get trace buffer pointer from Xen"); - exit(EXIT_FAILURE); - } - - *mfn = op.u.tbufcontrol.buffer_mfn; - *size = op.u.tbufcontrol.size; } /** _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2006-May-16 07:52 UTC
[Xen-devel] Re: [PATCH] Re: Fix xentrace to initialise the trace buffers if they are not set up.
On 16 May 2006, at 01:04, John Levon wrote:> Here''s a patch that does that. BTW, there seems to be little point to > ''setsize'' and ''tbctl''. They have to be invoked in a particular order. > We > could cut even more code here if we got rid of them. Is there a reason > they exist?Ian''s already said he wants them to go. Is there equivalent functionality elsewhere already? -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2006-May-16 14:21 UTC
[Xen-devel] Re: [PATCH] Re: Fix xentrace to initialise the trace buffers if they are not set up.
On Tue, May 16, 2006 at 08:52:05AM +0100, Keir Fraser wrote:> >Here''s a patch that does that. BTW, there seems to be little point to > >''setsize'' and ''tbctl''. They have to be invoked in a particular order. > >We > >could cut even more code here if we got rid of them. Is there a reason > >they exist? > > Ian''s already said he wants them to go. Is there equivalent > functionality elsewhere already?Well, that depends what they''re intended for. At the very least, it would be easy to modify xentrace to have an -e / -d option for enabling and disabling tracing without actually capturing the data, but I don''t know if anyone would actually want to do that independent of running xentrace/xentrace_format (or xenmon). regards john _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel