Changeset 22226e4

Timestamp:

Mar 18, 2022, 12:42:39 PM (3 years ago)

Author:

Thierry Delisle <tdelisle@…>

Branches:

ADT, ast-experimental, enum, master, pthread-emulation, qualifiedEnum

Children:

0b4ddb71, 51239d1b

Parents:

3bc69f2

Message:

Tentative fix for spurious deadlock in some concurrency tests

Location:

libcfa/src/concurrency

Files:

• io/setup.cfa (modified) (3 diffs)
• kernel.cfa (modified) (11 diffs)
• kernel.hfa (modified) (3 diffs)
• kernel/startup.cfa (modified) (9 diffs)

libcfa/src/concurrency/io/setup.cfa

@@ -115 +115 @@
                 this.ext_sq.empty = true;
                 (this.ext_sq.queue){};
-                __io_uring_setup( this, cl.io.params, proc->idle_fd );
+                __io_uring_setup( this, cl.io.params, proc->idle_wctx.evfd );
                 __cfadbg_print_safe(io_core, "Kernel I/O : Created ring for io_context %u (%p)\n", this.fd, &this);
         }
@@ -125 +125 @@
                 __cfadbg_print_safe(io_core, "Kernel I/O : Destroyed ring for io_context %u\n", this.fd);
         }
-
-        extern void __disable_interrupts_hard();
-        extern void __enable_interrupts_hard();
 
         static void __io_uring_setup( $io_context & this, const io_context_params & params_in, int procfd ) {
@@ -230 +227 @@
                         __cfadbg_print_safe(io_core, "Kernel I/O : registering %d for completion with ring %d\n", procfd, fd);
 
-                        __disable_interrupts_hard();
-
                         int ret = syscall( __NR_io_uring_register, fd, IORING_REGISTER_EVENTFD, &procfd, 1);
                         if (ret < 0) {
                                 abort("KERNEL ERROR: IO_URING EVENTFD REGISTER - %s\n", strerror(errno));
                         }
-
-                        __enable_interrupts_hard();
 
                         __cfadbg_print_safe(io_core, "Kernel I/O : registered %d for completion with ring %d\n", procfd, fd);

libcfa/src/concurrency/kernel.cfa

@@ -136 +136 @@
 static void mark_awake(__cluster_proc_list & idles, processor & proc);
 
-extern void __cfa_io_start( processor * );
 extern bool __cfa_io_drain( processor * );
 extern bool __cfa_io_flush( processor *, int min_comp );
-extern void __cfa_io_stop ( processor * );
 static inline bool __maybe_io_drain( processor * );
 
@@ -164 +162 @@
         verify(this);
 
-        io_future_t future; // used for idle sleep when io_uring is present
-        future.self.ptr = 1p;  // mark it as already fulfilled so we know if there is a pending request or not
-        eventfd_t idle_val;
-        iovec idle_iovec = { &idle_val, sizeof(idle_val) };
-
-        __cfa_io_start( this );
+        /* paranoid */ verify( this->idle_wctx.ftr   != 0p );
+        /* paranoid */ verify( this->idle_wctx.rdbuf != 0p );
+
+        // used for idle sleep when io_uring is present
+        // mark it as already fulfilled so we know if there is a pending request or not
+        this->idle_wctx.ftr->self.ptr = 1p;
+        iovec idle_iovec = { this->idle_wctx.rdbuf, sizeof(eventfd_t) };
 
         __cfadbg_print_safe(runtime_core, "Kernel : core %p starting\n", this);
@@ -236 +235 @@
                                 }
 
-                                idle_sleep( this, future, idle_iovec );
+                                idle_sleep( this, *this->idle_wctx.ftr, idle_iovec );
 
                                 // We were woken up, remove self from idle
@@ -264 +263 @@
                 __cfadbg_print_safe(runtime_core, "Kernel : core %p stopping\n", this);
         }
-
-        for(int i = 0; !available(future); i++) {
-                if(i > 1000) __cfaabi_dbg_write( "ERROR: kernel has bin spinning on a flush after exit loop.\n", 60);
-                __cfa_io_flush( this, 1 );
-        }
-
-        __cfa_io_stop( this );
 
         post( this->terminated );
