Re: [Xen-devel] [PATCH 1/2 linux-next] Revert "ufs: fix deadlocks introduced by sb mutex merge"

[Al Viro <viro@xxxxxxxxxxxxxxxxxx>]

On Fri, Jun 05, 2015 at 06:27:01PM +0200, Fabian Frederick wrote:

> You're asking to remove lock_ufs() in allocation and replace it by
> truncate_mutex. I guess you're talking about doing that on current rc
> (without s_lock restored).
> 
> I tried a quick patch on rc trying to convert lock_ufs()/unlock_ufs()
> with per inode truncate_mutex (see attachment).
> Is it going the right direction ?

Nope.  First of all, allocation should be protected by fs-wide mutex, for
obvious reasons.  What ->truncate_mutex is protecting (outside of that
one) is modifications of block pointers in inode and in indirect blocks.

Check what ext2 is doing.  It tries to follow pointers without taking
a lock.  If it succeeds, fine, no allocation is needed, just map it.
If it fails, it grabs ->truncate_mutex, follows pointers again (checking
the chain it followed before grabbing the lock first, in hope it has
remained valid) and does allocations and block pointer modifications,
with ->truncate_mutex making sure that nobody else would touch those
under it.  And on truncate side it deals with the page into which EOF
will fall, then (still holding ->i_mutex) does setting ->i_size and
eviction of pages beyond EOF (by truncate_setsize()), then does freeing
the blocks past the new EOF.  Which is where it grabs ->truncate_mutex
(inside __ext2_truncate_blocks()).  And it doesn't need to do those
insane retries - on get_block side we need to redo the pointer chasing
after having taken ->truncate_mutex (when it decides that it might need
to do allocation, after all), but that's done once; truncate side just
grabs ->truncate_mutex as soon as it gets to __ext2_truncate_blocks(),
which means that nobody can change anything under it.

Actual allocation/freeing of blocks (as well as that of inodes) is done
under fs-wide mutex, nesting inside the rest.

Basically, we have
        i_mutex: file size changes, contents-affecting syscalls.  Per-inode.
        truncate_mutex: block pointers changes.  Per-inode.
        s_lock: block and inode bitmaps changes.  Per-filesystem.

For UFS it's slightly more complicated due to tail packing they are doing for
short files, but most of that complexity is due to having that stuff handled
way too deep in call chain.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel