Changeset ad58fd2 in mainline

Timestamp:

2022-08-15T16:31:58Z (20 months ago)

Author:

Jiří Zárevúcky <zarevucky.jiri@…>

Branches:

master, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

46b305a

Parents:

742f95ec

git-author:

Jiří Zárevúcky <zarevucky.jiri@…> (2022-08-15 16:26:16)

git-committer:

Jiří Zárevúcky <zarevucky.jiri@…> (2022-08-15 16:31:58)

Message:

Make timeout→cpu immutable

We ensure timeout→cpu is only changed in timeout_register(),
which by its nature is externally synchronized with timeout_unregister(),
and internally synchronized with clock(). Thus in both those contexts,
timeout→cpu is always a valid constant pointing to the CPU of last
call to timeout_register().

Doing so removes the need for synchronization using timeout→lock.
Instead, timeout→link is synchronized by timeout→cpu→timeoutlock,
and all other fields of timeout_t are also immutable outside
timeout_register(), which means they are safely synchronized by
a combination of timeout→cpu→timoutlock and external sychronization
of timeout_register/unregister.

Location:

kernel/generic

Files:

• include/time/timeout.h (modified) (2 diffs)
• src/time/clock.c (modified) (1 diff)
• src/time/timeout.c (modified) (4 diffs)

kernel/generic/include/time/timeout.h

@@ -45 +45 @@
         IRQ_SPINLOCK_DECLARE(lock);
 
-        /** Link to the list of active timeouts on CURRENT->cpu */
+        /** Link to the list of active timeouts on timeout->cpu */
         link_t link;
         /** Timeout will be activated when current clock tick reaches this value. */
@@ -61 +61 @@
 extern void timeout_init(void);
 extern void timeout_initialize(timeout_t *);
-extern void timeout_reinitialize(timeout_t *);
 extern void timeout_register(timeout_t *, uint64_t, timeout_handler_t, void *);
 extern bool timeout_unregister(timeout_t *);

kernel/generic/src/time/clock.c

@@ -172 +172 @@
                         timeout_handler_t handler = timeout->handler;
                         void *arg = timeout->arg;
-                        timeout_reinitialize(timeout);
 
                         irq_spinlock_unlock(&timeout->lock, false);

kernel/generic/src/time/timeout.c

@@ -57 +57 @@
 }
 
-/** Reinitialize timeout
- *
- * Initialize all members except the lock.
- *
- * @param timeout Timeout to be initialized.
- *
- */
-void timeout_reinitialize(timeout_t *timeout)
-{
-        timeout->cpu = NULL;
-        timeout->deadline = 0;
-        timeout->handler = NULL;
-        timeout->arg = NULL;
-        link_initialize(&timeout->link);
-}
-
 /** Initialize timeout
  *
@@ -83 +67 @@
 {
         irq_spinlock_initialize(&timeout->lock, "timeout_t_lock");
-        timeout_reinitialize(timeout);
+        link_initialize(&timeout->link);
+        timeout->cpu = NULL;
 }
 
@@ -103 +88 @@
         irq_spinlock_lock(&CPU->timeoutlock, true);
         irq_spinlock_lock(&timeout->lock, false);
-
-        if (timeout->cpu)
-                panic("Unexpected: timeout->cpu != 0.");
 
         timeout->cpu = CPU;
@@ -152 +134 @@
 bool timeout_unregister(timeout_t *timeout)
 {
-        DEADLOCK_PROBE_INIT(p_tolock);
+        assert(timeout->cpu);
 
-grab_locks:
-        irq_spinlock_lock(&timeout->lock, true);
-        if (!timeout->cpu) {
-                irq_spinlock_unlock(&timeout->lock, true);
-                return false;
+        irq_spinlock_lock(&timeout->cpu->timeoutlock, true);
+
+        bool success = link_in_use(&timeout->link);
+        if (success) {
+                list_remove(&timeout->link);
         }
 
-        if (!irq_spinlock_trylock(&timeout->cpu->timeoutlock)) {
-                irq_spinlock_unlock(&timeout->lock, true);
-                DEADLOCK_PROBE(p_tolock, DEADLOCK_THRESHOLD);
-                goto grab_locks;
-        }
-
-        /*
-         * Now we know for sure that timeout hasn't been activated yet
-         * and is lurking in timeout->cpu->timeout_active_list.
-         */
-
-        link_t *cur = list_next(&timeout->link,
-            &timeout->cpu->timeout_active_list);
-        if (cur != NULL) {
-                timeout_t *tmp = list_get_instance(cur, timeout_t, link);
-                irq_spinlock_lock(&tmp->lock, false);
-                irq_spinlock_unlock(&tmp->lock, false);
-        }
-
-        list_remove(&timeout->link);
-        irq_spinlock_unlock(&timeout->cpu->timeoutlock, false);
-
-        timeout_reinitialize(timeout);
-        irq_spinlock_unlock(&timeout->lock, true);
-
-        return true;
+        irq_spinlock_unlock(&timeout->cpu->timeoutlock, true);
+        return success;
 }