On Wed, Jun 27, 2018 at 10:09:47AM -0700, Ian Gudger wrote: > Actually, as far as I can tell, it never did anything other than cause > signals to be delivered that were promptly ignored. It appears to have > been added in eaf1f651cf86.
I came to the same conclusion, but I'll double check with Igor before proceeding with removing this. > There are two things in ngx_master_process_cycle() with names related > to SIGIO. One is adding SIGIO to the set. That is removed in this > patch. The other is a variable named sigio, added in 8abb88374c6c. > > This variable does not appear to have anything to do with SIGIO > despite the name. It is indeed related, please see the explanation in the above mentioned commit here: http://hg.nginx.org/nginx/rev/8abb88374c6c What I was trying to say is that your patch needs to revert this change as well: diff --git a/src/os/unix/ngx_process_cycle.c b/src/os/unix/ngx_process_cycle.c --- a/src/os/unix/ngx_process_cycle.c +++ b/src/os/unix/ngx_process_cycle.c @@ -77,7 +77,7 @@ ngx_master_process_cycle(ngx_cycle_t *cy u_char *p; size_t size; ngx_int_t i; - ngx_uint_t n, sigio; + ngx_uint_t n; sigset_t set; struct itimerval itv; ngx_uint_t live; @@ -134,13 +134,11 @@ ngx_master_process_cycle(ngx_cycle_t *cy ngx_new_binary = 0; delay = 0; - sigio = 0; live = 1; for ( ;; ) { if (delay) { if (ngx_sigalrm) { - sigio = 0; delay *= 2; ngx_sigalrm = 0; } @@ -165,8 +163,7 @@ ngx_master_process_cycle(ngx_cycle_t *cy ngx_time_update(); - ngx_log_debug1(NGX_LOG_DEBUG_EVENT, cycle->log, 0, - "wake up, sigio %i", sigio); + ngx_log_debug0(NGX_LOG_DEBUG_EVENT, cycle->log, 0, "wake up"); if (ngx_reap) { ngx_reap = 0; @@ -184,13 +181,6 @@ ngx_master_process_cycle(ngx_cycle_t *cy delay = 50; } - if (sigio) { - sigio--; - continue; - } - - sigio = ccf->worker_processes + 2 /* cache processes */; - if (delay > 1000) { ngx_signal_worker_processes(cycle, SIGKILL); } else { > I ran the tests with this patch and they all passed. Receiving signals > isn't free, so this patch may improve performance. Highly unlikely in this particular case. If you want, you can update your patch. > On Wed, Jun 27, 2018 at 5:57 AM Ruslan Ermilov <r...@nginx.com> wrote: > > > > On Mon, Jun 25, 2018 at 11:16:12AM -0700, Ian Gudger via nginx-devel wrote: > > > # HG changeset patch > > > # User Ian Gudger <igud...@google.com> > > > # Date 1529449008 25200 > > > # Tue Jun 19 15:56:48 2018 -0700 > > > # Node ID 9427538acbc50142afbe91a11a1d4f907a00d257 > > > # Parent 118885f7a5774962f1145693d9c26a4c199ca6ea > > > Core: remove unused FIOASYNC. > > > > > > FIOASYNC, F_SETOWN and SIGIO seem to no longer serve any function. > > > > Can you decode your "seem to no longer server any function", please? > > > > > diff --git a/src/os/unix/ngx_process.c b/src/os/unix/ngx_process.c > > > --- a/src/os/unix/ngx_process.c > > > +++ b/src/os/unix/ngx_process.c > > > @@ -71,8 +71,6 @@ ngx_signal_t signals[] = { > > > > > > { SIGINT, "SIGINT", "", ngx_signal_handler }, > > > > > > - { SIGIO, "SIGIO", "", ngx_signal_handler }, > > > - > > > { SIGCHLD, "SIGCHLD", "", ngx_signal_handler }, > > > > > > { SIGSYS, "SIGSYS, SIG_IGN", "", NULL }, > > > @@ -87,7 +85,6 @@ ngx_pid_t > > > ngx_spawn_process(ngx_cycle_t *cycle, ngx_spawn_proc_pt proc, void *data, > > > char *name, ngx_int_t respawn) > > > { > > > - u_long on; > > > ngx_pid_t pid; > > > ngx_int_t s; > > > > > > @@ -142,21 +139,6 @@ ngx_spawn_process(ngx_cycle_t *cycle, ng > > > return NGX_INVALID_PID; > > > } > > > > > > - on = 1; > > > - if (ioctl(ngx_processes[s].channel[0], FIOASYNC, &on) == -1) { > > > - ngx_log_error(NGX_LOG_ALERT, cycle->log, ngx_errno, > > > - "ioctl(FIOASYNC) failed while spawning > > > \"%s\"", name); > > > - ngx_close_channel(ngx_processes[s].channel, cycle->log); > > > - return NGX_INVALID_PID; > > > - } > > > - > > > - if (fcntl(ngx_processes[s].channel[0], F_SETOWN, ngx_pid) == -1) > > > { > > > - ngx_log_error(NGX_LOG_ALERT, cycle->log, ngx_errno, > > > - "fcntl(F_SETOWN) failed while spawning > > > \"%s\"", name); > > > - ngx_close_channel(ngx_processes[s].channel, cycle->log); > > > - return NGX_INVALID_PID; > > > - } > > > - > > > if (fcntl(ngx_processes[s].channel[0], F_SETFD, FD_CLOEXEC) == > > > -1) { > > > ngx_log_error(NGX_LOG_ALERT, cycle->log, ngx_errno, > > > "fcntl(FD_CLOEXEC) failed while spawning > > > \"%s\"", > > > @@ -394,10 +376,6 @@ ngx_signal_handler(int signo, siginfo_t > > > ngx_sigalrm = 1; > > > break; > > > > > > - case SIGIO: > > > - ngx_sigio = 1; > > > - break; > > > - > > > case SIGCHLD: > > > ngx_reap = 1; > > > break; > > > @@ -433,8 +411,6 @@ ngx_signal_handler(int signo, siginfo_t > > > > > > case ngx_signal_value(NGX_RECONFIGURE_SIGNAL): > > > case ngx_signal_value(NGX_CHANGEBIN_SIGNAL): > > > - case SIGIO: > > > - action = ", ignoring"; > > > > Removing setting of an "action" variable looks like an error. > > > > > break; > > > } > > > > > > diff --git a/src/os/unix/ngx_process_cycle.c > > > b/src/os/unix/ngx_process_cycle.c > > > --- a/src/os/unix/ngx_process_cycle.c > > > +++ b/src/os/unix/ngx_process_cycle.c > > > @@ -34,7 +34,6 @@ ngx_pid_t ngx_pid; > > > ngx_pid_t ngx_parent; > > > > > > sig_atomic_t ngx_reap; > > > -sig_atomic_t ngx_sigio; > > > sig_atomic_t ngx_sigalrm; > > > sig_atomic_t ngx_terminate; > > > sig_atomic_t ngx_quit; > > > @@ -88,7 +87,6 @@ ngx_master_process_cycle(ngx_cycle_t *cy > > > sigemptyset(&set); > > > sigaddset(&set, SIGCHLD); > > > sigaddset(&set, SIGALRM); > > > - sigaddset(&set, SIGIO); > > > sigaddset(&set, SIGINT); > > > sigaddset(&set, ngx_signal_value(NGX_RECONFIGURE_SIGNAL)); > > > sigaddset(&set, ngx_signal_value(NGX_REOPEN_SIGNAL)); > > > diff --git a/src/os/unix/ngx_process_cycle.h > > > b/src/os/unix/ngx_process_cycle.h > > > --- a/src/os/unix/ngx_process_cycle.h > > > +++ b/src/os/unix/ngx_process_cycle.h > > > @@ -47,7 +47,6 @@ extern ngx_uint_t ngx_daemonized; > > > extern ngx_uint_t ngx_exiting; > > > > > > extern sig_atomic_t ngx_reap; > > > -extern sig_atomic_t ngx_sigio; > > > extern sig_atomic_t ngx_sigalrm; > > > extern sig_atomic_t ngx_quit; > > > extern sig_atomic_t ngx_debug_quit; > > > > There's also a SIGIO related code in ngx_master_process_cycle(), > > added in 8abb88374c6c. > -- Ruslan Ermilov Assume stupidity not malice _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel