[newlib-cygwin] Cygwin: select: revamp non-polling code for signalfd

Corinna Vinschen corinna@sourceware.org
Sun Aug 18 12:03:00 GMT 2019


https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=7097b05eda2f8e9058eab4fda8dedacdfb7ffd7f

commit 7097b05eda2f8e9058eab4fda8dedacdfb7ffd7f
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Fri Aug 16 16:36:06 2019 +0200

    Cygwin: select: revamp non-polling code for signalfd
    
    Rather than waiting for signalfd_select_wait in a thread, which is racy,
    create a global event "my_pendingsigs_evt" which is set and reset by
    wait_sig depending only on the fact if blocked signals are pending or not.
    
    This in turn allows to WFMO on this event in select as soon as signalfds
    are present in the read descriptor set.  Select's peek and verify
    will then check if one of the present signalfds is affected.
    
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/cygtls.h       |  2 +-
 winsup/cygwin/exceptions.cc  |  8 -----
 winsup/cygwin/select.cc      | 80 +++-----------------------------------------
 winsup/cygwin/select.h       | 11 ++----
 winsup/cygwin/signal.cc      |  1 -
 winsup/cygwin/sigproc.cc     | 17 ++++++++++
 winsup/cygwin/tlsoffsets.h   | 16 ++++-----
 winsup/cygwin/tlsoffsets64.h | 16 ++++-----
 8 files changed, 40 insertions(+), 111 deletions(-)

diff --git a/winsup/cygwin/cygtls.h b/winsup/cygwin/cygtls.h
index 65a905c..a2e3676 100644
--- a/winsup/cygwin/cygtls.h
+++ b/winsup/cygwin/cygtls.h
@@ -189,8 +189,8 @@ public:
   stack_t altstack;
   siginfo_t *sigwait_info;
   HANDLE signal_arrived;
-  HANDLE signalfd_select_wait;
   bool will_wait_for_signal;
+  long __align;			/* Needed to align context to 16 byte. */
   /* context MUST be aligned to 16 byte, otherwise RtlCaptureContext fails.
      If you prepend cygtls members here, make sure context stays 16 byte
      aligned. */
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 1765f43..848f9bd 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1469,14 +1469,6 @@ sigpacket::process ()
   if (issig_wait)
     {
       tls->sigwait_mask = 0;
-      /* If the catching thread is running select on a signalfd, don't call
-	 the signal handler and don't remove the signal from the queue. */
-      if (tls->signalfd_select_wait)
-	{
-	  SetEvent (tls->signalfd_select_wait);
-	  rc = 0;
-	  goto done;
-	}
       goto dosig;
     }
 
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index 4e9256b..ac73e0a 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -1849,80 +1849,6 @@ fhandler_windows::select_except (select_stuff *ss)
   return s;
 }
 
-static int start_thread_signalfd (select_record *, select_stuff *);
-
-static DWORD WINAPI
-thread_signalfd (void *arg)
-{
-  select_signalfd_info *si = (select_signalfd_info *) arg;
-  bool event = false;
-
-  while (!event)
-    {
-      sigset_t set = 0;
-      _cygtls *tls = si->start->tls;
-
-      for (select_record *s = si->start; (s = s->next); )
-	if (s->startup == start_thread_signalfd)
-	  set |= ((fhandler_signalfd *) s->fh)->get_sigset ();
-      set_signal_mask (tls->sigwait_mask, set);
-      tls->signalfd_select_wait = si->evt;
-      sig_dispatch_pending (true);
-      switch (WaitForSingleObject (si->evt, INFINITE))
-	{
-	case WAIT_OBJECT_0:
-	  tls->signalfd_select_wait = NULL;
-	  event = true;
-	  break;
-	default:
-	  break;
-	}
-      if (si->stop_thread)
-	break;
-      if (!event)
-	Sleep (1L);
-    }
-  CloseHandle (si->evt);
-  return 0;
-}
-
-static int
-start_thread_signalfd (select_record *me, select_stuff *stuff)
-{
-  select_signalfd_info *si;
-
-  if ((si = stuff->device_specific_signalfd))
-    {
-      me->h = *si->thread;
-      return 1;
-    }
-  si = new select_signalfd_info;
-  si->start = &stuff->start;
-  si->stop_thread = false;
-  si->evt = CreateEventW (&sec_none_nih, TRUE, FALSE, NULL);
-  si->thread = new cygthread (thread_signalfd, si, "signalfdsel");
-  me->h = *si->thread;
-  stuff->device_specific_signalfd = si;
-  return 1;
-}
-
-static void
-signalfd_cleanup (select_record *, select_stuff *stuff)
-{
-  select_signalfd_info *si;
-
-  if (!(si = stuff->device_specific_signalfd))
-    return;
-  if (si->thread)
-    {
-      si->stop_thread = true;
-      SetEvent (si->evt);
-      si->thread->detach ();
-    }
-  delete si;
-  stuff->device_specific_signalfd = NULL;
-}
-
 static int
 peek_signalfd (select_record *me, bool)
 {
@@ -1942,17 +1868,19 @@ verify_signalfd (select_record *me, fd_set *rfds, fd_set *wfds, fd_set *efds)
   return peek_signalfd (me, true);
 }
 
+extern HANDLE my_pendingsigs_evt;
+
 select_record *
 fhandler_signalfd::select_read (select_stuff *stuff)
 {
   select_record *s = stuff->start.next;
   if (!s->startup)
     {
-      s->startup = start_thread_signalfd;
+      s->startup = no_startup;
       s->verify = verify_signalfd;
-      s->cleanup = signalfd_cleanup;
     }
   s->peek = peek_signalfd;
+  s->h = my_pendingsigs_evt;	/* wait_sig sets this if signal are pending */
   s->read_selected = true;
   s->read_ready = false;
   return s;
diff --git a/winsup/cygwin/select.h b/winsup/cygwin/select.h
index 19f9d7d..7d6dee7 100644
--- a/winsup/cygwin/select.h
+++ b/winsup/cygwin/select.h
@@ -71,12 +71,6 @@ struct select_serial_info: public select_info
   select_serial_info (): select_info () {}
 };
 
-struct select_signalfd_info: public select_info
-{
-  HANDLE evt;
-  select_signalfd_info (): select_info () {}
-};
-
 class select_stuff
 {
 public:
@@ -97,7 +91,6 @@ public:
   select_fifo_info *device_specific_fifo;
   select_socket_info *device_specific_socket;
   select_serial_info *device_specific_serial;
-  select_signalfd_info *device_specific_signalfd;
 
   bool test_and_set (int, fd_set *, fd_set *, fd_set *);
   int poll (fd_set *, fd_set *, fd_set *);
@@ -110,8 +103,8 @@ public:
 		   device_specific_pipe (NULL),
 		   device_specific_fifo (NULL),
 		   device_specific_socket (NULL),
-		   device_specific_serial (NULL),
-		   device_specific_signalfd (NULL) {}
+		   device_specific_serial (NULL)
+		   {}
 };
 
 extern "C" int cygwin_select (int , fd_set *, fd_set *, fd_set *,
diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc
index 920a533..8ac59d4 100644
--- a/winsup/cygwin/signal.cc
+++ b/winsup/cygwin/signal.cc
@@ -606,7 +606,6 @@ sigwait_common (const sigset_t *set, siginfo_t *info, PLARGE_INTEGER waittime)
   __try
     {
       set_signal_mask (_my_tls.sigwait_mask, *set);
-      _my_tls.signalfd_select_wait = NULL;
       sig_dispatch_pending (true);
 
       switch (cygwait (NULL, waittime,
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index 8003e2d..91abb71 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -57,6 +57,9 @@ _cygtls NO_COPY *_sig_tls;
 Static HANDLE my_sendsig;
 Static HANDLE my_readsig;
 
+/* Used in select if a signalfd is part of the read descriptor set */
+HANDLE NO_COPY my_pendingsigs_evt;
+
 /* Function declarations */
 static int __reg1 checkstate (waitq *);
 static __inline__ bool get_proc_lock (DWORD, DWORD);
@@ -455,6 +458,10 @@ sigproc_init ()
     }
   ProtectHandle (my_readsig);
   myself->sendsig = my_sendsig;
+  my_pendingsigs_evt = CreateEvent (NULL, TRUE, FALSE, NULL);
+  if (!my_pendingsigs_evt)
+    api_fatal ("couldn't create pending signal event, %E");
+
   /* sync_proc_subproc is used by proc_subproc.  It serializes
      access to the children and proc arrays.  */
   sync_proc_subproc.init ("sync_proc_subproc");
@@ -1398,6 +1405,16 @@ wait_sig (VOID *)
 		      qnext->si.si_signo = 0;
 		    }
 		}
+	      /* At least one signal still queued?  The event is used in select
+		 only, and only to decide if WFMO should wake up in case a
+		 signalfd is waiting via select/poll for being ready to read a
+		 pending signal.  This method wakes up all threads hanging in
+		 select and having a signalfd, as soon as a pending signal is
+		 available, but it's certainly better than constant polling. */
+	      if (sigq.start.next)
+		SetEvent (my_pendingsigs_evt);
+	      else
+		ResetEvent (my_pendingsigs_evt);
 	      if (pack.si.si_signo == SIGCHLD)
 		clearwait = true;
 	    }
diff --git a/winsup/cygwin/tlsoffsets.h b/winsup/cygwin/tlsoffsets.h
index 8003a1f..13d1003 100644
--- a/winsup/cygwin/tlsoffsets.h
+++ b/winsup/cygwin/tlsoffsets.h
@@ -29,10 +29,10 @@
 //; $tls::psigwait_info = 2852;
 //; $tls::signal_arrived = -9844;
 //; $tls::psignal_arrived = 2856;
-//; $tls::signalfd_select_wait = -9840;
-//; $tls::psignalfd_select_wait = 2860;
-//; $tls::will_wait_for_signal = -9836;
-//; $tls::pwill_wait_for_signal = 2864;
+//; $tls::will_wait_for_signal = -9840;
+//; $tls::pwill_wait_for_signal = 2860;
+//; $tls::__align = -9836;
+//; $tls::p__align = 2864;
 //; $tls::context = -9832;
 //; $tls::pcontext = 2868;
 //; $tls::thread_id = -9084;
@@ -91,10 +91,10 @@
 #define tls_psigwait_info (2852)
 #define tls_signal_arrived (-9844)
 #define tls_psignal_arrived (2856)
-#define tls_signalfd_select_wait (-9840)
-#define tls_psignalfd_select_wait (2860)
-#define tls_will_wait_for_signal (-9836)
-#define tls_pwill_wait_for_signal (2864)
+#define tls_will_wait_for_signal (-9840)
+#define tls_pwill_wait_for_signal (2860)
+#define tls___align (-9836)
+#define tls_p__align (2864)
 #define tls_context (-9832)
 #define tls_pcontext (2868)
 #define tls_thread_id (-9084)
diff --git a/winsup/cygwin/tlsoffsets64.h b/winsup/cygwin/tlsoffsets64.h
index 2e9d590..d137408 100644
--- a/winsup/cygwin/tlsoffsets64.h
+++ b/winsup/cygwin/tlsoffsets64.h
@@ -29,10 +29,10 @@
 //; $tls::psigwait_info = 4144;
 //; $tls::signal_arrived = -8648;
 //; $tls::psignal_arrived = 4152;
-//; $tls::signalfd_select_wait = -8640;
-//; $tls::psignalfd_select_wait = 4160;
-//; $tls::will_wait_for_signal = -8632;
-//; $tls::pwill_wait_for_signal = 4168;
+//; $tls::will_wait_for_signal = -8640;
+//; $tls::pwill_wait_for_signal = 4160;
+//; $tls::__align = -8632;
+//; $tls::p__align = 4168;
 //; $tls::context = -8624;
 //; $tls::pcontext = 4176;
 //; $tls::thread_id = -7328;
@@ -91,10 +91,10 @@
 #define tls_psigwait_info (4144)
 #define tls_signal_arrived (-8648)
 #define tls_psignal_arrived (4152)
-#define tls_signalfd_select_wait (-8640)
-#define tls_psignalfd_select_wait (4160)
-#define tls_will_wait_for_signal (-8632)
-#define tls_pwill_wait_for_signal (4168)
+#define tls_will_wait_for_signal (-8640)
+#define tls_pwill_wait_for_signal (4160)
+#define tls___align (-8632)
+#define tls_p__align (4168)
 #define tls_context (-8624)
 #define tls_pcontext (4176)
 #define tls_thread_id (-7328)



More information about the Cygwin-cvs mailing list