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 <ru@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 <igudger@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
