Re: Race between __sync_single_inode() and LogFS garbage collector

["Jörn Engel" <joern@lazybastard.org>]

On Mon, 19 February 2007 21:06:33 -0600, Dave Kleikamp wrote:
> 
> You've obviously given this a lot more thought that I have, but this
> sounds like something that has possibilities.  You couldn't implement
> your own drop_inode method that does better locking against the garbage
> collector?

Bloody hell!  What a great question for an unexpected reason.  GC also
races against iput().

Basically whenever anyone locks any inode, I may have a problem.  If
that inode remains locked until some writeout happens, I will have a
problem.  Afaics, locking happens in three places:

__sync_single_inode()
	Causes writeout of inode pages and inode itself

iput()
	Causes writeout of inode itself

get_new_inode() or get_new_inode_fast()
	No writes from here

Both __sync_single_inode() and iput() would need to prevent GC before
locking the inode.  GC can be caused by any writes to LogFS, so they
need to lock against all write paths in LogFS.  Once they start writeout
themselves, they also need to pass information that they have taken the
lock onward, otherwise taking the lock itself would deadlock.  Nasty.

Alternatively there could be a non-blocking version of iget() that
simply returns when an inode is locked.  Now GC would have to spin in a
loop, waiting for one of two possible events.  Either the inode is
getting unlocked or someone calls into LogFS with the inode in question
and GC needs to take that by some means.  Again, nasty.

The more I think about it, the more I like the idea of double-caching
inodes.  As ugly as it sounds, it causes the least amount of problems.

Thanks, Shaggy!

Jörn

-- 
Linux [...] existed just for discussion between people who wanted
to show off how geeky they were.
-- Rob Enderle
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html