[PATCH] ext3 block allocator locking fix

When the BKL was removed from ext3 we lost locking coverage for
get_block()-versus-get_block().  Nobody seems to have hit the race because
get_block() almost always runs under i_sem: only memory pressure-based
writeout over a file hole runs outside i_sem.

ext2 uses the dedicated i_meta_lock spinlock in the inode to provide the
needed locking.  But ext3 already has an rwsem around all the get_block()
activity to protect it from truncate-related races.

So this patch just converts that rwsem into a semaphore, so concurrent
get_block() can never occur.  This will be more efficient than adding the new
spinlock.

We lose the ability to have two threads run get_block() against the same file
at the same time but again, that only happens during pageout over a hole
anyway.

(Kudos Alex Tomas for noticing the bug)

fs/ext3/inode.c

@@ -812,15 +812,17 @@ ext3_get_block_handle(handle_t *handle, struct inode *inode, sector_t iblock,
 	if (err == -EAGAIN)
 		goto changed;
 
-	if (ext3_find_goal(inode, iblock, chain, partial, &goal) < 0)
+	down(&ei->truncate_sem);
+	if (ext3_find_goal(inode, iblock, chain, partial, &goal) < 0) {
+		up(&ei->truncate_sem);
 		goto changed;
+	}
 
 	left = (chain + depth) - partial;
 
 	/*
 	 * Block out ext3_truncate while we alter the tree
 	 */
-	down_read(&ei->truncate_sem);
 	err = ext3_alloc_branch(handle, inode, left, goal,
 					offsets+(partial-chain), partial);
 
@@ -832,7 +834,7 @@ ext3_get_block_handle(handle_t *handle, struct inode *inode, sector_t iblock,
 	if (!err)
 		err = ext3_splice_branch(handle, inode, iblock, chain,
 					 partial, left);
-	up_read(&ei->truncate_sem);
+	up(&ei->truncate_sem);
 	if (err == -EAGAIN)
 		goto changed;
 	if (err)
@@ -2205,7 +2207,7 @@ void ext3_truncate(struct inode * inode)
 	 * From here we block out all ext3_get_block() callers who want to
 	 * modify the block allocation tree.
 	 */
-	down_write(&ei->truncate_sem);
+	down(&ei->truncate_sem);
 
 	if (n == 1) {		/* direct blocks */
 		ext3_free_data(handle, inode, NULL, i_data+offsets[0],
@@ -2269,7 +2271,7 @@ void ext3_truncate(struct inode * inode)
 		case EXT3_TIND_BLOCK:
 			;
 	}
-	up_write(&ei->truncate_sem);
+	up(&ei->truncate_sem);
 	inode->i_mtime = inode->i_ctime = CURRENT_TIME;
 	ext3_mark_inode_dirty(handle, inode);
 

fs/ext3/super.c

@@ -460,7 +460,7 @@ static void init_once(void * foo, kmem_cache_t * cachep, unsigned long flags)
 #ifdef CONFIG_EXT3_FS_XATTR
 		init_rwsem(&ei->xattr_sem);
 #endif
-		init_rwsem(&ei->truncate_sem);
+		init_MUTEX(&ei->truncate_sem);
 		inode_init_once(&ei->vfs_inode);
 	}
 }

include/linux/ext3_fs_i.h

@@ -106,7 +106,7 @@ struct ext3_inode_info {
 	 * during recovery.  Hence we must fix the get_block-vs-truncate race
 	 * by other means, so we have truncate_sem.
 	 */
-	struct rw_semaphore truncate_sem;
+	struct semaphore truncate_sem;
 	struct inode vfs_inode;
 };