@@ -639 +631 @@
 
         int fd = 1;
-        if( __atomic_load_n(&fdp->fd, __ATOMIC_SEQ_CST) != 1 ) {
-                fd = __atomic_exchange_n(&fdp->fd, 1, __ATOMIC_RELAXED);
+        if( __atomic_load_n(&fdp->sem, __ATOMIC_SEQ_CST) != 1 ) {
+                fd = __atomic_exchange_n(&fdp->sem, 1, __ATOMIC_RELAXED);
         }
 
@@ -682 +674 @@
         __cfadbg_print_safe(runtime_core, "Kernel : waking Processor %p\n", this);
 
-        this->idle_wctx.fd = 1;
+        this->idle_wctx.sem = 1;
 
         eventfd_t val;
         val = 1;
-        eventfd_write( this->idle_fd, val );
+        eventfd_write( this->idle_wctx.evfd, val );
 
         /* paranoid */ verify( ! __preemption_enabled() );
@@ -694 +686 @@
         // Tell everyone we are ready to go do sleep
         for() {
-                int expected = this->idle_wctx.fd;
+                int expected = this->idle_wctx.sem;
 
                 // Someone already told us to wake-up! No time for a nap.
@@ -700 +692 @@
 
                 // Try to mark that we are going to sleep
-                if(__atomic_compare_exchange_n(&this->idle_wctx.fd, &expected, this->idle_fd, false,  __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST) ) {
+                if(__atomic_compare_exchange_n(&this->idle_wctx.sem, &expected, this->idle_wctx.evfd, false,  __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST) ) {
                         // Every one agreed, taking a nap
                         break;
@@ -718 +710 @@
                 {
                         eventfd_t val;
-                        ssize_t ret = read( this->idle_fd, &val, sizeof(val) );
+                        ssize_t ret = read( this->idle_wctx.evfd, &val, sizeof(val) );
                         if(ret < 0) {
                                 switch((int)errno) {
@@ -745 +737 @@
                         reset(future);
 
-                        __kernel_read(this, future, iov, this->idle_fd );
+                        __kernel_read(this, future, iov, this->idle_wctx.evfd );
                 }
 
@@ -755 +747 @@
         __STATS__(true, ready.sleep.halts++; )
 
-        proc.idle_wctx.fd = 0;
+        proc.idle_wctx.sem = 0;
 
         /* paranoid */ verify( ! __preemption_enabled() );

libcfa/src/concurrency/kernel.hfa

@@ -48 +48 @@
 extern struct cluster * mainCluster;
 
-// Processor id, required for scheduling threads
-
-
+// Coroutine used py processors for the 2-step context switch
 coroutine processorCtx_t {
         struct processor * proc;
 };
 
-
+struct io_future_t;
+
+// Information needed for idle sleep
 struct __fd_waitctx {
-        volatile int fd;
+        // semaphore/future like object
+        // values can be 0, 1 or some file descriptor.
+        // 0 - is the default state
+        // 1 - means the proc should wake-up immediately
+        // FD - means the proc is going asleep and should be woken by writing to the FD.
+        volatile int sem;
+
+        // The event FD that corresponds to this processor
+        int evfd;
+
+        // buffer into which the proc will read from evfd
+        // unused if not using io_uring for idle sleep
+        void * rdbuf;
+
+        // future use to track the read of the eventfd
+        // unused if not using io_uring for idle sleep
+        io_future_t * ftr;
 };
 
@@ -103 +119 @@
         bool pending_preemption;
 
-        // Idle lock (kernel semaphore)
-        int idle_fd;
-
-        // Idle waitctx
+        // context for idle sleep
         struct __fd_waitctx idle_wctx;
 
@@ -168 +181 @@
         volatile unsigned id;
 };
-
-// //TODO adjust cache size to ARCHITECTURE
-// // Structure holding the ready queue
-// struct __ready_queue_t {
-//      // Data tracking the actual lanes
-//      // On a seperate cacheline from the used struct since
-//      // used can change on each push/pop but this data
-//      // only changes on shrink/grow
-//      struct {
-//              // Arary of lanes
-//              __intrusive_lane_t * volatile data;
-
-//              __cache_id_t * volatile caches;
-
-//              // Number of lanes (empty or not)
-//              volatile size_t count;
-//      } lanes;
-// };
-
-// void  ?{}(__ready_queue_t & this);
-// void ^?{}(__ready_queue_t & this);
 
 // Idle Sleep

libcfa/src/concurrency/kernel/startup.cfa

@@ -33 +33 @@
 // CFA Includes
 #include "kernel/private.hfa"
+#include "iofwd.hfa"
 #include "startup.hfa"                                  // STARTUP_PRIORITY_XXX
 #include "limits.hfa"
@@ -97 +98 @@
 extern void __kernel_alarm_startup(void);
 extern void __kernel_alarm_shutdown(void);
+extern void __cfa_io_start( processor * );
+extern void __cfa_io_stop ( processor * );
 
 //-----------------------------------------------------------------------------
@@ -111 +114 @@
 KERNEL_STORAGE(__stack_t,            mainThreadCtx);
 KERNEL_STORAGE(__scheduler_RWLock_t, __scheduler_lock);
+KERNEL_STORAGE(eventfd_t,            mainIdleEventFd);
+KERNEL_STORAGE(io_future_t,          mainIdleFuture);
 #if !defined(__CFA_NO_STATISTICS__)
 KERNEL_STORAGE(__stats_t, mainProcStats);
@@ -224 +229 @@
         (*mainProcessor){};
 
+        mainProcessor->idle_wctx.rdbuf = &storage_mainIdleEventFd;
+        mainProcessor->idle_wctx.ftr   = (io_future_t*)&storage_mainIdleFuture;
+        /* paranoid */ verify( sizeof(storage_mainIdleEventFd) == sizeof(eventfd_t) );
+
         register_tls( mainProcessor );
+        __cfa_io_start( mainProcessor );
 
         // Start by initializing the main thread
@@ -304 +314 @@
         mainProcessor->local_data = 0p;
 
+        __cfa_io_stop( mainProcessor );
         unregister_tls( mainProcessor );
 
@@ -355 +366 @@
         register_tls( proc );
 
+        __cfa_io_start( proc );
+
+        // used for idle sleep when io_uring is present
+        io_future_t future;
+        eventfd_t idle_buf;
+        proc->idle_wctx.ftr = &future;
+        proc->idle_wctx.rdbuf = &idle_buf;
+
+
         // SKULLDUGGERY: We want to create a context for the processor coroutine
         // which is needed for the 2-step context switch. However, there is no reason
@@ -381 +401 @@
         // Main routine of the core returned, the core is now fully terminated
         __cfadbg_print_safe(runtime_core, "Kernel : core %p main ended (%p)\n", proc, &proc->runner);
+
+        __cfa_io_stop( proc );
 
         #if !defined(__CFA_NO_STATISTICS__)
@@ -532 +554 @@
         this.local_data = 0p;
 
-        this.idle_fd = eventfd(0, 0);
-        if (idle_fd < 0) {
+        idle_wctx.evfd = eventfd(0, 0);
+        if (idle_wctx.evfd < 0) {
                 abort("KERNEL ERROR: PROCESSOR EVENTFD - %s\n", strerror(errno));
         }
 
-        this.idle_wctx.fd = 0;
+        idle_wctx.sem = 0;
 
         // I'm assuming these two are reserved for standard input and output
         // so I'm using them as sentinels with idle_wctx.
-        /* paranoid */ verify( this.idle_fd != 0 );
-        /* paranoid */ verify( this.idle_fd != 1 );
+        /* paranoid */ verify( idle_wctx.evfd != 0 );
+        /* paranoid */ verify( idle_wctx.evfd != 1 );
 
         #if !defined(__CFA_NO_STATISTICS__)
@@ -554 +576 @@
 // Not a ctor, it just preps the destruction but should not destroy members
 static void deinit(processor & this) {
-        close(this.idle_fd);
+        close(this.idle_wctx.evfd);
 }