Skip to content

Commit cda8f34

Browse files
committed
Kernel/TmpFS: Remove inode map from TmpFS
The HashMap of InodeIndex->Inode in TmpFS only had one purpose: looking up parent inodes by index. Instead of using a map for this, we can simply give each inode a WeakPtr to its parent inode. This saves us the trouble of dealing with the fallibility of HashMap allocations, and it just generally simpler. :^)
1 parent 2d9bd24 commit cda8f34

File tree

2 files changed

+16
-55
lines changed

2 files changed

+16
-55
lines changed

Kernel/FileSystem/TmpFS.cpp

Lines changed: 13 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -35,47 +35,17 @@ Inode& TmpFS::root_inode()
3535
return *m_root_inode;
3636
}
3737

38-
void TmpFS::register_inode(TmpFSInode& inode)
39-
{
40-
VERIFY(inode.identifier().fsid() == fsid());
41-
42-
Inode::all_instances().with([&](auto&) {
43-
auto index = inode.identifier().index();
44-
m_inodes.set(index, &inode);
45-
});
46-
}
47-
48-
void TmpFS::unregister_inode(InodeIdentifier identifier)
49-
{
50-
VERIFY(identifier.fsid() == fsid());
51-
52-
Inode::all_instances().with([&](auto&) {
53-
m_inodes.remove(identifier.index());
54-
});
55-
}
56-
5738
unsigned TmpFS::next_inode_index()
5839
{
5940
MutexLocker locker(m_lock);
6041

6142
return m_next_inode_index++;
6243
}
6344

64-
ErrorOr<NonnullRefPtr<Inode>> TmpFS::get_inode(InodeIdentifier identifier) const
65-
{
66-
return Inode::all_instances().with([&](auto&) -> ErrorOr<NonnullRefPtr<Inode>> {
67-
VERIFY(identifier.fsid() == fsid());
68-
auto it = m_inodes.find(identifier.index());
69-
if (it == m_inodes.end())
70-
return ENOENT;
71-
return *it->value;
72-
});
73-
}
74-
75-
TmpFSInode::TmpFSInode(TmpFS& fs, const InodeMetadata& metadata, InodeIdentifier parent)
45+
TmpFSInode::TmpFSInode(TmpFS& fs, const InodeMetadata& metadata, WeakPtr<TmpFSInode> parent)
7646
: Inode(fs, fs.next_inode_index())
7747
, m_metadata(metadata)
78-
, m_parent(parent)
48+
, m_parent(move(parent))
7949
{
8050
m_metadata.inode = identifier();
8151
}
@@ -84,11 +54,9 @@ TmpFSInode::~TmpFSInode()
8454
{
8555
}
8656

87-
ErrorOr<NonnullRefPtr<TmpFSInode>> TmpFSInode::try_create(TmpFS& fs, InodeMetadata const& metadata, InodeIdentifier parent)
57+
ErrorOr<NonnullRefPtr<TmpFSInode>> TmpFSInode::try_create(TmpFS& fs, InodeMetadata const& metadata, WeakPtr<TmpFSInode> parent)
8858
{
89-
auto inode = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) TmpFSInode(fs, metadata, parent)));
90-
fs.register_inode(inode);
91-
return inode;
59+
return adopt_nonnull_ref_or_enomem(new (nothrow) TmpFSInode(fs, metadata, move(parent)));
9260
}
9361

9462
ErrorOr<NonnullRefPtr<TmpFSInode>> TmpFSInode::try_create_root(TmpFS& fs)
@@ -99,7 +67,7 @@ ErrorOr<NonnullRefPtr<TmpFSInode>> TmpFSInode::try_create_root(TmpFS& fs)
9967
metadata.ctime = now;
10068
metadata.mtime = now;
10169
metadata.mode = S_IFDIR | S_ISVTX | 0777;
102-
return try_create(fs, metadata, { fs.fsid(), 1 });
70+
return try_create(fs, metadata, {});
10371
}
10472

10573
InodeMetadata TmpFSInode::metadata() const
@@ -117,7 +85,8 @@ ErrorOr<void> TmpFSInode::traverse_as_directory(Function<ErrorOr<void>(FileSyste
11785
return ENOTDIR;
11886

11987
TRY(callback({ ".", identifier(), 0 }));
120-
TRY(callback({ "..", m_parent, 0 }));
88+
if (auto parent = m_parent.strong_ref())
89+
TRY(callback({ "..", parent->identifier(), 0 }));
12190

12291
for (auto& child : m_children) {
12392
TRY(callback({ child.name->view(), child.inode->identifier(), 0 }));
@@ -194,8 +163,11 @@ ErrorOr<NonnullRefPtr<Inode>> TmpFSInode::lookup(StringView name)
194163

195164
if (name == ".")
196165
return *this;
197-
if (name == "..")
198-
return fs().get_inode(m_parent);
166+
if (name == "..") {
167+
if (auto parent = m_parent.strong_ref())
168+
return parent.release_nonnull();
169+
return ENOENT;
170+
}
199171

200172
auto* child = find_child_by_name(name);
201173
if (!child)
@@ -260,7 +232,7 @@ ErrorOr<NonnullRefPtr<Inode>> TmpFSInode::create_child(StringView name, mode_t m
260232
metadata.ctime = now;
261233
metadata.mtime = now;
262234

263-
auto child = TRY(TmpFSInode::try_create(fs(), metadata, identifier()));
235+
auto child = TRY(TmpFSInode::try_create(fs(), metadata, *this));
264236
TRY(add_child(*child, name, mode));
265237
return child;
266238
}
@@ -365,9 +337,4 @@ ErrorOr<void> TmpFSInode::set_mtime(time_t t)
365337
return {};
366338
}
367339

368-
void TmpFSInode::remove_from_secondary_lists()
369-
{
370-
fs().unregister_inode(identifier());
371-
}
372-
373340
}

Kernel/FileSystem/TmpFS.h

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,6 @@ class TmpFS final : public FileSystem {
3333

3434
RefPtr<TmpFSInode> m_root_inode;
3535

36-
HashMap<InodeIndex, TmpFSInode*> m_inodes;
37-
ErrorOr<NonnullRefPtr<Inode>> get_inode(InodeIdentifier identifier) const;
38-
void register_inode(TmpFSInode&);
39-
void unregister_inode(InodeIdentifier);
40-
4136
unsigned m_next_inode_index { 1 };
4237
unsigned next_inode_index();
4338
};
@@ -67,11 +62,10 @@ class TmpFSInode final : public Inode {
6762
virtual ErrorOr<void> set_atime(time_t) override;
6863
virtual ErrorOr<void> set_ctime(time_t) override;
6964
virtual ErrorOr<void> set_mtime(time_t) override;
70-
virtual void remove_from_secondary_lists() override;
7165

7266
private:
73-
TmpFSInode(TmpFS& fs, const InodeMetadata& metadata, InodeIdentifier parent);
74-
static ErrorOr<NonnullRefPtr<TmpFSInode>> try_create(TmpFS&, InodeMetadata const& metadata, InodeIdentifier parent);
67+
TmpFSInode(TmpFS& fs, const InodeMetadata& metadata, WeakPtr<TmpFSInode> parent);
68+
static ErrorOr<NonnullRefPtr<TmpFSInode>> try_create(TmpFS&, InodeMetadata const& metadata, WeakPtr<TmpFSInode> parent);
7569
static ErrorOr<NonnullRefPtr<TmpFSInode>> try_create_root(TmpFS&);
7670

7771
struct Child {
@@ -84,7 +78,7 @@ class TmpFSInode final : public Inode {
8478
Child* find_child_by_name(StringView);
8579

8680
InodeMetadata m_metadata;
87-
InodeIdentifier m_parent;
81+
WeakPtr<TmpFSInode> m_parent;
8882

8983
OwnPtr<KBuffer> m_content;
9084

0 commit comments

Comments
 (0)