From b204da94b09fcb915319fff490045117bebc0078 Mon Sep 17 00:00:00 2001 From: Pankaj Raghav Date: Tue, 14 Mar 2023 09:44:21 +0100 Subject: [PATCH] Kernel/Storage: Use NonnullRefPtr for storage controllers Storage controllers are initialized during init and are never modified. NonnullRefPtr can be safely used instead of the NonnullLockRefPtr. This also fixes one of the UB issue that was there when using an NVMe device because of NonnullLockRefPtr. We can add proper locking when we need to modify the storage controllers after init. --- Kernel/Arch/x86_64/ISABus/IDEController.cpp | 4 ++-- Kernel/Arch/x86_64/ISABus/IDEController.h | 2 +- Kernel/Arch/x86_64/PCI/IDELegacyModeController.cpp | 4 ++-- Kernel/Arch/x86_64/PCI/IDELegacyModeController.h | 2 +- Kernel/Storage/ATA/AHCI/Controller.cpp | 4 ++-- Kernel/Storage/ATA/AHCI/Controller.h | 2 +- Kernel/Storage/NVMe/NVMeController.cpp | 4 ++-- Kernel/Storage/NVMe/NVMeController.h | 2 +- Kernel/Storage/Ramdisk/Controller.cpp | 4 ++-- Kernel/Storage/Ramdisk/Controller.h | 2 +- Kernel/Storage/StorageManagement.h | 2 +- 11 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Kernel/Arch/x86_64/ISABus/IDEController.cpp b/Kernel/Arch/x86_64/ISABus/IDEController.cpp index 870564725d..26826febab 100644 --- a/Kernel/Arch/x86_64/ISABus/IDEController.cpp +++ b/Kernel/Arch/x86_64/ISABus/IDEController.cpp @@ -15,9 +15,9 @@ namespace Kernel { -UNMAP_AFTER_INIT ErrorOr> ISAIDEController::initialize() +UNMAP_AFTER_INIT ErrorOr> ISAIDEController::initialize() { - auto controller = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) ISAIDEController())); + auto controller = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) ISAIDEController())); TRY(controller->initialize_channels()); return controller; } diff --git a/Kernel/Arch/x86_64/ISABus/IDEController.h b/Kernel/Arch/x86_64/ISABus/IDEController.h index ed4e37a6b3..1e094e190a 100644 --- a/Kernel/Arch/x86_64/ISABus/IDEController.h +++ b/Kernel/Arch/x86_64/ISABus/IDEController.h @@ -18,7 +18,7 @@ class AsyncBlockDeviceRequest; class ISAIDEController final : public IDEController { public: - static ErrorOr> initialize(); + static ErrorOr> initialize(); private: ISAIDEController(); diff --git a/Kernel/Arch/x86_64/PCI/IDELegacyModeController.cpp b/Kernel/Arch/x86_64/PCI/IDELegacyModeController.cpp index 6fb3a0e99f..66191cafd1 100644 --- a/Kernel/Arch/x86_64/PCI/IDELegacyModeController.cpp +++ b/Kernel/Arch/x86_64/PCI/IDELegacyModeController.cpp @@ -15,9 +15,9 @@ namespace Kernel { -UNMAP_AFTER_INIT ErrorOr> PCIIDELegacyModeController::initialize(PCI::DeviceIdentifier const& device_identifier, bool force_pio) +UNMAP_AFTER_INIT ErrorOr> PCIIDELegacyModeController::initialize(PCI::DeviceIdentifier const& device_identifier, bool force_pio) { - auto controller = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) PCIIDELegacyModeController(device_identifier))); + auto controller = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) PCIIDELegacyModeController(device_identifier))); PCI::enable_io_space(device_identifier); PCI::enable_memory_space(device_identifier); PCI::enable_bus_mastering(device_identifier); diff --git a/Kernel/Arch/x86_64/PCI/IDELegacyModeController.h b/Kernel/Arch/x86_64/PCI/IDELegacyModeController.h index 367e31906c..f23591ee9b 100644 --- a/Kernel/Arch/x86_64/PCI/IDELegacyModeController.h +++ b/Kernel/Arch/x86_64/PCI/IDELegacyModeController.h @@ -19,7 +19,7 @@ class AsyncBlockDeviceRequest; class PCIIDELegacyModeController final : public IDEController , public PCI::Device { public: - static ErrorOr> initialize(PCI::DeviceIdentifier const&, bool force_pio); + static ErrorOr> initialize(PCI::DeviceIdentifier const&, bool force_pio); virtual StringView device_name() const override { return "PCIIDELegacyModeController"sv; } diff --git a/Kernel/Storage/ATA/AHCI/Controller.cpp b/Kernel/Storage/ATA/AHCI/Controller.cpp index 23105ba65d..3bbaa6f17a 100644 --- a/Kernel/Storage/ATA/AHCI/Controller.cpp +++ b/Kernel/Storage/ATA/AHCI/Controller.cpp @@ -18,9 +18,9 @@ namespace Kernel { -UNMAP_AFTER_INIT NonnullLockRefPtr AHCIController::initialize(PCI::DeviceIdentifier const& pci_device_identifier) +UNMAP_AFTER_INIT NonnullRefPtr AHCIController::initialize(PCI::DeviceIdentifier const& pci_device_identifier) { - auto controller = adopt_lock_ref_if_nonnull(new (nothrow) AHCIController(pci_device_identifier)).release_nonnull(); + auto controller = adopt_ref_if_nonnull(new (nothrow) AHCIController(pci_device_identifier)).release_nonnull(); controller->initialize_hba(pci_device_identifier); return controller; } diff --git a/Kernel/Storage/ATA/AHCI/Controller.h b/Kernel/Storage/ATA/AHCI/Controller.h index eb80a54401..9fccadcdd1 100644 --- a/Kernel/Storage/ATA/AHCI/Controller.h +++ b/Kernel/Storage/ATA/AHCI/Controller.h @@ -24,7 +24,7 @@ class AHCIController final : public ATAController friend class AHCIInterruptHandler; public: - static NonnullLockRefPtr initialize(PCI::DeviceIdentifier const& pci_device_identifier); + static NonnullRefPtr initialize(PCI::DeviceIdentifier const& pci_device_identifier); virtual ~AHCIController() override; virtual StringView device_name() const override { return "AHCI"sv; } diff --git a/Kernel/Storage/NVMe/NVMeController.cpp b/Kernel/Storage/NVMe/NVMeController.cpp index 7f9335dfe0..6d848e0b36 100644 --- a/Kernel/Storage/NVMe/NVMeController.cpp +++ b/Kernel/Storage/NVMe/NVMeController.cpp @@ -19,9 +19,9 @@ namespace Kernel { -UNMAP_AFTER_INIT ErrorOr> NVMeController::try_initialize(Kernel::PCI::DeviceIdentifier const& device_identifier, bool is_queue_polled) +UNMAP_AFTER_INIT ErrorOr> NVMeController::try_initialize(Kernel::PCI::DeviceIdentifier const& device_identifier, bool is_queue_polled) { - auto controller = TRY(adopt_nonnull_lock_ref_or_enomem(new NVMeController(device_identifier, StorageManagement::generate_relative_nvme_controller_id({})))); + auto controller = TRY(adopt_nonnull_ref_or_enomem(new NVMeController(device_identifier, StorageManagement::generate_relative_nvme_controller_id({})))); TRY(controller->initialize(is_queue_polled)); return controller; } diff --git a/Kernel/Storage/NVMe/NVMeController.h b/Kernel/Storage/NVMe/NVMeController.h index e07d447acf..b2e27f5cb9 100644 --- a/Kernel/Storage/NVMe/NVMeController.h +++ b/Kernel/Storage/NVMe/NVMeController.h @@ -25,7 +25,7 @@ namespace Kernel { class NVMeController : public PCI::Device , public StorageController { public: - static ErrorOr> try_initialize(PCI::DeviceIdentifier const&, bool is_queue_polled); + static ErrorOr> try_initialize(PCI::DeviceIdentifier const&, bool is_queue_polled); ErrorOr initialize(bool is_queue_polled); LockRefPtr device(u32 index) const override; size_t devices_count() const override; diff --git a/Kernel/Storage/Ramdisk/Controller.cpp b/Kernel/Storage/Ramdisk/Controller.cpp index 495c070836..2893c737db 100644 --- a/Kernel/Storage/Ramdisk/Controller.cpp +++ b/Kernel/Storage/Ramdisk/Controller.cpp @@ -11,9 +11,9 @@ namespace Kernel { -NonnullLockRefPtr RamdiskController::initialize() +NonnullRefPtr RamdiskController::initialize() { - return adopt_lock_ref(*new RamdiskController()); + return adopt_ref(*new RamdiskController()); } bool RamdiskController::reset() diff --git a/Kernel/Storage/Ramdisk/Controller.h b/Kernel/Storage/Ramdisk/Controller.h index 64661663b2..4a92dce5a4 100644 --- a/Kernel/Storage/Ramdisk/Controller.h +++ b/Kernel/Storage/Ramdisk/Controller.h @@ -19,7 +19,7 @@ class AsyncBlockDeviceRequest; class RamdiskController final : public StorageController { public: - static NonnullLockRefPtr initialize(); + static NonnullRefPtr initialize(); virtual ~RamdiskController() override; virtual LockRefPtr device(u32 index) const override; diff --git a/Kernel/Storage/StorageManagement.h b/Kernel/Storage/StorageManagement.h index bdc30935ed..7e21376c92 100644 --- a/Kernel/Storage/StorageManagement.h +++ b/Kernel/Storage/StorageManagement.h @@ -66,7 +66,7 @@ private: StringView m_boot_argument; LockWeakPtr m_boot_block_device; - Vector> m_controllers; + Vector> m_controllers; IntrusiveList<&StorageDevice::m_list_node> m_storage_devices; };