[PATCH] Core: remove unused FIOASYNC.
Ian Gudger
igudger at google.com
Wed Jun 27 20:40:26 UTC 2018
Sorry, I understand now.
Here is a new patch which removes that too:
# HG changeset patch
# User Ian Gudger <igudger at google.com>
# Date 1529449008 25200
# Tue Jun 19 15:56:48 2018 -0700
# Node ID 8fd0b85081a1cb91fa4495258bb5f9d3a6ef5785
# Parent 118885f7a5774962f1145693d9c26a4c199ca6ea
Core: remove FIOASYNC as the SIGIOs it generated were ignored.
FIOASYNC and F_SETOWN cause a pid or pgid to receive signals when a file is
ready for IO. When using master process mode, this was setup, but the SIGIO
signals were ignored. This has been the case since use of FIOASYNC was first
added in eaf1f651cf86. Logic ignore the SIGIOs in a case where they
unintentionally did something was added in 8abb88374c6c.
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";
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;
@@ -77,7 +76,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;
@@ -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));
@@ -134,13 +132,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,9 +161,6 @@ 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);
-
if (ngx_reap) {
ngx_reap = 0;
ngx_log_debug0(NGX_LOG_DEBUG_EVENT, cycle->log, 0, "reap
children");
@@ -184,13 +177,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 {
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;
On Wed, Jun 27, 2018 at 11:31 AM Ruslan Ermilov <ru at nginx.com> wrote:
>
> 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 at 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 at 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
More information about the nginx-devel
mailing list