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

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





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

 


Rackspace

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