[haiku-bugs] Re: [Haiku] #10522: Assert failed in TrashWatcher
- From: "bonefish" <trac@xxxxxxxxxxxx>
- Date: Mon, 09 Jun 2014 08:57:39 -0000
#10522: Assert failed in TrashWatcher
-----------------------------+----------------------------
Reporter: kallisti5 | Owner: axeld
Type: bug | Status: new
Priority: normal | Milestone: R1
Component: System/Kernel | Version: R1/Development
Resolution: | Keywords: vfs
Blocked By: | Blocking:
Has a Patch: 0 | Platform: x86-64
-----------------------------+----------------------------
Comment (by bonefish):
Replying to [comment:10 korli]:
> I had a look at the code:
> * [http://cgit.haiku-os.org/haiku/tree/src/system/kernel/fs/vfs.cpp#n689
get_mount()] acquires a read lock for sVnodeLock and a mutex lock for
sMountMutex
> * [http://cgit.haiku-
os.org/haiku/tree/src/system/kernel/fs/vfs.cpp#n1084
inc_vnode_ref_count()] requires sVnodeLock read locked and the vnode's
lock, or sVnodeLock write locked.
As the documentation says, an alternative is "ensuring that a reference to
the node exists and remains in existence". As long as the mount isn't
unmounted (`fs_unmount()`) it owns a reference to the root node. So,
ensuring that the mount isn't unmounted would satisfy the condition.
`fs_unmount()` checks the ref counts of all nodes (must be 0, respectively
1 for the root node) and then marks them busy. All while holding
`sVnodeLock` write-locked (well, safe for the force unmount case, which I
believe to be broken anyway).
So, by holding `sVnodeLock` read-locked `get_mount()` ensures that
`fs_unmounted()` can't process the nodes. If it is already past that
point, the root node check (not NULL, not busy, ref count > 0) is supposed
to detect that. But it doesn't look like this can work. `fs_unmount()`
doesn't set the root node to NULL (the root node field is NULL only during
a short period in `fs_mount()`), but it just frees the nodes after
releasing `sVnodeLock`. So the busy and ref count > 0 checks could already
access freed memory.
> Other questions are:
> * why we increment the refcount on {{{mount->root_vnode}}} and not
{{{rootNode}}}.
It doesn't make a difference. The `root_vnode` field is immutable after
`fs_mount()` sets it to non-NULL. For performance reasons one could change
the check.
> * what to do if the rootNode is unused after acquiring the lock.
The lock doesn't need to be acquired. Besides, if the root node is unused,
it is too late already, since it would be (or already has been) freed at
any time.
I would change the root node check in `get_mount()` to first check whether
the mount's `unmounting` flag is already set.
--
Ticket URL: <https://dev.haiku-os.org/ticket/10522#comment:12>
Haiku <https://dev.haiku-os.org>
Haiku - the operating system.
Other related posts:
