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

Re: [Minios-devel] [UNIKRAFT PATCH 2/8] lib/ukdebug: Register a tracepoint



Hi Yuri,

Please see my comments inline.

On 5/10/19 9:29 PM, Yuri Volchkov wrote:
> Create a 'special' section .uk_tracepoints_list, where we will be
> storing static definitions of tracepoints. This section is need only
> offline, so it will exist only in *.dbg image.
> 
> Signed-off-by: Yuri Volchkov <yuri.volchkov@xxxxxxxxx>
> ---
>  lib/ukdebug/Config.uk          |  9 ++++
>  lib/ukdebug/Makefile.uk        |  3 ++
>  lib/ukdebug/extra.ld           | 23 +++++++++
>  lib/ukdebug/include/uk/trace.h | 88 ++++++++++++++++++++++++++++++++++
>  4 files changed, 123 insertions(+)
>  create mode 100644 lib/ukdebug/extra.ld
> 
> diff --git a/lib/ukdebug/Config.uk b/lib/ukdebug/Config.uk
> index 746ac3de..ab238aa0 100644
> --- a/lib/ukdebug/Config.uk
> +++ b/lib/ukdebug/Config.uk
> @@ -75,4 +75,13 @@ config LIBUKDEBUG_ENABLE_ASSERT
>       help
>         Build code with assertions.
>  
> +menuconfig LIBUKDEBUG_TRACEPOINTS
> +     bool "enable tracepoints"

If there will be a v2, please properly edit this string, inline with the
other config options of Unikraft.

> +     default n
> +if LIBUKDEBUG_TRACEPOINTS
> +config LIBUKDEBUG_ALL_TRACEPOINTS
> +     bool "enable all tracepoints at once"
> +     default n
> +endif
> +
>  endif
> diff --git a/lib/ukdebug/Makefile.uk b/lib/ukdebug/Makefile.uk
> index 4aaa5189..abc340dd 100644
> --- a/lib/ukdebug/Makefile.uk
> +++ b/lib/ukdebug/Makefile.uk
> @@ -8,3 +8,6 @@ LIBUKDEBUG_CXXFLAGS-y += -D__IN_LIBUKDEBUG__
>  
>  LIBUKDEBUG_SRCS-y += $(LIBUKDEBUG_BASE)/print.c
>  LIBUKDEBUG_SRCS-y += $(LIBUKDEBUG_BASE)/hexdump.c
> +
> +EXTRA_LD_SCRIPT-$(CONFIG_LIBVFSCORE) += $(LIBUKDEBUG_BASE)/extra.ld
> +STRIP_SECTIONS_FLAGS-$(CONFIG_LIBUKDEBUG_TRACEPOINTS) += -R 
> .uk_tracepoints_list
> diff --git a/lib/ukdebug/extra.ld b/lib/ukdebug/extra.ld
> new file mode 100644
> index 00000000..43154f9d
> --- /dev/null
> +++ b/lib/ukdebug/extra.ld
> @@ -0,0 +1,23 @@
> +SECTIONS
> +{
> +     .uk_tracepoints_list : {
> +             uk_tracepoints_start = .;
> +             KEEP (*(.uk_tracepoints_list));
> +             uk_tracepoints_end = .;
> +     }
> +}
> +
> +/* We do not want these section to be in the final image, they are
> + * needed only offline. Solution is obvious - ask the build system to
> + * strip them together with debug symbols.
> + *
> + * However, we need to put them at them somewhere close to the end of
> + * the elf, otherwise strip operation will make a hole in the address
> + * space. More then that, they have to reside _after_ the '_end'
> + * symbol.
> + *
> + * The trick is to allocate the sections after the '.comment'
> + * section. The other way would have been allocating a dummy section
> + * '.cut_here', but linker drops it if there is nothing in it.
> + */
> +INSERT AFTER .comment;
> diff --git a/lib/ukdebug/include/uk/trace.h b/lib/ukdebug/include/uk/trace.h
> index 3e17b30e..34f0ed61 100644
> --- a/lib/ukdebug/include/uk/trace.h
> +++ b/lib/ukdebug/include/uk/trace.h
> @@ -34,6 +34,16 @@
>  
>  #ifndef _UK_TRACE_H_
>  #define _UK_TRACE_H_
> +#include <uk/essentials.h>
> +#include <stdint.h>
> +
> +#define UK_TP_DEF_MAGIC 0x65645054 /* TPde */
> +
> +enum __uk_trace_arg_type {
> +     __UK_TRACE_ARG_INT = 0,
> +     __UK_TRACE_ARG_STRING = 1,
> +};
> +
>  /* TODO: consider to move UK_CONCAT into public headers */
>  #define __UK_CONCAT_X(a, b) a##b
>  #define UK_CONCAT(a, b) __UK_CONCAT_X(a, b)
> @@ -41,6 +51,13 @@
>  #define __UK_NARGS_X(a, b, c, d, e, f, g, h, n, ...) n
>  #define UK_NARGS(...)  __UK_NARGS_X(, ##__VA_ARGS__, 7, 6, 5, 4, 3, 2, 1, 0)
>  
> +#define __UK_TRACE_GET_TYPE(arg) (                                   \
> +     __builtin_types_compatible_p(typeof(arg), const char *) *       \
> +             __UK_TRACE_ARG_STRING +                                 \
> +     __builtin_types_compatible_p(typeof(arg), char *) *             \
> +             __UK_TRACE_ARG_STRING +                                 \
> +     0)

