[PATCH] Core: remove unused FIOASYNC.
Ruslan Ermilov
ru at nginx.com
Wed Jul 11 14:21:03 UTC 2018
On Sun, Jul 08, 2018 at 11:36:45PM -0700, Ian Gudger wrote:
> Any update on this?
The original idea with SIGIO is to wake up master process from
sigsuspend() when the message channel is ready for I/O. This
feature is currently de-facto unused because if message writing
fails, the message is lost and not resent. If this happens in
ngx_signal_worker_processes(), the real signal is sent to the
worker process as a backup option. If this happens when passing
descriptors of inter-worker channels in ngx_pass_open_channel(),
there is no backup, but this is not a problem as inter-worker
channels aren't currently used.
We decided not to commit this patch now because we have plans
(https://trac.nginx.org/nginx/ticket/376) to pass descriptors
of re-opened log files via the master-to-worker channels, but
with reliable delivery (as opposed to the current use cases
described above). While using poll() for this could be one
of the options, the existing SIGIO mechanism is another one.
> On Thu, Jun 28, 2018 at 4:47 AM Ruslan Ermilov <ru at nginx.com> wrote:
> >
> > On Thu, Jun 28, 2018 at 12:27:40PM +0300, Ruslan Ermilov wrote:
> > > On Wed, Jun 27, 2018 at 01:40:26PM -0700, Ian Gudger wrote:
> > > > 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
> > > [...]
> > > > @@ -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;
> > > > }
> > >
> > > On Wed, Jun 27, 2018 at 03:57:05PM +0300, Ruslan Ermilov wrote:
> > > > Removing setting of an "action" variable looks like an error.
> > >
> > > No need to resend the patch.
> >
> > Here's a slightly cleaned up patch and commit log:
> >
> > # HG changeset patch
> > # User Ian Gudger <igudger at google.com>
> > # Date 1529449008 25200
> > # Tue Jun 19 15:56:48 2018 -0700
> > # Node ID 9d24aafa6626f2915176e80e5279704af6f6d575
> > # Parent f2396ecf608bab9acc0545e3e53e36cc2cb9b2e6
> > Core: removed 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 I/O. When using master process mode, this was set up, but the SIGIO
> > signals were ignored. This has been the case since use of FIOASYNC was first
> > added in eaf1f651cf86. Logic to 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,7 +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,8 +161,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 +179,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;
>
--
Ruslan Ermilov
Assume stupidity not malice
More information about the nginx-devel
mailing list