[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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




On 5/31/19 19:10, Jan Beulich wrote:
On 30.05.19 at 12:17, <chenbaodong@xxxxxxxxxx> wrote:
Default: enabled.
Can be disabled for smaller code footprint.
But you're aware that we're, for now at least, trying to limit the
number of independently selectable config options? Ones depending
on EXPERT are sort of an exception in certain cases.

Limit the number of independently selectable config sounds good to me.

Does the following looks good?

+config HAS_TRACEBUFFER
+       bool "Enable/Disable tracebuffer"  if EXPERT = "y"
+       ---help---
+         Enable or disable tracebuffer function.
+         Xen internal running status(trace event) will be saved to trace memory
+         when enabled.
+


--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -368,4 +368,10 @@ config DOM0_MEM
Leave empty if you are not sure what to specify. +config HAS_TRACEBUFFER
+       bool "Enable/Disable tracebuffer"
+       default y
+       ---help---
+         Enable or disable tracebuffer function.
HAS_* options should not come with a prompt, and should
only be "select"-ed by (normally) per-architecture Kconfig
files. If we are to permit having this option, then I also think
the help text needs expanding.
Tanks for your clarification.
--- a/xen/include/xen/trace.h
+++ b/xen/include/xen/trace.h
@@ -21,12 +21,14 @@
  #ifndef __XEN_TRACE_H__
  #define __XEN_TRACE_H__
-extern int tb_init_done; #include <public/sysctl.h>
  #include <public/trace.h>
  #include <asm/trace.h>
+#ifdef CONFIG_HAS_TRACEBUFFER
+
+extern int tb_init_done;
  /* Used to initialise trace buffer functionality */
  void init_trace_bufs(void);
I wonder if there hadn't been a reason for the declaration to live
up where it was. Also please separate independent blocks of code
by a blank line.
Roger that.

@@ -47,6 +49,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 (0)
Perhaps better "false", and no real need for the parentheses afaict.

+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(u32 event) { return 0; }
+static inline void trace_var(u32 event, int cycles, int extra,
+                             const void *extra_data) {}
+static inline void __trace_var(u32 event, bool_t 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
We try to do away with u32 and friends, so please don't introduce
new uses - even less so when in one case here you already use
uint32_t. Similarly please use "bool" in favor of "bool_t".
Roger that.
Jan


.


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.