Re: [Minios-devel] [UNIKRAFT PATCH 2/2] lib/ukdebug: Add time and thread info to debugs

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 16.03.2018 07:44, Dafna Hirschfeld wrote:
> 1. Add timestamp - the number of seconds since boot,
> to the debug messages.
>
> 2. Add the name of the thread to the debug messages.
> If the thread's name is NULL, print the pointer to it's
> struct. If it has no name (the boot process for example)
> then the thread info is not printed.
>
> Both timestamp and thread name can be enabled/disabled.
Probably you should split these changes into two separate commits. It 
seems that these changes are independent to each other.
> Signed-off-by: Dafna Hirschfeld <dafna3@xxxxxxxxx>
> ---
>   lib/ukdebug/Config.uk | 10 ++++++++++
>   lib/ukdebug/print.c   | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 60 insertions(+)
>
> diff --git a/lib/ukdebug/Config.uk b/lib/ukdebug/Config.uk
> index d2bc02b..2a1ea7e 100644
> --- a/lib/ukdebug/Config.uk
> +++ b/lib/ukdebug/Config.uk
> @@ -39,6 +39,16 @@ config LIBUKDEBUG_PRINTD_CRIT
>         bool "Show critical messages only"
>   endchoice
>   
> +config LIBUKDEBUG_PRINTD_TIME
> +       bool "Show timestamp in debug messages"
> +       default y
> +       depends on LIBUKDEBUG_PRINTD
> +
> +config LIBUKDEBUG_PRINTD_THREAD
> +       bool "Show name of thread in debug messages"
> +       default y
> +       depends on LIBUKDEBUG_PRINTD && LIBUKSCHED
> +
>   choice
>         prompt "Message redirection"
>         default LIBUKDEBUG_NOREDIR
> diff --git a/lib/ukdebug/print.c b/lib/ukdebug/print.c
> index 175baaf..1bf2825 100644
> --- a/lib/ukdebug/print.c
> +++ b/lib/ukdebug/print.c
> @@ -42,8 +42,13 @@
>   #include <stdarg.h>
>   
>   #include <uk/plat/console.h>
> +#include <uk/plat/time.h>
>   #include <uk/print.h>
> +#if LIBUKSCHED
> +#include <uk/thread.h>
> +#endif
>   #include <uk/arch/lcpu.h>
> +#include <uk/errptr.h>
>   
>   /*
>    * Note: Console redirection is implemented in this file. All pre-compiled 
> code
> @@ -75,6 +80,44 @@ static inline void _vprintk(const char *fmt, va_list ap)
>   #define _ukplat_coutd(lbuf, len) ukplat_coutd((lbuf), (len))
>   #endif
>   
> +#if LIBUKDEBUG_PRINTD_TIME
> +static void _printd_timestamp(void)
> +{
> +       char buf[BUFLEN];
> +       int len;
> +       __nsec nansec =  ukplat_monotonic_clock();
> +       __nsec sec = ukarch_time_nsec_to_sec(nansec);
> +       __nsec rem_usec = ukarch_time_subseconds(nansec);
> +
> +       rem_usec = ukarch_time_nsec_to_usec(rem_usec);
> +       len = snprintf(buf, BUFLEN, "[%5lu.%06lu] ", sec, rem_usec);
Hum, %lu works on x86 64 bit architecture since __nsec is a 64bit 
integer type but on 32 bit the value has to be printed with %llu.
My suggestion is to introduce a __PRInsec and __PRIsnsec macro next to 
the __nsec datatype definition in uk/arch/time.h. These macros should 
use __PRIu64 and __PRIs64 to get the right format definition for 
printing a 64 bit integer.
> +       _ukplat_coutd((char *)buf, len);
> +}
> +#endif > +
> +#if LIBUKDEBUG_PRINTD_THREAD
> +static void _printd_thread(void)
> +{
> +       struct uk_thread *thread;
> +
> +       thread = uk_thread_current();
> +       if (!PTRISERR(thread)) {
> +               if (thread->name) {
> +                       _ukplat_coutd("<", 1);
> +                       _ukplat_coutd((char *)thread->name,
> +                                       strlen(thread->name));
> +                       _ukplat_coutd("> ", 2);
> +               } else {
> +                       char buf[BUFLEN];
> +                       int len;
> +
> +                       len = snprintf(buf, BUFLEN, "<%p> ", thread);
> +                       _ukplat_coutd((char *)buf, len);
> +               }
> +       }
Hum, maybe we want to print the thread pointer value also in the error 
case. This extra information may be useful in some debugging situations. 
But this is personal preference - other from this, this is fine.
> +}
> +#endif
> +
>   static inline void _vprintd(int lvl, const char *libname, const char *srcname,
>                             unsigned int srcline, const char *fmt, va_list ap)
>   {
> @@ -129,7 +172,14 @@ static inline void _vprintd(int lvl, const char *libname, 
> const char *srcname,
>         lptr = lbuf;
>         while (len > 0) {
>                 if (newline) {
> +#if LIBUKDEBUG_PRINTD_TIME
> +                       _printd_timestamp();
> +#endif
>                         _ukplat_coutd(DECONST(char *, msghdr), 6);
> +#if LIBUKDEBUG_PRINTD_THREAD
> +                       _printd_thread();
> +#endif
> +
>                         if (libname) {
>                                 _ukplat_coutd("[", 1);
>                                 _ukplat_coutd(DECONST(char *, libname),
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel