Skip to content

Commit b02ee66

Browse files
supercomputer7linusg
authored andcommitted
Kernel: Get rid of *LockRefPtr in the SysFS filesystem code
To do this we also need to get rid of LockRefPtrs in the USB code as well. Most of the SysFS nodes are statically generated during boot and are not mutated afterwards. The same goes for general device code - once we generate the appropriate SysFS nodes, we almost never mutate the node pointers afterwards, making locking unnecessary.
1 parent dd7633c commit b02ee66

File tree

114 files changed

+230
-218
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

114 files changed

+230
-218
lines changed

Kernel/Bus/USB/USBDevice.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ ErrorOr<NonnullLockRefPtr<Device>> Device::try_create(USBController const& contr
2121
auto pipe = TRY(ControlPipe::create(controller, 0, 8, 0));
2222
auto device = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) Device(controller, port, speed, move(pipe))));
2323
auto sysfs_node = TRY(SysFSUSBDeviceInformation::create(*device));
24-
device->m_sysfs_device_info_node = move(sysfs_node);
24+
device->m_sysfs_device_info_node.with([&](auto& node) {
25+
node = move(sysfs_node);
26+
});
2527
TRY(device->enumerate_device());
2628
return device;
2729
}

Kernel/Bus/USB/USBDevice.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <AK/Vector.h>
1313
#include <Kernel/Bus/USB/USBConfiguration.h>
1414
#include <Kernel/Bus/USB/USBPipe.h>
15+
#include <Kernel/Locking/SpinlockProtected.h>
1516

1617
namespace Kernel {
1718
class SysFSUSBDeviceInformation;
@@ -57,7 +58,7 @@ class Device : public AtomicRefCounted<Device> {
5758

5859
Vector<USBConfiguration> const& configurations() const { return m_configurations; }
5960

60-
SysFSUSBDeviceInformation& sysfs_device_info_node(Badge<USB::Hub>) { return *m_sysfs_device_info_node; }
61+
SpinlockProtected<RefPtr<SysFSUSBDeviceInformation>, LockRank::None>& sysfs_device_info_node(Badge<USB::Hub>) { return m_sysfs_device_info_node; }
6162

6263
protected:
6364
Device(NonnullLockRefPtr<USBController> controller, u8 address, u8 port, DeviceSpeed speed, NonnullOwnPtr<ControlPipe> default_pipe);
@@ -79,7 +80,7 @@ class Device : public AtomicRefCounted<Device> {
7980
IntrusiveListNode<Device, NonnullLockRefPtr<Device>> m_hub_child_node;
8081

8182
protected:
82-
LockRefPtr<SysFSUSBDeviceInformation> m_sysfs_device_info_node;
83+
SpinlockProtected<RefPtr<SysFSUSBDeviceInformation>, LockRank::None> m_sysfs_device_info_node;
8384

8485
public:
8586
using List = IntrusiveList<&Device::m_hub_child_node>;

Kernel/Bus/USB/USBHub.cpp

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@ ErrorOr<void> Hub::enumerate_and_power_on_hub()
4646
// USBDevice::enumerate_device must be called before this.
4747
VERIFY(m_address > 0);
4848

49-
m_sysfs_device_info_node = TRY(SysFSUSBDeviceInformation::create(*this));
49+
TRY(m_sysfs_device_info_node.with([&](auto& node) -> ErrorOr<void> {
50+
node = TRY(SysFSUSBDeviceInformation::create(*this));
51+
return {};
52+
}));
5053

5154
if (m_device_descriptor.device_class != USB_CLASS_HUB) {
5255
dbgln("USB Hub: Trying to enumerate and power on a device that says it isn't a hub.");
@@ -133,8 +136,11 @@ ErrorOr<void> Hub::set_port_feature(u8 port, HubFeatureSelector feature_selector
133136

134137
void Hub::remove_children_from_sysfs()
135138
{
136-
for (auto& child : m_children)
137-
SysFSUSBBusDirectory::the().unplug({}, child.sysfs_device_info_node({}));
139+
for (auto& child : m_children) {
140+
child.sysfs_device_info_node({}).with([](auto& node) {
141+
SysFSUSBBusDirectory::the().unplug({}, *node);
142+
});
143+
}
138144
}
139145

140146
void Hub::check_for_port_updates()
@@ -260,10 +266,14 @@ void Hub::check_for_port_updates()
260266

261267
auto hub = hub_or_error.release_value();
262268
m_children.append(hub);
263-
SysFSUSBBusDirectory::the().plug({}, hub->sysfs_device_info_node({}));
269+
hub->sysfs_device_info_node({}).with([](auto& node) {
270+
SysFSUSBBusDirectory::the().plug({}, *node);
271+
});
264272
} else {
265273
m_children.append(device);
266-
SysFSUSBBusDirectory::the().plug({}, device->sysfs_device_info_node({}));
274+
device->sysfs_device_info_node({}).with([](auto& node) {
275+
SysFSUSBBusDirectory::the().plug({}, *node);
276+
});
267277
}
268278

269279
} else {
@@ -278,7 +288,9 @@ void Hub::check_for_port_updates()
278288
}
279289

280290
if (device_to_remove) {
281-
SysFSUSBBusDirectory::the().unplug({}, device_to_remove->sysfs_device_info_node({}));
291+
device_to_remove->sysfs_device_info_node({}).with([](auto& node) {
292+
SysFSUSBBusDirectory::the().unplug({}, *node);
293+
});
282294
if (device_to_remove->device_descriptor().device_class == USB_CLASS_HUB) {
283295
auto* hub_child = static_cast<Hub*>(device_to_remove.ptr());
284296
hub_child->remove_children_from_sysfs();

Kernel/Devices/Device.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,19 @@ void Device::after_inserting_add_to_device_management()
3434

3535
ErrorOr<void> Device::after_inserting()
3636
{
37-
after_inserting_add_to_device_management();
3837
VERIFY(!m_sysfs_component);
3938
auto sys_fs_component = SysFSDeviceComponent::must_create(*this);
4039
m_sysfs_component = sys_fs_component;
4140
after_inserting_add_to_device_identifier_directory();
41+
after_inserting_add_to_device_management();
4242
return {};
4343
}
4444

4545
void Device::will_be_destroyed()
4646
{
4747
VERIFY(m_sysfs_component);
48-
before_will_be_destroyed_remove_from_device_identifier_directory();
4948
before_will_be_destroyed_remove_from_device_management();
49+
before_will_be_destroyed_remove_from_device_identifier_directory();
5050
}
5151

5252
Device::~Device()

Kernel/Devices/Device.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,10 @@ class Device : public File {
9696
protected:
9797
// FIXME: This pointer will be eventually removed after all nodes in /sys/dev/block/ and
9898
// /sys/dev/char/ are symlinks.
99-
LockRefPtr<SysFSDeviceComponent> m_sysfs_component;
99+
RefPtr<SysFSDeviceComponent> m_sysfs_component;
100100

101-
LockRefPtr<SysFSSymbolicLinkDeviceComponent> m_symlink_sysfs_component;
102-
LockRefPtr<SysFSDirectory> m_sysfs_device_directory;
101+
RefPtr<SysFSSymbolicLinkDeviceComponent> m_symlink_sysfs_component;
102+
RefPtr<SysFSDirectory> m_sysfs_device_directory;
103103
};
104104

105105
}

Kernel/FileSystem/SysFS/Component.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,6 @@ ErrorOr<size_t> SysFSSymbolicLink::read_bytes(off_t offset, size_t count, UserOr
7474
ErrorOr<NonnullOwnPtr<KBuffer>> SysFSSymbolicLink::try_to_generate_buffer() const
7575
{
7676
auto return_path_to_mount_point = TRY(try_generate_return_path_to_mount_point());
77-
if (!m_pointed_component)
78-
return Error::from_errno(EIO);
7977
auto pointed_component_base_name = MUST(KString::try_create(m_pointed_component->name()));
8078
auto pointed_component_relative_path = MUST(m_pointed_component->relative_path(move(pointed_component_base_name), 0));
8179
auto full_return_and_target_path = TRY(KString::formatted("{}{}", return_path_to_mount_point->view(), pointed_component_relative_path->view()));
@@ -126,9 +124,9 @@ ErrorOr<void> SysFSDirectory::traverse_as_directory(FileSystemID fsid, Function<
126124
});
127125
}
128126

129-
LockRefPtr<SysFSComponent> SysFSDirectory::lookup(StringView name)
127+
RefPtr<SysFSComponent> SysFSDirectory::lookup(StringView name)
130128
{
131-
return m_child_components.with([&](auto& list) -> LockRefPtr<SysFSComponent> {
129+
return m_child_components.with([&](auto& list) -> RefPtr<SysFSComponent> {
132130
for (auto& child_component : list) {
133131
if (child_component.name() == name) {
134132
return child_component;

Kernel/FileSystem/SysFS/Component.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@
99
#include <AK/AtomicRefCounted.h>
1010
#include <AK/Error.h>
1111
#include <AK/Function.h>
12+
#include <AK/RefPtr.h>
1213
#include <AK/StringView.h>
1314
#include <AK/Types.h>
1415
#include <Kernel/FileSystem/File.h>
1516
#include <Kernel/FileSystem/FileSystem.h>
1617
#include <Kernel/FileSystem/OpenFileDescription.h>
1718
#include <Kernel/Forward.h>
18-
#include <Kernel/Library/LockRefPtr.h>
1919

2020
namespace Kernel {
2121

@@ -31,7 +31,7 @@ class SysFSComponent : public AtomicRefCounted<SysFSComponent> {
3131
virtual StringView name() const = 0;
3232
virtual ErrorOr<size_t> read_bytes(off_t, size_t, UserOrKernelBuffer&, OpenFileDescription*) const { return Error::from_errno(ENOTIMPL); }
3333
virtual ErrorOr<void> traverse_as_directory(FileSystemID, Function<ErrorOr<void>(FileSystem::DirectoryEntryView const&)>) const { VERIFY_NOT_REACHED(); }
34-
virtual LockRefPtr<SysFSComponent> lookup(StringView) { VERIFY_NOT_REACHED(); };
34+
virtual RefPtr<SysFSComponent> lookup(StringView) { VERIFY_NOT_REACHED(); };
3535
virtual mode_t permissions() const;
3636
virtual ErrorOr<void> truncate(u64) { return EPERM; }
3737
virtual size_t size() const { return 0; }
@@ -51,9 +51,9 @@ class SysFSComponent : public AtomicRefCounted<SysFSComponent> {
5151
explicit SysFSComponent(SysFSDirectory const& parent_directory);
5252
SysFSComponent();
5353

54-
LockRefPtr<SysFSDirectory> m_parent_directory;
54+
RefPtr<SysFSDirectory> const m_parent_directory;
5555

56-
IntrusiveListNode<SysFSComponent, NonnullLockRefPtr<SysFSComponent>> m_list_node;
56+
IntrusiveListNode<SysFSComponent, NonnullRefPtr<SysFSComponent>> m_list_node;
5757

5858
private:
5959
InodeIndex m_component_index {};
@@ -70,13 +70,13 @@ class SysFSSymbolicLink : public SysFSComponent {
7070

7171
explicit SysFSSymbolicLink(SysFSDirectory const& parent_directory, SysFSComponent const& pointed_component);
7272

73-
LockRefPtr<SysFSComponent> m_pointed_component;
73+
NonnullRefPtr<SysFSComponent> const m_pointed_component;
7474
};
7575

7676
class SysFSDirectory : public SysFSComponent {
7777
public:
7878
virtual ErrorOr<void> traverse_as_directory(FileSystemID, Function<ErrorOr<void>(FileSystem::DirectoryEntryView const&)>) const override final;
79-
virtual LockRefPtr<SysFSComponent> lookup(StringView name) override final;
79+
virtual RefPtr<SysFSComponent> lookup(StringView name) override final;
8080

8181
virtual ErrorOr<NonnullRefPtr<SysFSInode>> to_inode(SysFS const& sysfs_instance) const override final;
8282

Kernel/FileSystem/SysFS/Inode.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class SysFSInode : public Inode {
4040
virtual ErrorOr<void> attach(OpenFileDescription& description) override final;
4141
virtual void did_seek(OpenFileDescription&, off_t) override final;
4242

43-
NonnullLockRefPtr<SysFSComponent> m_associated_component;
43+
NonnullRefPtr<SysFSComponent> const m_associated_component;
4444
};
4545

4646
}

Kernel/FileSystem/SysFS/Registry.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ SysFSBusDirectory& SysFSComponentRegistry::buses_directory()
4545

4646
void SysFSComponentRegistry::register_new_bus_directory(SysFSDirectory& new_bus_directory)
4747
{
48-
VERIFY(!m_root_directory->m_buses_directory.is_null());
4948
MUST(m_root_directory->m_buses_directory->m_child_components.with([&](auto& list) -> ErrorOr<void> {
5049
list.append(new_bus_directory);
5150
return {};

Kernel/FileSystem/SysFS/Registry.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class SysFSComponentRegistry {
3131
SysFSBusDirectory& buses_directory();
3232

3333
private:
34-
NonnullLockRefPtr<SysFSRootDirectory> m_root_directory;
34+
NonnullRefPtr<SysFSRootDirectory> const m_root_directory;
3535
Spinlock<LockRank::None> m_root_directory_lock {};
3636
};
3737

0 commit comments

Comments
 (0)