Do we really need this +0 ?

> +
>  #define __UK_GET_ARG1(a1, ...) a1
>  #define __UK_GET_ARG2(a1, a2, ...) a2
>  #define __UK_GET_ARG3(a1, a2, a3, ...) a3
> @@ -73,4 +90,75 @@
>  #define UK_FOREACH(n, f, ...)                                        \
>       UK_CONCAT(UK_FOREACH, n)(f, __VA_ARGS__)
>  
> +
> +#define UK_FOREACH_SIZEOF(a, b) sizeof(b)
> +#define __UK_TRACE_ARG_SIZES(n, ...)                         \
> +     { UK_FOREACH(n, UK_FOREACH_SIZEOF, __VA_ARGS__) }
> +
> +#define __UK_TRACE_GET_TYPE_FOREACH(n, arg)  \
> +     __UK_TRACE_GET_TYPE(arg)
> +#define __UK_TRACE_ARG_TYPES(n, ...)                                 \
> +     { UK_FOREACH(n, __UK_TRACE_GET_TYPE_FOREACH, __VA_ARGS__) }
> +
> +#define __UK_TRACE_REG(NR, regname, trace_name, fmt, ...)    \
> +     UK_CTASSERT(sizeof(#trace_name) < 255);                 \
> +     UK_CTASSERT(sizeof(fmt) < 255);                         \
> +     __attribute((__section__(".uk_tracepoints_list")))      \
> +     static struct {                                         \
> +             uint32_t magic;                                 \
> +             uint32_t size;                                  \
> +             uint64_t cookie;                                \
> +             uint8_t args_nr;                                \
> +             uint8_t name_len;                               \
> +             uint8_t format_len;                             \
> +             uint8_t sizes[NR];                              \
> +             uint8_t types[NR];                              \

If we get rid of the n parameter in FOREACH,  then we can also get rid
of the NR parameter. I don't feel strong about this one, it's your call.

> +             char name[sizeof(#trace_name)];                 \
> +             char format[sizeof(fmt)];                       \
> +     } regname __used = {                                    \
> +             UK_TP_DEF_MAGIC,                                \
> +             sizeof(regname),                                \
> +             (uint64_t) &regname,                            \
> +             NR,                                             \
> +             sizeof(#trace_name), sizeof(fmt),               \
> +             __UK_TRACE_ARG_SIZES(NR, __VA_ARGS__),          \
> +             __UK_TRACE_ARG_TYPES(NR, __VA_ARGS__),          \
> +             #trace_name, fmt }
> +
> +/* Makes from "const char*" "const char* arg1".
> + */
> +#define __UK_ARGS_MAP_FN(n, t) t UK_CONCAT(arg, n)
> +#define __UK_TRACE_ARGS_MAP(n, ...) \
> +     UK_FOREACH(n, __UK_ARGS_MAP_FN, __VA_ARGS__)
> +
> +#define __UK_ARGS_MAP_FN_UNUSED(n, t) t UK_CONCAT(arg, n) __unused
> +#define __UK_TRACE_ARGS_MAP_UNUSED(n, ...) \
> +     UK_FOREACH(n, __UK_ARGS_MAP_FN_UNUSED, __VA_ARGS__)
> +
> +
> +#if (defined(CONFIG_LIBUKDEBUG_TRACEPOINTS) &&                               
> \
> +     (defined(UK_DEBUG_TRACE) || defined(CONFIG_LIBUKDEBUG_ALL_TRACEPOINTS)))
> +#define ____UK_TRACEPOINT(n, regdata_name, trace_name, fmt, ...)     \
> +     __UK_TRACE_REG(n, regdata_name, trace_name, fmt,                \
> +                    __VA_ARGS__);                                    \
> +     static inline void trace_name(__UK_TRACE_ARGS_MAP_UNUSED(n, 
> __VA_ARGS__)) \
> +     {                                                               \
> +     }
> +#else
> +#define ____UK_TRACEPOINT(n, regdata_name, trace_name, fmt, ...)     \
> +     static inline void trace_name(                                  \
> +             __UK_TRACE_ARGS_MAP_UNUSED(n, __VA_ARGS__))             \
> +     {                                                               \
> +     }
> +#endif
> +
> +#define __UK_TRACEPOINT(n, regdata_name, trace_name, fmt, ...)       \
> +     ____UK_TRACEPOINT(n, regdata_name, trace_name, fmt,     \
> +                       __VA_ARGS__)
> +#define UK_TRACEPOINT(trace_name, fmt, ...)                          \
> +     __UK_TRACEPOINT(UK_NARGS(__VA_ARGS__),                          \
> +                     __ ## trace_name ## _regdata,                   \
> +                     trace_name, fmt, __VA_ARGS__)
> +
> +
>  #endif /* _UK_TRACE_H_ */
> 

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

 


Rackspace

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