tbodt · November 24, 2019 23:08
diff --git a/inode-race-notes.txt b/inode-race-notes.txt
 SEQUENCE OF EVENTS
 ==================

 - /file has inode 1.
 - Thread A opens /file and creates the in-memory inode 1.
 - Thread A closes its file handle and enters the code path to destroy the in-memory inode 1, but is preempted after releasing inode_lock and before calling inode_orphaned.
 - Thread B opens /file and re-creates the in-memory inode 1.
 - Thread B renames /file2, which has inode 2, to /file. This removes the last reference to inode 1 in the database.
 - Thread A wakes up and goes to finish the destruction of inode 1, which removes it from the database since it has no more references.
 - Thread B calls fstat on its open handle to inode 1. This fails because it's gone from the database.

 WORRIES ABOUT GENERIC_OPEN
 ==========================

 - Thread A opens a file, but is preempted after releasing the filesystem lock and before returning to generic_open.
 - Thread B deletes the file. generic_open hasn't managed to take the reference to the in-memory inode yet, so the inode is deleted from the database.
 - Thread A returns to generic_open, which calls fstat, which fails because the inode is gone.

 why have I never seen this?

 --------------

 let's simplify

 A: open the file
 A: close the file. inode_release but preempted before inode_orphaned
 B: open the file, delete the file
 A: wake up, run inode_orphaned

 A: in inode_release between list_remove and inode_orphaned. has no locks, having released inodes_lock and inode->lock.
 B: opens the file, deletes the file. now has a handle to a deleted file, which should be fine and keep the inode alive.
 A: wakes up and calls inode_orphaned.
 B: calls fstat, crashes because the inode is gone.
 what should happen: B in generic_open should wait for A in inode_release to call inode_orphaned.

 A: in generic_open between fakefs_open and inode_get.
 B: deletes the file. inode_is_orphaned returns true, so the inode is removed from the db.
 A: now has a file handle for an inode that doesn't exist.
 what should happen: B in inode_is_orphaned should wait for A in generic_open to call inode_get.

 so there needs to be a lock around fakefs_open and inode_get in generic_open, around inode_is_orphaned and try_cleanup_inode in path_unlink, and around the entire contents of inode_release. all of this is only necessary for filesystems that use inode_orphaned. i think. this lock can be per-mount.

 say it's a new lock. since inodes_lock surrounds most of inode_release, inodes_lock has to nest inside it. since the mount may be locked by fakefs_open, mount->lock has to nest inside it. it doesn't need to nest inside anything that I can think of. sounds ok. what to call it, though?

 conclusion: some weird hack to make inodes_lock public for one reason.
	SEQUENCE OF EVENTS
	==================

	- /file has inode 1.
	- Thread A opens /file and creates the in-memory inode 1.
	- Thread A closes its file handle and enters the code path to destroy the in-memory inode 1, but is preempted after releasing inode_lock and before calling inode_orphaned.
	- Thread B opens /file and re-creates the in-memory inode 1.
	- Thread B renames /file2, which has inode 2, to /file. This removes the last reference to inode 1 in the database.
	- Thread A wakes up and goes to finish the destruction of inode 1, which removes it from the database since it has no more references.
	- Thread B calls fstat on its open handle to inode 1. This fails because it's gone from the database.

	WORRIES ABOUT GENERIC_OPEN
	==========================

	- Thread A opens a file, but is preempted after releasing the filesystem lock and before returning to generic_open.
	- Thread B deletes the file. generic_open hasn't managed to take the reference to the in-memory inode yet, so the inode is deleted from the database.
	- Thread A returns to generic_open, which calls fstat, which fails because the inode is gone.

	why have I never seen this?

	--------------

	let's simplify

	A: open the file
	A: close the file. inode_release but preempted before inode_orphaned
	B: open the file, delete the file
	A: wake up, run inode_orphaned

	A: in inode_release between list_remove and inode_orphaned. has no locks, having released inodes_lock and inode->lock.
	B: opens the file, deletes the file. now has a handle to a deleted file, which should be fine and keep the inode alive.
	A: wakes up and calls inode_orphaned.
	B: calls fstat, crashes because the inode is gone.
	what should happen: B in generic_open should wait for A in inode_release to call inode_orphaned.

	A: in generic_open between fakefs_open and inode_get.
	B: deletes the file. inode_is_orphaned returns true, so the inode is removed from the db.
	A: now has a file handle for an inode that doesn't exist.
	what should happen: B in inode_is_orphaned should wait for A in generic_open to call inode_get.

	so there needs to be a lock around fakefs_open and inode_get in generic_open, around inode_is_orphaned and try_cleanup_inode in path_unlink, and around the entire contents of inode_release. all of this is only necessary for filesystems that use inode_orphaned. i think. this lock can be per-mount.

	say it's a new lock. since inodes_lock surrounds most of inode_release, inodes_lock has to nest inside it. since the mount may be locked by fakefs_open, mount->lock has to nest inside it. it doesn't need to nest inside anything that I can think of. sounds ok. what to call it, though?

	conclusion: some weird hack to make inodes_lock public for one reason.