Commit Graph

7 Commits

Author SHA1 Message Date
Anhad Singh 7d42e8d966 fix(switch): deadlock
During the snapshot shown, interrupts would be _disabled_ on both CPUs and both
would have have the _same_ address space (i.e.
`PercpuBlock::current().current_addrsp`).

```rs
// CPU 0                                                                                             CPU 1 (inside `context::switch()`)
// ------------------------------------------------------------------------------------------------------------------------------------------------------------------------
// let guard = current_addrsp.acquire_write()
// let flusher = Flusher::with_cpu_set(&addrsp.used_by, &current_addrsp.tlb_ack);
// ...
// drop(flusher); // Flusher::flush()
//     // send IPI to CPU 1 as it is in the `active_set`
//     shootdown_tlb_ipi(Some(1));
//                                                                                                    percpu::switch_arch_hook();
//     // spin until TLB shootdown IPI has been acknowledged by CPU 1                                 // acquire_read() to remove the CPU from the `used_by` set in `prev_addrsp`
//     while ackword < affected_cpu_count                                                             prev_addrsp.acquire_read()
//         PercpuBlock::current().maybe_handle_tlb_shootdown();
//         spin_loop();
//     }
// drop(guard);
```

Now CPU 0 will spin _until_ CPU 1 has acknowledged the IPI. However, CPU 1 has
interrupts disabled and is in the middle of a context switch. To complete the
context switch it needs to remove itself from the `used_by` set in
`prev_addrsp` (which is the current address space atm). To do that it needs to
`acquire_read()` lock but it cannot since it is held by CPU 0 and won't release
it until CPU 1 has acknowledged the IPI; creating a deadlock.

Also note:

```rs
// src/context/memory.rs
// NOTE: Lock must be held, which must be guaranteed by the caller.
pub fn flush(&mut self) {
```

Here is some output from GDB to back this up:

```
pwndbg> info threads
  Id   Target Id                    Frame
  1    Thread 1.1 (CPU#0 [running]) core::sync::atomic::atomic_compare_exchange_weak<usize> (dst=0xffffff7f801972f0, old=0, new=1, success=core::sync::atomic::Ordering::Acquire, failure=core::sync::atomic::Ordering::Relaxed)
    at /mnt/redox/prefix/x86_64-unknown-redox/sysroot/lib/rustlib/src/rust/library/core/src/sync/atomic.rs:4100
  2    Thread 1.2 (CPU#1 [running]) kernel::context::memory::Flusher::flush (self=0xffff80004212f4b8) at src/context/memory.rs:2827
  3    Thread 1.3 (CPU#2 [running]) spin::rwlock::RwLock<kernel::context::context::Context, spin::relax::Spin>::write<kernel::context::context::Context, spin::relax::Spin> (self=0xffffff7f801a99c0)
    at /root/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/spin-0.9.8/src/rwlock.rs:239
* 4    Thread 1.4 (CPU#3 [running]) 0xffffffff80059cf9 in kernel::context::memory::AddrSpaceWrapper::acquire_read (self=0xffffff7f80188110)
    at /mnt/redox/prefix/x86_64-unknown-redox/sysroot/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/x86/sse2.rs:25
pwndbg> p kernel::context::arch::CONTEXT_SWITCH_LOCK
$1 = core::sync::atomic::AtomicBool {
  v: core::cell::UnsafeCell<u8> {
    value: 1
  }
}
pwndbg> p (*kernel::percpu::ALL_PERCPU_BLOCKS[0].p.value).wants_tlb_shootdown.v.value
$2 = 0
pwndbg> p (*kernel::percpu::ALL_PERCPU_BLOCKS[1].p.value).wants_tlb_shootdown.v.value
$3 = 0
pwndbg> p (*kernel::percpu::ALL_PERCPU_BLOCKS[2].p.value).wants_tlb_shootdown.v.value
$4 = 1
pwndbg> p (*kernel::percpu::ALL_PERCPU_BLOCKS[3].p.value).wants_tlb_shootdown.v.value
$5 = 0
pwndbg> p (*kernel::percpu::ALL_PERCPU_BLOCKS[3].p.value).current_addrsp
$6 = core::cell::RefCell<core::option::Option<alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global>>> {
  borrow: core::cell::Cell<isize> {
    value: core::cell::UnsafeCell<isize> {
      value: 0
    }
  },
  value: core::cell::UnsafeCell<core::option::Option<alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global>>> {
    value: core::option::Option<alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global>>::Some(alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global> {
        ptr: core::ptr::non_null::NonNull<alloc::sync::ArcInner<kernel::context::memory::AddrSpaceWrapper>> {
          pointer: 0xffffff7f80188100
        },
        phantom: core::marker::PhantomData<alloc::sync::ArcInner<kernel::context::memory::AddrSpaceWrapper>>,
        alloc: alloc::alloc::Global
      })
  }
}
pwndbg> p (*kernel::percpu::ALL_PERCPU_BLOCKS[2].p.value).current_addrsp
$7 = core::cell::RefCell<core::option::Option<alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global>>> {
  borrow: core::cell::Cell<isize> {
    value: core::cell::UnsafeCell<isize> {
      value: 0
    }
  },
  value: core::cell::UnsafeCell<core::option::Option<alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global>>> {
    value: core::option::Option<alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global>>::Some(alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global> {
        ptr: core::ptr::non_null::NonNull<alloc::sync::ArcInner<kernel::context::memory::AddrSpaceWrapper>> {
          pointer: 0xffffff7f80188100
        },
        phantom: core::marker::PhantomData<alloc::sync::ArcInner<kernel::context::memory::AddrSpaceWrapper>>,
        alloc: alloc::alloc::Global
      })
  }
}
pwndbg> p (*kernel::percpu::ALL_PERCPU_BLOCKS[1].p.value).current_addrsp
$8 = core::cell::RefCell<core::option::Option<alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global>>> {
  borrow: core::cell::Cell<isize> {
    value: core::cell::UnsafeCell<isize> {
      value: 0
    }
  },
  value: core::cell::UnsafeCell<core::option::Option<alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global>>> {
    value: core::option::Option<alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global>>::Some(alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global> {
        ptr: core::ptr::non_null::NonNull<alloc::sync::ArcInner<kernel::context::memory::AddrSpaceWrapper>> {
          pointer: 0xffffff7f80188100
        },
        phantom: core::marker::PhantomData<alloc::sync::ArcInner<kernel::context::memory::AddrSpaceWrapper>>,
        alloc: alloc::alloc::Global
      })
  }
}
pwndbg> p (*kernel::percpu::ALL_PERCPU_BLOCKS[0].p.value).current_addrsp
$9 = core::cell::RefCell<core::option::Option<alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global>>> {
  borrow: core::cell::Cell<isize> {
    value: core::cell::UnsafeCell<isize> {
      value: 0
    }
  },
  value: core::cell::UnsafeCell<core::option::Option<alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global>>> {
    value: core::option::Option<alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global>>::Some(alloc::sync::Arc<kernel::context::memory::AddrSpaceWrapper, alloc::alloc::Global> {
        ptr: core::ptr::non_null::NonNull<alloc::sync::ArcInner<kernel::context::memory::AddrSpaceWrapper>> {
          pointer: 0xffffff7f80133ac8
        },
        phantom: core::marker::PhantomData<alloc::sync::ArcInner<kernel::context::memory::AddrSpaceWrapper>>,
        alloc: alloc::alloc::Global
      })
  }
}
```

```
pwndbg> thread 4
[Switching to thread 4 (Thread 1.4)]
25	in /mnt/redox/prefix/x86_64-unknown-redox/sysroot/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/x86/sse2.rs
pwndbg> info threads
  Id   Target Id                    Frame
  1    Thread 1.1 (CPU#0 [running]) core::sync::atomic::atomic_compare_exchange_weak<usize> (dst=0xffffff7f801972f0, old=0, new=1, success=core::sync::atomic::Ordering::Acquire, failure=core::sync::atomic::Ordering::Relaxed)
    at /mnt/redox/prefix/x86_64-unknown-redox/sysroot/lib/rustlib/src/rust/library/core/src/sync/atomic.rs:4100
  2    Thread 1.2 (CPU#1 [running]) kernel::context::memory::Flusher::flush (self=0xffff80004212f4b8) at src/context/memory.rs:2827
  3    Thread 1.3 (CPU#2 [running]) spin::rwlock::RwLock<kernel::context::context::Context, spin::relax::Spin>::write<kernel::context::context::Context, spin::relax::Spin> (self=0xffffff7f801a99c0)
    at /root/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/spin-0.9.8/src/rwlock.rs:239
* 4    Thread 1.4 (CPU#3 [running]) 0xffffffff80059cf9 in kernel::context::memory::AddrSpaceWrapper::acquire_read (self=0xffffff7f80188110)
    at /mnt/redox/prefix/x86_64-unknown-redox/sysroot/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/x86/sse2.rs:25
pwndbg> thread 4
[Switching to thread 4 (Thread 1.4)]
25	in /mnt/redox/prefix/x86_64-unknown-redox/sysroot/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/x86/sse2.rs
pwndbg> bt
```

This is *after* everything freezes.

For relibc tests, if I run `while true; do
/root/relibc-tests/pthread/once; done` it eventually does freeze up and
does so at the same location:

```
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────[ THREADS (4 TOTAL) ]──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
  ► 1   "" stopped: 0xffffffff800750a2 <kernel::context::switch::switch+1282>
    4   "" stopped: 0xffffffff80074d5a <kernel::context::switch::switch+442>
    3   "" stopped: 0xffffffff80059cf9 <kernel::context::context::Context::set_addr_space+217>
    2   "" stopped: 0xffffffff80020839 <kernel::context::memory::Flusher::flush+361>
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
```

Related: https://gitlab.redox-os.org/redox-os/kernel/-/issues/175

Signed-off-by: Anhad Singh <andypython@protonmail.com>
2026-01-19 15:21:13 +11:00
Speedy_Lex 0931a7f49f Resolve a huge portion of the clippy lints 2025-10-06 06:30:23 -06:00
bjorn3 f58bcd584e Separate logical and hardware cpu ids 2025-09-20 15:33:08 +02:00
4lDO2 d94baa2712 Run rustfmt. 2024-07-15 17:50:18 +02:00
4lDO2 f3532f4549 Fix static mut ref warnings. 2024-07-15 17:48:46 +02:00
4lDO2 6bf90f9eee Build the foundation for TLB shootdown. 2024-03-04 19:03:00 +01:00
4lDO2 0c6c857012 Improve LogicalCpu{Id,Set}, make set atomic.
Technically there should be an atomic and nonatomic set type, but perf
should be the same, as the nonatomic one would only be used when setting
the sched_affinity.
2024-03-04 19:02:56 +01:00