kernel/fs: Handle root vnode ownership correctly and add more assertions.

In 5b14757a30 (2005), a change was made
to have root nodes be owned by filesystems, rather than
by the VFS (as BeOS did.) However, later on,
in fe5928847a (2011),
the adding of covered vnodes broke that.

As put_vnode's error was silently ignored, this went unnoticed
until this week, when the new asserts started firing.

So, to fix this, increment the reference count of the root when
setting its covering vnode, and handle the root specially.
Assert in ~fs_mount that there are no vnodes left; if the
filesystem failed to free it, this assert will trip.

Fixes the new assert seen in #19642.
This commit is contained in:
Augustin Cavalier
2025-08-29 15:04:03 -04:00
parent ad49acb949
commit 6b86bc6ea3
+22 -4
View File
@@ -144,6 +144,8 @@ struct fs_mount {
~fs_mount()
{
ASSERT(vnodes.IsEmpty());
mutex_destroy(&lock);
free(device_name);
@@ -1069,6 +1071,9 @@ dec_vnode_ref_count(struct vnode* vnode, bool alwaysFree, bool reenter)
if (vnode->IsBusy())
panic("dec_vnode_ref_count: called on busy vnode %p\n", vnode);
if (vnode->mount->unmounting)
alwaysFree = true;
bool freeNode = false;
bool freeUnusedNodes = false;
@@ -1359,7 +1364,7 @@ free_unused_vnodes(int32 level)
if (vnode != sUnusedVnodeList.First())
continue;
ASSERT(!vnode->IsBusy());
ASSERT(!vnode->IsBusy() && vnode->ref_count == 0);
// grab a reference
inc_vnode_ref_count(vnode);
@@ -7732,6 +7737,7 @@ fs_mount(char* path, const char* device, const char* fsName, uint32 flags,
coveredNode->covered_by = mount->root_vnode;
coveredNode->SetCovered(true);
inc_vnode_ref_count(mount->root_vnode);
}
rw_lock_write_unlock(&sVnodeLock);
@@ -7864,6 +7870,8 @@ fs_unmount(char* path, dev_t mountID, uint32 flags, bool kernel)
refCount--;
if (vnode->covered_by != NULL)
refCount--;
if (vnode == mount->root_vnode)
refCount--;
if (refCount != 0) {
dprintf("fs_unmount(): inode %" B_PRIdINO " is still referenced\n", vnode->id);
@@ -7955,6 +7963,9 @@ fs_unmount(char* path, dev_t mountID, uint32 flags, bool kernel)
}
}
if (vnode == mount->root_vnode)
continue;
vnode->SetBusy(true);
vnode_to_be_freed(vnode);
}
@@ -7962,8 +7973,6 @@ fs_unmount(char* path, dev_t mountID, uint32 flags, bool kernel)
vnodesWriteLocker.Unlock();
// Free all vnodes associated with this mount.
// They will be removed from the mount list by free_vnode(), so
// we don't have to do this.
while (struct vnode* vnode = mount->vnodes.Head()) {
// Put the references to external covered/covering vnodes we kept above.
if (Vnode* coveredNode = vnode->covers)
@@ -7971,9 +7980,18 @@ fs_unmount(char* path, dev_t mountID, uint32 flags, bool kernel)
if (Vnode* coveringNode = vnode->covered_by)
put_vnode(coveringNode);
free_vnode(vnode, false);
// free_vnode() removes nodes from the mount list. However, the root
// will still be referenced by the FS, so we can't free it yet.
if (vnode == mount->root_vnode)
remove_vnode_from_mount_list(vnode, mount);
else
free_vnode(vnode, false);
}
// Re-add the root to the mount list, so it can be freed.
add_vnode_to_mount_list(mount->root_vnode, mount);
mount->root_vnode = NULL;
// remove the mount structure from the hash table
rw_lock_write_lock(&sMountLock);
sMountsTable->Remove(mount);