Use au->lock exclusively for locking in async I/O.

2020-02-18  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/93599
	* io/async.c (destroy_adv_cond): Do not destroy lock.
	(async_io): Make sure au->lock is locked for finishing of thread.
	Do not lock/unlock around signalling emptysignal. Unlock au->lock
	before return.
	(init_adv_cond): Do not initialize lock.
	(enqueue_transfer): Unlock after signal.
	(enqueue_done_id): Likewise.
	(enqueue_done): Likewise.
	(enqueue_close): Likewise.
	(enqueue_data_transfer): Likewise.
	(async_wait_id): Do not lock/unlock around signalling au->work.
	(async_wait): Unlock after signal.
	* io/async.h (SIGNAL): Add comment about needed au->lock.
	Remove locking/unlocking of advcond->lock.
	(WAIT_SIGNAL_MUTEX): Add comment. Remove locking/unlocking of
	advcond->lock.  Unlock mutex only at the end.  Loop on
	__ghread_cond_wait returning zero.
	(REVOKE_SIGNAL): Add comment. Remove locking/unlocking of
	advcond->lock.
	(struct adv_cond): Remove mutex from struct.

asdf

diff --git a/libgfortran/ChangeLog b/libgfortran/ChangeLog
index 5632fbc8272b6db1573b09fa843bc4c944aefadf..bef6306fbc64408a19b03b398945e2845c629319 100644 (file)
--- a/libgfortran/ChangeLog
+++ b/libgfortran/ChangeLog
@@ -1,3 +1,27 @@
+2020-02-18  Thomas Koenig  <tkoenig@gcc.gnu.org>
+
+       PR fortran/93599
+       * io/async.c (destroy_adv_cond): Do not destroy lock.
+       (async_io): Make sure au->lock is locked for finishing of thread.
+       Do not lock/unlock around signalling emptysignal. Unlock au->lock
+       before return.
+       (init_adv_cond): Do not initialize lock.
+       (enqueue_transfer): Unlock after signal.
+       (enqueue_done_id): Likewise.
+       (enqueue_done): Likewise.
+       (enqueue_close): Likewise.
+       (enqueue_data_transfer): Likewise.
+       (async_wait_id): Do not lock/unlock around signalling au->work.
+       (async_wait): Unlock after signal.
+       * io/async.h (SIGNAL): Add comment about needed au->lock.
+       Remove locking/unlocking of advcond->lock.
+       (WAIT_SIGNAL_MUTEX): Add comment. Remove locking/unlocking of
+       advcond->lock.  Unlock mutex only at the end.  Loop on
+       __ghread_cond_wait returning zero.
+       (REVOKE_SIGNAL): Add comment. Remove locking/unlocking of
+       advcond->lock.
+       (struct adv_cond): Remove mutex from struct.
+
 2020-02-12  Sandra Loosemore  <sandra@codesourcery.com>
 
        PR libstdc++/79193

diff --git a/libgfortran/io/async.c b/libgfortran/io/async.c
index ab214af8a66ad74dacd06805e28510c9a937eb55..63b9158c0ba0ee661c3857e8e07acdff235a3ddb 100644 (file)
--- a/libgfortran/io/async.c
+++ b/libgfortran/io/async.c
@@ -80,7 +80,6 @@ update_pdt (st_parameter_dt **old, st_parameter_dt *new) {
 static void
 destroy_adv_cond (struct adv_cond *ac)
 {
-  T_ERROR (__gthread_mutex_destroy, &ac->lock);
   T_ERROR (__gthread_cond_destroy, &ac->signal);
 }
 
@@ -156,6 +155,7 @@ async_io (void *arg)
 
                case AIO_CLOSE:
                  NOTE ("Received AIO_CLOSE");
+                 LOCK (&au->lock);
                  goto finish_thread;
 
                default:
@@ -175,7 +175,6 @@ async_io (void *arg)
              else if (ctq->type == AIO_CLOSE)
                {
                  NOTE ("Received AIO_CLOSE during error condition");
-                 UNLOCK (&au->lock);
                  goto finish_thread;
                }
            }
