[PATCH v1] xen: make tracebuffer configurable

[PATCH v1] xen: make tracebuffer configurable

[Baodong Chen]

Xen internal running status(trace event) will be saved to
trace memory when enabled. trace event data and config params can be
read/changed by system control hypercall at run time.

Can be disabled for smaller code footprint.

Signed-off-by: Baodong Chen <chenbaodong@mxnavi.com>
---
 xen/common/Kconfig      | 12 ++++++++++++
 xen/common/Makefile     |  2 +-
 xen/include/xen/trace.h | 17 +++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index c838506..3459f56 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -368,4 +368,16 @@ config DOM0_MEM
 
 	  Leave empty if you are not sure what to specify.
 
+config TRACEBUFFER
+	bool "Enable trace event logs"  if EXPERT = "y"
+	---help---
+	  Xen internal running status(trace event) will be saved to trace memory
+	  when enabled. trace event data and config params can be read/changed
+	  by system control hypercall at run time.
+
+	  This is used to record xen internal running status.
+	  with a litte performance overhead.
+	  Can be set to 'N' if you dont want this function.
+	  When not configured, 'XEN_STSCTL_tbuf_op' command will result 'ENOSYS'.
+
 endmenu
diff --git a/xen/common/Makefile b/xen/common/Makefile
index bca48e6..9fd1bd2 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -56,7 +56,7 @@ obj-y += sysctl.o
 obj-y += tasklet.o
 obj-y += time.o
 obj-y += timer.o
-obj-y += trace.o
+obj-$(CONFIG_TRACEBUFFER) += trace.o
 obj-y += version.o
 obj-y += virtual_region.o
 obj-y += vm_event.o
diff --git a/xen/include/xen/trace.h b/xen/include/xen/trace.h
index 12966ea..da781fa 100644
--- a/xen/include/xen/trace.h
+++ b/xen/include/xen/trace.h
@@ -21,12 +21,15 @@
 #ifndef __XEN_TRACE_H__
 #define __XEN_TRACE_H__
 
+#ifdef CONFIG_TRACEBUFFER
 extern int tb_init_done;
+#endif
 
 #include <public/sysctl.h>
 #include <public/trace.h>
 #include <asm/trace.h>
 
+#ifdef CONFIG_TRACEBUFFER
 /* Used to initialise trace buffer functionality */
 void init_trace_bufs(void);
 
