diff mbox series

[ovs-dev,PATCHv2] fatal-signal: Log backtrace when no monitor daemon.

Message ID 1584978512-29875-1-git-send-email-u9012063@gmail.com
State Accepted
Commit ecd4a8fcdff2ebf35cdb36355167d34e99df6dd5
Headers show
Series [ovs-dev,PATCHv2] fatal-signal: Log backtrace when no monitor daemon. | expand

Commit Message

William Tu March 23, 2020, 3:48 p.m. UTC
Currently the backtrace logging is only available when monitor
daemon is running.  This patch enables backtrace logging when
no monitor daemon exists.  At signal handling context, it detects
whether monitor daemon exists.  If not, write directly the backtrace
to the vlog fd.  Note that using VLOG_* macro doesn't work due to
it's buffer I/O, so this patch directly issue write() syscall to
the file descriptor.

For some system we stop using monitor daemon and use systemd to
monitor ovs-vswitchd, thus need this patch. Example of
ovs-vswitchd.log (note that there is no timestamp printed):
  2020-03-23T14:42:12.949Z|00049|memory|INFO|175332 kB peak resident
  2020-03-23T14:42:12.949Z|00050|memory|INFO|handlers:2 ports:3 reva
  SIGSEGV detected, backtrace:
  0x0000000000486969 <fatal_signal_handler+0x49>
  0x00007f7f5e57f4b0 <killpg+0x40>
  0x000000000047daa8 <pmd_thread_main+0x238>
  0x0000000000504edd <ovsthread_wrapper+0x7d>
  0x00007f7f5f0476ba <start_thread+0xca>
  0x00007f7f5e65141d <clone+0x6d>
  0x0000000000000000 <+0x0>

Signed-off-by: William Tu <u9012063@gmail.com>
---
v2: check log_fd < 0 and return
    add example to the commit message
---
 include/openvswitch/vlog.h |  3 +++
 lib/daemon-private.h       |  1 +
 lib/daemon-unix.c          |  2 +-
 lib/fatal-signal.c         | 27 ++++++++++++++++++++++++++-
 lib/vlog.c                 |  6 ++++++
 5 files changed, 37 insertions(+), 2 deletions(-)

Comments

Ben Pfaff March 23, 2020, 4:30 p.m. UTC | #1
On Mon, Mar 23, 2020 at 08:48:32AM -0700, William Tu wrote:
> Currently the backtrace logging is only available when monitor
> daemon is running.  This patch enables backtrace logging when
> no monitor daemon exists.  At signal handling context, it detects
> whether monitor daemon exists.  If not, write directly the backtrace
> to the vlog fd.  Note that using VLOG_* macro doesn't work due to
> it's buffer I/O, so this patch directly issue write() syscall to
> the file descriptor.
> 
> For some system we stop using monitor daemon and use systemd to
> monitor ovs-vswitchd, thus need this patch. Example of
> ovs-vswitchd.log (note that there is no timestamp printed):
>   2020-03-23T14:42:12.949Z|00049|memory|INFO|175332 kB peak resident
>   2020-03-23T14:42:12.949Z|00050|memory|INFO|handlers:2 ports:3 reva
>   SIGSEGV detected, backtrace:
>   0x0000000000486969 <fatal_signal_handler+0x49>
>   0x00007f7f5e57f4b0 <killpg+0x40>
>   0x000000000047daa8 <pmd_thread_main+0x238>
>   0x0000000000504edd <ovsthread_wrapper+0x7d>
>   0x00007f7f5f0476ba <start_thread+0xca>
>   0x00007f7f5e65141d <clone+0x6d>
>   0x0000000000000000 <+0x0>
> 
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
> v2: check log_fd < 0 and return
>     add example to the commit message