@@ -189,9 +188,7 @@ async_io (void *arg)
       au->tail = NULL;
       au->head = NULL;
       au->empty = 1;
-      UNLOCK (&au->lock);
       SIGNAL (&au->emptysignal);
-      LOCK (&au->lock);
     }
  finish_thread:
   au->tail = NULL;
@@ -199,6 +196,7 @@ async_io (void *arg)
   au->empty = 1;
   SIGNAL (&au->emptysignal);
   free (ctq);
+  UNLOCK (&au->lock);
   return NULL;
 }
 
@@ -223,7 +221,6 @@ static void
 init_adv_cond (struct adv_cond *ac)
 {
   ac->pending = 0;
-  __GTHREAD_MUTEX_INIT_FUNCTION (&ac->lock);
   __GTHREAD_COND_INIT_FUNCTION (&ac->signal);
 }
 
@@ -279,8 +276,8 @@ enqueue_transfer (async_unit *au, transfer_args *arg, enum aio_do type)
   au->tail = tq;
   REVOKE_SIGNAL (&(au->emptysignal));
   au->empty = false;
-  UNLOCK (&au->lock);
   SIGNAL (&au->work);
+  UNLOCK (&au->lock);
 }
 
 /* Enqueue an st_write_done or st_read_done which contains an ID.  */
@@ -303,8 +300,8 @@ enqueue_done_id (async_unit *au, enum aio_do type)
   au->empty = false;
   ret = au->id.high++;
   NOTE ("Enqueue id: %d", ret);
-  UNLOCK (&au->lock);
   SIGNAL (&au->work);
+  UNLOCK (&au->lock);
   return ret;
 }
 
@@ -324,8 +321,8 @@ enqueue_done (async_unit *au, enum aio_do type)
   au->tail = tq;
   REVOKE_SIGNAL (&(au->emptysignal));
   au->empty = false;
-  UNLOCK (&au->lock);
   SIGNAL (&au->work);
+  UNLOCK (&au->lock);
 }
 
 /* Enqueue a CLOSE statement.  */
@@ -344,8 +341,8 @@ enqueue_close (async_unit *au)
   au->tail = tq;
   REVOKE_SIGNAL (&(au->emptysignal));
   au->empty = false;
-  UNLOCK (&au->lock);
   SIGNAL (&au->work);
+  UNLOCK (&au->lock);
 }
 
 /* The asynchronous unit keeps the currently active PDT around.
@@ -374,9 +371,9 @@ enqueue_data_transfer_init (async_unit *au, st_parameter_dt *dt, int read_flag)
     au->tail->next = tq;
   au->tail = tq;
   REVOKE_SIGNAL (&(au->emptysignal));
-  au->empty = 0;
-  UNLOCK (&au->lock);
+  au->empty = false;
   SIGNAL (&au->work);
+  UNLOCK (&au->lock);
 }
 
 /* Collect the errors that may have happened asynchronously.  Return true if
@@ -430,9 +427,7 @@ async_wait_id (st_parameter_common *cmp, async_unit *au, int i)
   NOTE ("Waiting for id %d", i);
   if (au->id.waiting < i)
     au->id.waiting = i;
-  UNLOCK (&au->lock);
   SIGNAL (&(au->work));
-  LOCK (&au->lock);
   WAIT_SIGNAL_MUTEX (&(au->id.done),
                     (au->id.low >= au->id.waiting || au->empty), &au->lock);
   LOCK (&au->lock);
@@ -454,8 +449,8 @@ async_wait (st_parameter_common *cmp, async_unit *au)
   if (cmp == NULL)
     cmp = au->error.cmp;
 
-  SIGNAL (&(au->work));
   LOCK (&(au->lock));
+  SIGNAL (&(au->work));
 
   if (au->empty)
     {

diff --git a/libgfortran/io/async.h b/libgfortran/io/async.h
index c6b2e0f94bda03561636ea885e99da55c4aff4d4..17d303c127b1a0dfee85568b96200417c5f3bcff 100644 (file)
--- a/libgfortran/io/async.h
+++ b/libgfortran/io/async.h
@@ -229,44 +229,44 @@
 
 #if ASYNC_IO
 
+/* au->lock has to be held when calling this macro.  */
+
 #define SIGNAL(advcond) do{                                            \
