[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2 linux-next] Revert "ufs: fix deadlocks introduced by sb mutex merge"
> On 05 June 2015 at 20:50 Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > > > 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. In your two explanations, there's only a place for one sb mutex: " 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. " " per-page exclusion for reallocation time (normal page locks are doing that) per-fs exclusion for block and fragment allocations (->s_lock?) per-fs exclusion for inode allocations (->s_lock?) per-inode exclusion for mapping changes (a-la ext2 truncate_mutex) per-directory exclusion for contents access (->i_mutex gives that)    " Meanwhile current linux-next has 2 in fs/ufs/ufs.h: struct ufs_sb_info struct mutex mutex struct mutex s_lock So commit 0244756edc4b98c129e "ufs: sb mutex merge + mutex_destroy") was finally not a bad thing but maybe it removed the bad one ? Regards, Fabian _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |