Kernel: Turn lock ranks into template parameters

This step would ideally not have been necessary (increases amount of
refactoring and templates necessary, which in turn increases build
times), but it gives us a couple of nice properties:
- SpinlockProtected inside Singleton (a very common combination) can now
  obtain any lock rank just via the template parameter. It was not
  previously possible to do this with SingletonInstanceCreator magic.
- SpinlockProtected's lock rank is now mandatory; this is the majority
  of cases and allows us to see where we're still missing proper ranks.
- The type already informs us what lock rank a lock has, which aids code
  readability and (possibly, if gdb cooperates) lock mismatch debugging.
- The rank of a lock can no longer be dynamic, which is not something we
  wanted in the first place (or made use of). Locks randomly changing
  their rank sounds like a disaster waiting to happen.
- In some places, we might be able to statically check that locks are
  taken in the right order (with the right lock rank checking
  implementation) as rank information is fully statically known.

This refactoring even more exposes the fact that Mutex has no lock rank
capabilites, which is not fixed here.

AK/Singleton.h

@@ -39,12 +39,11 @@ struct SingletonInstanceCreator {
 
 #ifdef KERNEL
 
-// FIXME: Find a nice way of injecting the lock rank into the singleton.
-template<typename T>
-struct SingletonInstanceCreator<Kernel::SpinlockProtected<T>> {
- static Kernel::SpinlockProtected<T>* create()
+template<typename T, Kernel::LockRank Rank>
+struct SingletonInstanceCreator<Kernel::SpinlockProtected<T, Rank>> {
+ static Kernel::SpinlockProtected<T, Rank>* create()
  {
- return new Kernel::SpinlockProtected<T> { Kernel::LockRank::None };
+ return new Kernel::SpinlockProtected<T, Rank> {};
  }
 };
 #endif

Kernel/Arch/x86_64/ISABus/I8042Controller.h

@@ -153,7 +153,7 @@ class I8042Controller final : public AtomicRefCounted<I8042Controller> {
  void do_write(u8 port, u8 data);
  u8 do_read(u8 port);
 
- Spinlock m_lock { LockRank::None };
+ Spinlock<LockRank::None> m_lock {};
  bool m_first_port_available { false };
  bool m_second_port_available { false };
  bool m_is_dual_channel { false };

Kernel/Arch/x86_64/PageDirectory.cpp

@@ -12,7 +12,7 @@
 namespace Kernel::Memory {
 
 struct CR3Map {
- SpinlockProtected<IntrusiveRedBlackTree<&PageDirectory::m_tree_node>> map { LockRank::None };
+ SpinlockProtected<IntrusiveRedBlackTree<&PageDirectory::m_tree_node>, LockRank::None> map {};
 };
 
 static Singleton<CR3Map> s_cr3_map;

Kernel/Arch/x86_64/VGA/IOArbiter.h

@@ -33,7 +33,7 @@ class VGAIOArbiter {
  void disable_vga_text_mode_console_cursor();
  void enable_vga_text_mode_console_cursor();
 
- RecursiveSpinlock m_main_vga_lock { LockRank::None };
+ RecursiveSpinlock<LockRank::None> m_main_vga_lock {};
  bool m_vga_access_is_disabled { false };
 };
 

Kernel/Bus/PCI/Access.h

@@ -41,8 +41,8 @@ class Access {
  u32 read32_field(Address address, u32 field);
  DeviceIdentifier get_device_identifier(Address address) const;
 
- Spinlock const& scan_lock() const { return m_scan_lock; }
- RecursiveSpinlock const& access_lock() const { return m_access_lock; }
+ Spinlock<LockRank::None> const& scan_lock() const { return m_scan_lock; }
+ RecursiveSpinlock<LockRank::None> const& access_lock() const { return m_access_lock; }
 
  ErrorOr<void> add_host_controller_and_enumerate_attached_devices(NonnullOwnPtr<HostController>, Function<void(DeviceIdentifier const&)> callback);
 
@@ -57,8 +57,8 @@ class Access {
  Vector<Capability> get_capabilities(Address);
  Optional<u8> get_capabilities_pointer(Address address);
 
- mutable RecursiveSpinlock m_access_lock { LockRank::None };
- mutable Spinlock m_scan_lock { LockRank::None };
+ mutable RecursiveSpinlock<LockRank::None> m_access_lock {};
+ mutable Spinlock<LockRank::None> m_scan_lock {};
 
  HashMap<u32, NonnullOwnPtr<PCI::HostController>> m_host_controllers;
  Vector<DeviceIdentifier> m_device_identifiers;

Kernel/Bus/PCI/Controller/VolumeManagementDevice.h

@@ -29,7 +29,7 @@ class VolumeManagementDevice final : public MemoryBackedHostBridge {
 
  // Note: All read and writes must be done with a spinlock because
  // Linux says that CPU might deadlock otherwise if access is not serialized.
- Spinlock m_config_lock { LockRank::None };
+ Spinlock<LockRank::None> m_config_lock {};
 };
 
 }

Kernel/Bus/USB/UHCI/UHCIController.cpp

@@ -90,8 +90,6 @@ UNMAP_AFTER_INIT UHCIController::UHCIController(PCI::DeviceIdentifier const& pci
  : PCI::Device(pci_device_identifier.address())
  , IRQHandler(pci_device_identifier.interrupt_line().value())
  , m_registers_io_window(move(registers_io_window))
- , m_async_lock(LockRank::None)
- , m_schedule_lock(LockRank::None)
 {
 }
 

Kernel/Bus/USB/UHCI/UHCIController.h

@@ -100,8 +100,8 @@ class UHCIController final
 
  NonnullOwnPtr<IOWindow> m_registers_io_window;
 
- Spinlock m_async_lock;
- Spinlock m_schedule_lock;
+ Spinlock<LockRank::None> m_async_lock {};
+ Spinlock<LockRank::None> m_schedule_lock {};
 
  OwnPtr<UHCIRootHub> m_root_hub;
  OwnPtr<UHCIDescriptorPool<QueueHead>> m_queue_head_pool;

Kernel/Bus/USB/UHCI/UHCIDescriptorPool.h

@@ -70,7 +70,6 @@ class UHCIDescriptorPool {
  UHCIDescriptorPool(NonnullOwnPtr<Memory::Region> pool_memory_block, StringView name)
  : m_pool_name(name)
  , m_pool_region(move(pool_memory_block))
- , m_pool_lock(LockRank::None)
  {
  // Go through the number of descriptors to create in the pool, and create a virtual/physical address mapping
  for (size_t i = 0; i < PAGE_SIZE / sizeof(T); i++) {
@@ -84,7 +83,7 @@ class UHCIDescriptorPool {
  StringView m_pool_name; // Name of this pool
  NonnullOwnPtr<Memory::Region> m_pool_region; // Memory region where descriptors actually reside
  Stack<T*, PAGE_SIZE / sizeof(T)> m_free_descriptor_stack; // Stack of currently free descriptor pointers
- Spinlock m_pool_lock;
+ Spinlock<LockRank::None> m_pool_lock;
 };
 
 }

Kernel/Bus/VirtIO/Queue.h

@@ -47,7 +47,7 @@ class Queue {
  QueueChain pop_used_buffer_chain(size_t& used);
  void discard_used_buffers();
 
- Spinlock& lock() { return m_lock; }
+ Spinlock<LockRank::None>& lock() { return m_lock; }
 
  bool should_notify() const;
 
@@ -96,7 +96,7 @@ class Queue {
  QueueDriver* m_driver { nullptr };
  QueueDevice* m_device { nullptr };
  NonnullOwnPtr<Memory::Region> m_queue_region;
- Spinlock m_lock { LockRank::None };
+ Spinlock<LockRank::None> m_lock {};
 
  friend class QueueChain;
 };

Kernel/CMakeLists.txt

@@ -236,7 +236,6 @@ set(KERNEL_SOURCES
  MiniStdLib.cpp
  Locking/LockRank.cpp
  Locking/Mutex.cpp
- Locking/Spinlock.cpp
  Net/Intel/E1000ENetworkAdapter.cpp
  Net/Intel/E1000NetworkAdapter.cpp
  Net/NE2000/NetworkAdapter.cpp

Kernel/Coredump.cpp

@@ -23,11 +23,11 @@
 
 #define INCLUDE_USERSPACE_HEAP_MEMORY_IN_COREDUMPS 0
 
-static Singleton<SpinlockProtected<OwnPtr<KString>>> s_coredump_directory_path;
+static Singleton<SpinlockProtected<OwnPtr<KString>, LockRank::None>> s_coredump_directory_path;
 
 namespace Kernel {
 
-SpinlockProtected<OwnPtr<KString>>& Coredump::directory_path()
+SpinlockProtected<OwnPtr<KString>, LockRank::None>& Coredump::directory_path()
 {
  return s_coredump_directory_path;
 }

Kernel/Coredump.h

@@ -19,7 +19,7 @@ namespace Kernel {
 class Coredump {
 public:
  static ErrorOr<NonnullOwnPtr<Coredump>> try_create(NonnullLockRefPtr<Process>, StringView output_path);
- static SpinlockProtected<OwnPtr<KString>>& directory_path();
+ static SpinlockProtected<OwnPtr<KString>, LockRank::None>& directory_path();
 
  ~Coredump() = default;
  ErrorOr<void> write();

Kernel/Devices/AsyncDeviceRequest.h

@@ -62,7 +62,7 @@ class AsyncDeviceRequest : public AtomicRefCounted<AsyncDeviceRequest> {
 
  [[nodiscard]] RequestWaitResult wait(Time* = nullptr);
 
- void do_start(SpinlockLocker<Spinlock>&& requests_lock)
+ void do_start(SpinlockLocker<Spinlock<LockRank::None>>&& requests_lock)
  {
  if (is_completed_result(m_result))
  return;
@@ -151,7 +151,7 @@ class AsyncDeviceRequest : public AtomicRefCounted<AsyncDeviceRequest> {
  WaitQueue m_queue;
  NonnullLockRefPtr<Process> m_process;
  void* m_private { nullptr };
- mutable Spinlock m_lock { LockRank::None };
+ mutable Spinlock<LockRank::None> m_lock {};
 };
 
 }

Kernel/Devices/Audio/AC97.h

@@ -147,7 +147,7 @@ class AC97 final
 
  NonnullOwnPtr<IOWindow> m_channel_io_window;
  PCI::Address m_device_pci_address;
- SpinlockProtected<bool> m_dma_running { LockRank::None, false };
+ SpinlockProtected<bool, LockRank::None> m_dma_running { false };
  StringView m_name;
  };
 

Kernel/Devices/ConsoleDevice.cpp

@@ -16,7 +16,7 @@
 
 namespace Kernel {
 
-Spinlock g_console_lock { LockRank::None };
+Spinlock<LockRank::None> g_console_lock {};
 
 UNMAP_AFTER_INIT NonnullLockRefPtr<ConsoleDevice> ConsoleDevice::must_create()
 {

Kernel/Devices/ConsoleDevice.h

@@ -12,7 +12,7 @@
 
 namespace Kernel {
 
-extern Spinlock g_console_lock;
+extern Spinlock<LockRank::None> g_console_lock;
 
 class ConsoleDevice final : public CharacterDevice {
  friend class DeviceManagement;

Kernel/Devices/Device.h

@@ -90,7 +90,7 @@ class Device : public File {
 
  State m_state { State::Normal };
 
- Spinlock m_requests_lock { LockRank::None };
+ Spinlock<LockRank::None> m_requests_lock {};
  DoublyLinkedList<LockRefPtr<AsyncDeviceRequest>> m_requests;
 
 protected:

Kernel/Devices/DeviceManagement.h

@@ -74,9 +74,9 @@ class DeviceManagement {
  LockRefPtr<ConsoleDevice> m_console_device;
  LockRefPtr<DeviceControlDevice> m_device_control_device;
  // FIXME: Once we have a singleton for managing many sound cards, remove this from here
- SpinlockProtected<HashMap<u64, Device*>> m_devices { LockRank::None };
+ SpinlockProtected<HashMap<u64, Device*>, LockRank::None> m_devices {};
 
- mutable Spinlock m_event_queue_lock { LockRank::None };
+ mutable Spinlock<LockRank::None> m_event_queue_lock {};
  CircularQueue<DeviceEvent, 100> m_event_queue;
 };
 

Kernel/Devices/HID/HIDManagement.h

@@ -47,7 +47,7 @@ class HIDManagement {
  Keyboard::CharacterMapData character_map;
  };
 
- SpinlockProtected<KeymapData>& keymap_data() { return m_keymap_data; }
+ SpinlockProtected<KeymapData, LockRank::None>& keymap_data() { return m_keymap_data; }
 
  u32 get_char_from_character_map(KeyEvent) const;
 
@@ -58,15 +58,15 @@ class HIDManagement {
  size_t generate_minor_device_number_for_mouse();
  size_t generate_minor_device_number_for_keyboard();
 
- SpinlockProtected<KeymapData> m_keymap_data { LockRank::None };
+ SpinlockProtected<KeymapData, LockRank::None> m_keymap_data {};
  size_t m_mouse_minor_number { 0 };
  size_t m_keyboard_minor_number { 0 };
  KeyboardClient* m_client { nullptr };
 #if ARCH(X86_64)
  LockRefPtr<I8042Controller> m_i8042_controller;
 #endif
  NonnullLockRefPtrVector<HIDDevice> m_hid_devices;
- Spinlock m_client_lock { LockRank::None };
+ Spinlock<LockRank::None> m_client_lock {};
 };
 
 class KeyboardClient {

Kernel/Devices/HID/KeyboardDevice.h

@@ -45,7 +45,7 @@ class KeyboardDevice : public HIDDevice {
 
 protected:
  KeyboardDevice();
- mutable Spinlock m_queue_lock { LockRank::None };
+ mutable Spinlock<LockRank::None> m_queue_lock {};
  CircularQueue<Event, 16> m_queue;
  // ^CharacterDevice
  virtual StringView class_name() const override { return "KeyboardDevice"sv; }

Kernel/Devices/HID/MouseDevice.h

@@ -35,7 +35,7 @@ class MouseDevice : public HIDDevice {
  // ^CharacterDevice
  virtual StringView class_name() const override { return "MouseDevice"sv; }
 
- mutable Spinlock m_queue_lock { LockRank::None };
+ mutable Spinlock<LockRank::None> m_queue_lock {};
  CircularQueue<MousePacket, 100> m_queue;
 };
 

Kernel/Devices/KCOVInstance.h

@@ -46,7 +46,7 @@ class KCOVInstance final {
 
  Memory::VMObject* vmobject() { return m_vmobject; }
 
- Spinlock& spinlock() { return m_lock; }
+ Spinlock<LockRank::None>& spinlock() { return m_lock; }
 
 private:
  ProcessID m_pid { 0 };
@@ -58,7 +58,7 @@ class KCOVInstance final {
  // Here to ensure it's not garbage collected at the end of open()
  OwnPtr<Memory::Region> m_kernel_region;
 
- Spinlock m_lock { LockRank::None };
+ Spinlock<LockRank::None> m_lock {};
 };
 
 }

Kernel/Devices/SerialDevice.h

@@ -130,7 +130,7 @@ class SerialDevice final : public CharacterDevice {
  bool m_break_enable { false };
  u8 m_modem_control { 0 };
  bool m_last_put_char_was_carriage_return { false };
- Spinlock m_serial_lock { LockRank::None };
+ Spinlock<LockRank::None> m_serial_lock {};
 };
 
 }

Kernel/FileSystem/Custody.cpp

@@ -14,9 +14,9 @@
 
 namespace Kernel {
 
-static Singleton<SpinlockProtected<Custody::AllCustodiesList>> s_all_instances;
+static Singleton<SpinlockProtected<Custody::AllCustodiesList, LockRank::None>> s_all_instances;
 
-SpinlockProtected<Custody::AllCustodiesList>& Custody::all_instances()
+SpinlockProtected<Custody::AllCustodiesList, LockRank::None>& Custody::all_instances()
 {
  return s_all_instances;
 }

Kernel/FileSystem/Custody.h

@@ -44,7 +44,7 @@ class Custody final : public ListedRefCounted<Custody, LockType::Spinlock> {
 
 public:
  using AllCustodiesList = IntrusiveList<&Custody::m_all_custodies_list_node>;
- static SpinlockProtected<Custody::AllCustodiesList>& all_instances();
+ static SpinlockProtected<Custody::AllCustodiesList, LockRank::None>& all_instances();
 };
 
 }

Kernel/FileSystem/FileSystem.h

@@ -61,7 +61,7 @@ class FileSystem : public AtomicRefCounted<FileSystem> {
  // Converts file types that are used internally by the filesystem to DT_* types
  virtual u8 internal_file_type_to_directory_entry_type(DirectoryEntryView const& entry) const { return entry.file_type; }
 
- SpinlockProtected<size_t>& mounted_count(Badge<VirtualFileSystem>) { return m_attach_count; }
+ SpinlockProtected<size_t, LockRank::FileSystem>& mounted_count(Badge<VirtualFileSystem>) { return m_attach_count; }
 
 protected:
  FileSystem();
@@ -79,7 +79,7 @@ class FileSystem : public AtomicRefCounted<FileSystem> {
  size_t m_fragment_size { 0 };
  bool m_readonly { false };
 
- SpinlockProtected<size_t> m_attach_count { LockRank::FileSystem, 0 };
+ SpinlockProtected<size_t, LockRank::FileSystem> m_attach_count { 0 };
  IntrusiveListNode<FileSystem> m_file_system_node;
 };
 

Kernel/FileSystem/Inode.cpp

@@ -22,9 +22,9 @@
 
 namespace Kernel {
 
-static Singleton<SpinlockProtected<Inode::AllInstancesList>> s_all_instances;
+static Singleton<SpinlockProtected<Inode::AllInstancesList, LockRank::None>> s_all_instances;
 
-SpinlockProtected<Inode::AllInstancesList>& Inode::all_instances()
+SpinlockProtected<Inode::AllInstancesList, LockRank::None>& Inode::all_instances()
 {
  return s_all_instances;
 }

Kernel/FileSystem/Inode.h

@@ -132,7 +132,7 @@ class Inode : public ListedRefCounted<Inode, LockType::Spinlock>
  InodeIndex m_index { 0 };
  LockWeakPtr<Memory::SharedInodeVMObject> m_shared_vmobject;
  LockRefPtr<LocalSocket> m_bound_socket;
- SpinlockProtected<HashTable<InodeWatcher*>> m_watchers { LockRank::None };
+ SpinlockProtected<HashTable<InodeWatcher*>, LockRank::None> m_watchers {};
  bool m_metadata_dirty { false };
  LockRefPtr<FIFO> m_fifo;
  IntrusiveListNode<Inode> m_inode_list_node;
@@ -146,11 +146,11 @@ class Inode : public ListedRefCounted<Inode, LockType::Spinlock>
  };
 
  Thread::FlockBlockerSet m_flock_blocker_set;
- SpinlockProtected<Vector<Flock>> m_flocks { LockRank::None };
+ SpinlockProtected<Vector<Flock>, LockRank::None> m_flocks {};
 
 public:
  using AllInstancesList = IntrusiveList<&Inode::m_inode_list_node>;
- static SpinlockProtected<Inode::AllInstancesList>& all_instances();
+ static SpinlockProtected<Inode::AllInstancesList, LockRank::None>& all_instances();
 };
 
 }

Kernel/FileSystem/Mount.cpp

@@ -14,15 +14,15 @@ namespace Kernel {
 Mount::Mount(FileSystem& guest_fs, Custody* host_custody, int flags)
  : m_guest(guest_fs.root_inode())
  , m_guest_fs(guest_fs)
- , m_host_custody(LockRank::None, host_custody)
+ , m_host_custody(host_custody)
  , m_flags(flags)
 {
 }
 
 Mount::Mount(Inode& source, Custody& host_custody, int flags)
  : m_guest(source)
  , m_guest_fs(source.fs())
- , m_host_custody(LockRank::None, host_custody)
+ , m_host_custody(host_custody)
  , m_flags(flags)
 {
 }

Kernel/FileSystem/Mount.h

@@ -40,7 +40,7 @@ class Mount {
 private:
  NonnullLockRefPtr<Inode> m_guest;
  NonnullLockRefPtr<FileSystem> m_guest_fs;
- SpinlockProtected<RefPtr<Custody>> m_host_custody;
+ SpinlockProtected<RefPtr<Custody>, LockRank::None> m_host_custody;
  int m_flags;
 
  IntrusiveListNode<Mount> m_vfs_list_node;