-    INTERN_LOCK (&(advcond)->lock);                                    \
     (advcond)->pending = 1;                                            \
     DEBUG_PRINTF ("%s%-75s %20s():%-5d %18p\n", aio_prefix, DEBUG_ORANGE "SIGNAL: " DEBUG_NORM \
                 #advcond, __FUNCTION__, __LINE__, (void *) advcond);   \
-    T_ERROR (__gthread_cond_broadcast, &(advcond)->signal);            \
-    INTERN_UNLOCK (&(advcond)->lock);                                  \
+    T_ERROR (__gthread_cond_broadcast, &(advcond)->signal);                    \
   } while (0)
 
+/* Has to be entered with mutex locked.  */
+
 #define WAIT_SIGNAL_MUTEX(advcond, condition, mutex) do{               \
     __label__ finish;                                                  \
-    INTERN_LOCK (&((advcond)->lock));                                  \
     DEBUG_PRINTF ("%s%-75s %20s():%-5d %18p\n", aio_prefix, DEBUG_BLUE "WAITING: " DEBUG_NORM \
                 #advcond, __FUNCTION__, __LINE__, (void *) advcond);   \
-    if ((advcond)->pending || (condition)){                            \
-      UNLOCK (mutex);                                                  \
+    if ((advcond)->pending || (condition))                             \
       goto finish;                                                     \
-    }                                                                  \
-    UNLOCK (mutex);                                                    \
-     while (!__gthread_cond_wait(&(advcond)->signal, &(advcond)->lock)) {      \
-       { int cond;                                                     \
-        LOCK (mutex); cond = condition; UNLOCK (mutex);        \
-          if (cond){                                                   \
-            DEBUG_PRINTF ("%s%-75s %20s():%-5d %18p\n", aio_prefix, DEBUG_ORANGE "REC: " DEBUG_NORM \
-                 #advcond,  __FUNCTION__, __LINE__, (void *)advcond);  \
-          break;                                                       \
-        }                                                      \
+    while (1)                                                          \
+      {                                                                        \
+       int err_ret = __gthread_cond_wait(&(advcond)->signal, mutex);   \
+       if (err_ret) internal_error (NULL, "WAIT_SIGNAL_MUTEX failed"); \
+       if (condition)                                                  \
+         {                                                             \
+           DEBUG_PRINTF ("%s%-75s %20s():%-5d %18p\n", aio_prefix, DEBUG_ORANGE \
+                         "REC: " DEBUG_NORM                            \
+                         #advcond,  __FUNCTION__, __LINE__, (void *)advcond); \
+           break;                                                      \
+         }                                                             \
       }                                                                        \
-    }                                                                  \
   finish:                                                              \
-                (advcond)->pending = 0;                                \
-                INTERN_UNLOCK (&((advcond)->lock));                    \
-                } while (0)
+    (advcond)->pending = 0;                                            \
+    UNLOCK (mutex);                                                    \
+  } while (0)
+
+/* au->lock has to be held when calling this macro.  */
 
 #define REVOKE_SIGNAL(advcond) do{             \
-    INTERN_LOCK (&(advcond)->lock);            \
     (advcond)->pending = 0;                    \
-    INTERN_UNLOCK (&(advcond)->lock);          \
   } while (0)
 
 #else
@@ -330,7 +330,6 @@ struct adv_cond
 {
 #if ASYNC_IO
   int pending;
-  __gthread_mutex_t lock;
   __gthread_cond_t signal;
 #endif
 };