Acked-by: Ben Pfaff <blp@ovn.org>
William Tu March 23, 2020, 4:43 p.m. UTC | #2
On Mon, Mar 23, 2020 at 9:30 AM Ben Pfaff <blp@ovn.org> wrote:
>
> On Mon, Mar 23, 2020 at 08:48:32AM -0700, William Tu wrote:
> > Currently the backtrace logging is only available when monitor
> > daemon is running.  This patch enables backtrace logging when
> > no monitor daemon exists.  At signal handling context, it detects
> > whether monitor daemon exists.  If not, write directly the backtrace
> > to the vlog fd.  Note that using VLOG_* macro doesn't work due to
> > it's buffer I/O, so this patch directly issue write() syscall to
> > the file descriptor.
> >
> > For some system we stop using monitor daemon and use systemd to
> > monitor ovs-vswitchd, thus need this patch. Example of
> > ovs-vswitchd.log (note that there is no timestamp printed):
> >   2020-03-23T14:42:12.949Z|00049|memory|INFO|175332 kB peak resident
> >   2020-03-23T14:42:12.949Z|00050|memory|INFO|handlers:2 ports:3 reva
> >   SIGSEGV detected, backtrace:
> >   0x0000000000486969 <fatal_signal_handler+0x49>
> >   0x00007f7f5e57f4b0 <killpg+0x40>
> >   0x000000000047daa8 <pmd_thread_main+0x238>
> >   0x0000000000504edd <ovsthread_wrapper+0x7d>
> >   0x00007f7f5f0476ba <start_thread+0xca>
> >   0x00007f7f5e65141d <clone+0x6d>
> >   0x0000000000000000 <+0x0>
> >
> > Signed-off-by: William Tu <u9012063@gmail.com>
> > ---
> > v2: check log_fd < 0 and return
> >     add example to the commit message
>
> Acked-by: Ben Pfaff <blp@ovn.org>

Applied to master, thanks
Ilya Maximets March 23, 2020, 5:42 p.m. UTC | #3
On 3/23/20 4:48 PM, William Tu wrote:
> Currently the backtrace logging is only available when monitor
> daemon is running.  This patch enables backtrace logging when
> no monitor daemon exists.  At signal handling context, it detects
> whether monitor daemon exists.  If not, write directly the backtrace
> to the vlog fd.  Note that using VLOG_* macro doesn't work due to
> it's buffer I/O, so this patch directly issue write() syscall to
> the file descriptor.
> 
> For some system we stop using monitor daemon and use systemd to
> monitor ovs-vswitchd, thus need this patch. Example of
> ovs-vswitchd.log (note that there is no timestamp printed):
>   2020-03-23T14:42:12.949Z|00049|memory|INFO|175332 kB peak resident
>   2020-03-23T14:42:12.949Z|00050|memory|INFO|handlers:2 ports:3 reva
>   SIGSEGV detected, backtrace:
>   0x0000000000486969 <fatal_signal_handler+0x49>
>   0x00007f7f5e57f4b0 <killpg+0x40>
>   0x000000000047daa8 <pmd_thread_main+0x238>
>   0x0000000000504edd <ovsthread_wrapper+0x7d>
>   0x00007f7f5f0476ba <start_thread+0xca>
>   0x00007f7f5e65141d <clone+0x6d>
>   0x0000000000000000 <+0x0>
> 
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---

Hi, William and Ben.
This patch broke clang build:

lib/vlog.c:618:12: error: reading variable 'log_fd' requires holding mutex
      'log_file_mutex' [-Werror,-Wthread-safety-analysis]
    return log_fd;
           ^

https://travis-ci.org/github/openvswitch/ovs/builds/665956070

Best regards, Ilya Maximets.
William Tu March 23, 2020, 6:54 p.m. UTC | #4
On Mon, Mar 23, 2020 at 10:42 AM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> On 3/23/20 4:48 PM, William Tu wrote:
> > Currently the backtrace logging is only available when monitor
> > daemon is running.  This patch enables backtrace logging when
> > no monitor daemon exists.  At signal handling context, it detects
> > whether monitor daemon exists.  If not, write directly the backtrace
> > to the vlog fd.  Note that using VLOG_* macro doesn't work due to
> > it's buffer I/O, so this patch directly issue write() syscall to
> > the file descriptor.
> >
> > For some system we stop using monitor daemon and use systemd to
> > monitor ovs-vswitchd, thus need this patch. Example of
> > ovs-vswitchd.log (note that there is no timestamp printed):
> >   2020-03-23T14:42:12.949Z|00049|memory|INFO|175332 kB peak resident
> >   2020-03-23T14:42:12.949Z|00050|memory|INFO|handlers:2 ports:3 reva
> >   SIGSEGV detected, backtrace:
> >   0x0000000000486969 <fatal_signal_handler+0x49>
> >   0x00007f7f5e57f4b0 <killpg+0x40>
> >   0x000000000047daa8 <pmd_thread_main+0x238>
> >   0x0000000000504edd <ovsthread_wrapper+0x7d>
> >   0x00007f7f5f0476ba <start_thread+0xca>
> >   0x00007f7f5e65141d <clone+0x6d>
> >   0x0000000000000000 <+0x0>
> >
> > Signed-off-by: William Tu <u9012063@gmail.com>
> > ---
>
> Hi, William and Ben.
> This patch broke clang build:
>
> lib/vlog.c:618:12: error: reading variable 'log_fd' requires holding mutex
>       'log_file_mutex' [-Werror,-Wthread-safety-analysis]
>     return log_fd;
>            ^
>
> https://travis-ci.org/github/openvswitch/ovs/builds/665956070
>
Thanks!
I'm working on the fix.
William
diff mbox series

Patch

diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
index 19da4ab62320..476bf3d6d5b4 100644
--- a/include/openvswitch/vlog.h
+++ b/include/openvswitch/vlog.h
@@ -143,6 +143,9 @@  void vlog_set_syslog_method(const char *method);
 /* Configure syslog target. */
 void vlog_set_syslog_target(const char *target);
 
+/* Return the log_fd. */
+int vlog_get_fd(void);
+
 /* Initialization. */
 void vlog_init(void);
 void vlog_enable_async(void);
diff --git a/lib/daemon-private.h b/lib/daemon-private.h
index 4e0667601001..2b90e004235c 100644
--- a/lib/daemon-private.h
+++ b/lib/daemon-private.h
@@ -20,6 +20,7 @@ 
 extern bool detach;
 extern char *pidfile;
 extern int daemonize_fd;
+extern bool monitor;
 
 char *make_pidfile_name(const char *name);
 
diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
index 6b2a5b9bd4e6..ae59ecf2c2b5 100644
--- a/lib/daemon-unix.c
+++ b/lib/daemon-unix.c
@@ -80,7 +80,7 @@  int daemonize_fd = -1;
 
 /* --monitor: Should a supervisory process monitor the daemon and restart it if
  * it dies due to an error signal? */
-static bool monitor;
+bool monitor;
 
 /* --user: Only root can use this option. Switch to new uid:gid after
  * initially running as root.  */
diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
index ba7f5bfd314d..4965c1ae82c0 100644
--- a/lib/fatal-signal.c
+++ b/lib/fatal-signal.c
@@ -187,7 +187,32 @@  send_backtrace_to_monitor(void) {
         dep++;
     }
 
-    ignore(write(daemonize_fd, unw_bt, dep * sizeof(struct unw_backtrace)));
+    if (monitor) {
+        ignore(write(daemonize_fd, unw_bt,
+                     dep * sizeof(struct unw_backtrace)));
+    } else {
+        /* Since there is no monitor daemon running, write backtrace
+         * in current process.  This is not asyn-signal-safe due to
+         * use of snprintf().
+         */
+        char str[] = "SIGSEGV detected, backtrace:\n";
+
+        if (vlog_get_fd() < 0) {
+            return;
+        }
+
+        ignore(write(vlog_get_fd(), str, strlen(str)));
+
+        for (int i = 0; i < dep; i++) {
+            char line[64];
+
+            snprintf(line, 64, "0x%016"PRIxPTR" <%s+0x%"PRIxPTR">\n",
+                     unw_bt[i].ip,
+                     unw_bt[i].func,
+                     unw_bt[i].offset);
+            ignore(write(vlog_get_fd(), line, strlen(line)));
+        }
+    }
 }
 #else
 static inline void
diff --git a/lib/vlog.c b/lib/vlog.c
index 559943d87937..502b33fc831e 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -612,6 +612,12 @@  vlog_set_syslog_target(const char *target)
     ovs_rwlock_unlock(&pattern_rwlock);
 }
 
+int
+vlog_get_fd(void)
+{
+    return log_fd;
+}
+
 /* Returns 'false' if 'facility' is not a valid string. If 'facility'
  * is a valid string, sets 'value' with the integer value of 'facility'
  * and returns 'true'. */