@@ -47,6 +50,20 @@ static inline void trace_var(u32 event, int cycles, int extra,
 void __trace_hypercall(uint32_t event, unsigned long op,
                        const xen_ulong_t *args);
 
+#else
+#define tb_init_done false
+static inline void init_trace_bufs(void) {}
+static inline int tb_control(struct xen_sysctl_tbuf_op *tbc) { return -ENOSYS; }
+
+static inline int trace_will_trace_event(uint32_t event) { return 0; }
+static inline void trace_var(uint32_t event, int cycles, int extra,
+                             const void *extra_data) {}
+static inline void __trace_var(uint32_t event, bool cycles, unsigned int extra,
+                               const void *extra_data) {}
+static inline void __trace_hypercall(uint32_t event, unsigned long op,
+                                     const xen_ulong_t *args) {}
+#endif
+
 /* Convenience macros for calling the trace function. */
 #define TRACE_0D(_e)                            \
     do {                                        \
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v1] xen: make tracebuffer configurable

[Baodong Chen]

Xen internal running status(trace event) will be saved to
trace memory when enabled. trace event data and config params can be
read/changed by system control hypercall at run time.

Can be disabled for smaller code footprint.

Signed-off-by: Baodong Chen <chenbaodong@mxnavi.com>
---
 xen/common/Kconfig      | 12 ++++++++++++
 xen/common/Makefile     |  2 +-
 xen/include/xen/trace.h | 17 +++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index c838506..3459f56 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -368,4 +368,16 @@ config DOM0_MEM
 
 	  Leave empty if you are not sure what to specify.
 
+config TRACEBUFFER
+	bool "Enable trace event logs"  if EXPERT = "y"
+	---help---
+	  Xen internal running status(trace event) will be saved to trace memory
+	  when enabled. trace event data and config params can be read/changed
+	  by system control hypercall at run time.
+
+	  This is used to record xen internal running status.
+	  with a litte performance overhead.
+	  Can be set to 'N' if you dont want this function.
+	  When not configured, 'XEN_STSCTL_tbuf_op' command will result 'ENOSYS'.
+
 endmenu
diff --git a/xen/common/Makefile b/xen/common/Makefile
index bca48e6..9fd1bd2 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -56,7 +56,7 @@ obj-y += sysctl.o
 obj-y += tasklet.o
 obj-y += time.o
 obj-y += timer.o
-obj-y += trace.o
+obj-$(CONFIG_TRACEBUFFER) += trace.o
 obj-y += version.o
 obj-y += virtual_region.o
 obj-y += vm_event.o
diff --git a/xen/include/xen/trace.h b/xen/include/xen/trace.h
index 12966ea..da781fa 100644
--- a/xen/include/xen/trace.h
+++ b/xen/include/xen/trace.h
@@ -21,12 +21,15 @@
 #ifndef __XEN_TRACE_H__
 #define __XEN_TRACE_H__
 
+#ifdef CONFIG_TRACEBUFFER
 extern int tb_init_done;
+#endif
 
 #include <public/sysctl.h>
 #include <public/trace.h>
 #include <asm/trace.h>
 
+#ifdef CONFIG_TRACEBUFFER
 /* Used to initialise trace buffer functionality */
 void init_trace_bufs(void);
 
@@ -47,6 +50,20 @@ static inline void trace_var(u32 event, int cycles, int extra,
 void __trace_hypercall(uint32_t event, unsigned long op,
                        const xen_ulong_t *args);
 
+#else
+#define tb_init_done false
+static inline void init_trace_bufs(void) {}
+static inline int tb_control(struct xen_sysctl_tbuf_op *tbc) { return -ENOSYS; }
+
+static inline int trace_will_trace_event(uint32_t event) { return 0; }
+static inline void trace_var(uint32_t event, int cycles, int extra,
+                             const void *extra_data) {}
+static inline void __trace_var(uint32_t event, bool cycles, unsigned int extra,
+                               const void *extra_data) {}
+static inline void __trace_hypercall(uint32_t event, unsigned long op,
+                                     const xen_ulong_t *args) {}
+#endif
+
 /* Convenience macros for calling the trace function. */
 #define TRACE_0D(_e)                            \
     do {                                        \
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v1] xen: make tracebuffer configurable

[Jan Beulich]

>>> On 04.06.19 at 02:44, <chenbaodong@mxnavi.com> wrote:
> --- a/xen/include/xen/trace.h
> +++ b/xen/include/xen/trace.h
> @@ -21,12 +21,15 @@
>  #ifndef __XEN_TRACE_H__
>  #define __XEN_TRACE_H__
>  
> +#ifdef CONFIG_TRACEBUFFER
>  extern int tb_init_done;
> +#endif

If this is to stay up here (which I'm still not sure it needs to; I had
merely indicated that there likely is a reason for this without actually
knowing what that reason might be), then I think the #define needs
to go here as well, in an #else.

> @@ -47,6 +50,20 @@ static inline void trace_var(u32 event, int cycles, int extra,
>  void __trace_hypercall(uint32_t event, unsigned long op,
>                         const xen_ulong_t *args);
>  
> +#else
> +#define tb_init_done false
> +static inline void init_trace_bufs(void) {}
> +static inline int tb_control(struct xen_sysctl_tbuf_op *tbc) { return -ENOSYS; }

-EOPNOTSUPP

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1] xen: make tracebuffer configurable

[Jan Beulich]

>>> On 04.06.19 at 02:44, <chenbaodong@mxnavi.com> wrote:
> --- a/xen/include/xen/trace.h
> +++ b/xen/include/xen/trace.h
> @@ -21,12 +21,15 @@
>  #ifndef __XEN_TRACE_H__
>  #define __XEN_TRACE_H__
>  
> +#ifdef CONFIG_TRACEBUFFER
>  extern int tb_init_done;
> +#endif

If this is to stay up here (which I'm still not sure it needs to; I had
merely indicated that there likely is a reason for this without actually
knowing what that reason might be), then I think the #define needs
to go here as well, in an #else.

> @@ -47,6 +50,20 @@ static inline void trace_var(u32 event, int cycles, int extra,
>  void __trace_hypercall(uint32_t event, unsigned long op,
>                         const xen_ulong_t *args);
>  
> +#else
> +#define tb_init_done false
> +static inline void init_trace_bufs(void) {}
> +static inline int tb_control(struct xen_sysctl_tbuf_op *tbc) { return -ENOSYS; }

-EOPNOTSUPP

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v1] xen: make tracebuffer configurable

[chenbaodong]

On 6/4/19 15:10, Jan Beulich wrote:
>>>> On 04.06.19 at 02:44, <chenbaodong@mxnavi.com> wrote:
>> --- a/xen/include/xen/trace.h
>> +++ b/xen/include/xen/trace.h
>> @@ -21,12 +21,15 @@
>>   #ifndef __XEN_TRACE_H__
>>   #define __XEN_TRACE_H__
>>   
>> +#ifdef CONFIG_TRACEBUFFER
>>   extern int tb_init_done;
>> +#endif
> If this is to stay up here (which I'm still not sure it needs to; I had
> merely indicated that there likely is a reason for this without actually
> knowing what that reason might be), then I think the #define needs
> to go here as well, in an #else.

Yes, need to stay here because 'tb_init_done' used in 'asm-x86/trace.h'

which included  by 'xen/trace.h'  at line 32: #include <asm/trace.h>

will be fixed in next version.

>
>> @@ -47,6 +50,20 @@ static inline void trace_var(u32 event, int cycles, int extra,
>>   void __trace_hypercall(uint32_t event, unsigned long op,
>>                          const xen_ulong_t *args);
>>   
>> +#else
>> +#define tb_init_done false
>> +static inline void init_trace_bufs(void) {}
>> +static inline int tb_control(struct xen_sysctl_tbuf_op *tbc) { return -ENOSYS; }
> -EOPNOTSUPP
>
> Jan
>
>
> .
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1] xen: make tracebuffer configurable

[chenbaodong]

On 6/4/19 15:10, Jan Beulich wrote:
>>>> On 04.06.19 at 02:44, <chenbaodong@mxnavi.com> wrote:
>> --- a/xen/include/xen/trace.h
>> +++ b/xen/include/xen/trace.h
>> @@ -21,12 +21,15 @@
>>   #ifndef __XEN_TRACE_H__
>>   #define __XEN_TRACE_H__
>>   
>> +#ifdef CONFIG_TRACEBUFFER
>>   extern int tb_init_done;
>> +#endif
> If this is to stay up here (which I'm still not sure it needs to; I had
> merely indicated that there likely is a reason for this without actually
> knowing what that reason might be), then I think the #define needs
> to go here as well, in an #else.

Yes, need to stay here because 'tb_init_done' used in 'asm-x86/trace.h'

which included  by 'xen/trace.h'  at line 32: #include <asm/trace.h>

will be fixed in next version.

>
>> @@ -47,6 +50,20 @@ static inline void trace_var(u32 event, int cycles, int extra,
>>   void __trace_hypercall(uint32_t event, unsigned long op,
>>                          const xen_ulong_t *args);
>>   
>> +#else
>> +#define tb_init_done false
>> +static inline void init_trace_bufs(void) {}
>> +static inline int tb_control(struct xen_sysctl_tbuf_op *tbc) { return -ENOSYS; }
> -EOPNOTSUPP
>
> Jan
>
>
> .
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1] xen: make tracebuffer configurable

[George Dunlap]

> On Jun 4, 2019, at 1:44 AM, Baodong Chen <chenbaodong@mxnavi.com> wrote:
> 
> Xen internal running status(trace event) will be saved to
> trace memory when enabled. trace event data and config params can be
> read/changed by system control hypercall at run time.
> 
> Can be disabled for smaller code footprint.
> 
> Signed-off-by: Baodong Chen <chenbaodong@mxnavi.com>
> ---
> xen/common/Kconfig      | 12 ++++++++++++
> xen/common/Makefile     |  2 +-
> xen/include/xen/trace.h | 17 +++++++++++++++++
> 3 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index c838506..3459f56 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -368,4 +368,16 @@ config DOM0_MEM
> 
> 	  Leave empty if you are not sure what to specify.
> 
> +config TRACEBUFFER
> +	bool "Enable trace event logs"  if EXPERT = "y"
> +	---help---
> +	  Xen internal running status(trace event) will be saved to trace memory
> +	  when enabled. trace event data and config params can be read/changed
> +	  by system control hypercall at run time.
> +
> +	  This is used to record xen internal running status.
> +	  with a litte performance overhead.
> +	  Can be set to 'N' if you dont want this function.
> +	  When not configured, 'XEN_STSCTL_tbuf_op' command will result 'ENOSYS’.

I think this would look better something like this:

“Enable tracing infrastructure”

“Enable in tracing infrastructure and pre-defined tracepoints within Xen.  This will allow live information about Xen’s execution and performance to be collected at run time for debugging or performance analysis.  Memory and execution overhead when not active is minimal."

Also, I’m not 100% familiar with the kconfig syntax — I think we want tracing enabled by default unless actively disabled; is that what will happen here?  Or will it only be enabled if EXPERT == ‘y’?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1] xen: make tracebuffer configurable

[Jan Beulich]

>>> On 04.06.19 at 12:49, <George.Dunlap@citrix.com> wrote:
>> On Jun 4, 2019, at 1:44 AM, Baodong Chen <chenbaodong@mxnavi.com> wrote:
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -368,4 +368,16 @@ config DOM0_MEM
>> 
>> 	  Leave empty if you are not sure what to specify.
>> 
>> +config TRACEBUFFER
>> +	bool "Enable trace event logs"  if EXPERT = "y"
>> +	---help---
>> +	  Xen internal running status(trace event) will be saved to trace memory
>> +	  when enabled. trace event data and config params can be read/changed
>> +	  by system control hypercall at run time.
>> +
>> +	  This is used to record xen internal running status.
>> +	  with a litte performance overhead.
>> +	  Can be set to 'N' if you dont want this function.
>> +	  When not configured, 'XEN_STSCTL_tbuf_op' command will result 'ENOSYS’.
> 
> I think this would look better something like this:
> 
> “Enable tracing infrastructure”
> 
> “Enable in tracing infrastructure and pre-defined tracepoints within Xen.  
> This will allow live information about Xen’s execution and performance to be 
> collected at run time for debugging or performance analysis.  Memory and 
> execution overhead when not active is minimal."
> 
> Also, I’m not 100% familiar with the kconfig syntax — I think we want 
> tracing enabled by default unless actively disabled; is that what will happen 
> here?  Or will it only be enabled if EXPERT == ‘y’?

Oh, indeed - there's a "default y" missing.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v1] xen: make tracebuffer configurable

[chenbaodong]

On 6/4/19 19:43, Jan Beulich wrote:
>>>> On 04.06.19 at 12:49, <George.Dunlap@citrix.com> wrote:
>>> On Jun 4, 2019, at 1:44 AM, Baodong Chen <chenbaodong@mxnavi.com> wrote:
>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -368,4 +368,16 @@ config DOM0_MEM
>>>
>>> 	  Leave empty if you are not sure what to specify.
>>>
>>> +config TRACEBUFFER
>>> +	bool "Enable trace event logs"  if EXPERT = "y"
>>> +	---help---
>>> +	  Xen internal running status(trace event) will be saved to trace memory
>>> +	  when enabled. trace event data and config params can be read/changed
>>> +	  by system control hypercall at run time.
>>> +
>>> +	  This is used to record xen internal running status.
>>> +	  with a litte performance overhead.
>>> +	  Can be set to 'N' if you dont want this function.
>>> +	  When not configured, 'XEN_STSCTL_tbuf_op' command will result 'ENOSYS’.
>> I think this would look better something like this:
>>
>> “Enable tracing infrastructure”
>>
>> “Enable in tracing infrastructure and pre-defined tracepoints within Xen.
>> This will allow live information about Xen’s execution and performance to be
>> collected at run time for debugging or performance analysis.  Memory and
>> execution overhead when not active is minimal."
>>
>> Also, I’m not 100% familiar with the kconfig syntax — I think we want
>> tracing enabled by default unless actively disabled; is that what will happen
>> here?  Or will it only be enabled if EXPERT == ‘y’?
> Oh, indeed - there's a "default y" missing.

Thanks for suggestion for the help text. and pointing out "default y" 
missing.

Fixed in v3 and please discard v2.

>
> Jan
>
> .
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel