On 10/19/22 11:31, Fengqi Li wrote:
> When segmentation fault occured in ovn-northd, the ovn-northd
> daemon process will try to restart for every 10s.
> Remove the segmentation fault condation,the ovn-northd daemon
> process becomes active again,but fatal_signal fds are not recreated
> resulting in ovn-northd 100% cpu utilization.

Hi, could you, please, add a bit more technical information
to the commit message?  As if, what exactly is causing the
high CPU usage?

I'm guessing that the monitor thread is constantly waking up
due to data being available in the signal_fds[0].  Is that
correct?

If so, we should probably just read it instead of re-creating
the pipe.  The pipe itself should still be usable.

Also, I don't remember seeing that issue and the code is there
for ages.  Is there some specific condition needed to trigger?

OTOH, signal_fds are not really supposed to be used for
communication between different processes.  It's a side effect
of forking that we have the same pipe shared, but it's not a
huge deal, since it's not really used for anything except for
signalling for attention.

Saying that, another solution would be to re-create this pipe
in the fatal_signal_fork() function, so the child doesn't use
the same pipe in the first place.

Best regards, Ilya Maximets.

> 
> Signed-off-by: Fengqi Li <[email protected]>
> ---
>  lib/daemon-unix.c  |  3 +++
>  lib/fatal-signal.c | 13 +++++++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> index 1a7ba427d..95d4f6626 100644
> --- a/lib/daemon-unix.c
> +++ b/lib/daemon-unix.c
> @@ -95,6 +95,8 @@ static int lock_pidfile(FILE *, int command);
>  static pid_t fork_and_clean_up(void);
>  static void daemonize_post_detach(void);
>  
> +extern bool fs_inited;
> +
>  /* Returns the file name that would be used for a pidfile if 'name' were
>   * provided to set_pidfile().  The caller must free the returned string. */
>  char *
> @@ -398,6 +400,7 @@ monitor_daemon(pid_t daemon_pid)
>                  log_received_backtrace(daemonize_fd);
>                  close(daemonize_fd);
>                  daemonize_fd = -1;
> +                fs_inited = false;
>  
>                  /* Throttle restarts to no more than once every 10 seconds. 
> */
>                  if (time(NULL) < last_restart + 10) {
> diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
> index bbb31ef27..8cd3e1dd9 100644
> --- a/lib/fatal-signal.c
> +++ b/lib/fatal-signal.c
> @@ -66,6 +66,7 @@ static size_t n_hooks;
>  
>  static int signal_fds[2];
>  static volatile sig_atomic_t stored_sig_nr = SIG_ATOMIC_MAX;
> +bool fs_inited = false;
>  
>  #ifdef _WIN32
>  static HANDLE wevent;
> @@ -85,16 +86,20 @@ static BOOL WINAPI ConsoleHandlerRoutine(DWORD 
> dwCtrlType);
>  void
>  fatal_signal_init(void)
>  {
> -    static bool inited = false;
> -
> -    if (!inited) {
> +    if (!fs_inited) {
>          size_t i;
>  
>          assert_single_threaded();
> -        inited = true;
> +        fs_inited = true;
>  
>          ovs_mutex_init_recursive(&mutex);
>  #ifndef _WIN32
> +        if (signal_fds[0]) {
> +            close(signal_fds[0]);
> +        }
> +        if (signal_fds[1]) {
> +            close(signal_fds[1]);
> +        }
>          xpipe_nonblocking(signal_fds);
>  #else
>          wevent = CreateEvent(NULL, TRUE, FALSE, NULL);

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to