Patchwork ext4: correct the number of blocks that has been deallocated in ext4_free_hole_blocks

login
register
mail settings
Submitter Zheng Liu
Date Jan. 28, 2013, 3:44 a.m.
Message ID <1359344667-12068-1-git-send-email-wenqing.lz@taobao.com>
Download mbox | patch
Permalink /patch/216105/
State Accepted
Headers show

Comments

Zheng Liu - Jan. 28, 2013, 3:44 a.m.
From: Zheng Liu <wenqing.lz@taobao.com>

This commit fixes a bug in ext4_free_hole_blocks that iff the number of
hole blocks is equal to EXT4_NDIR_BLOCKS and the first block of the hole
is a direct block, the number of deallocated blocks is wrong.

This bug can be triggered by the following commands:

  dd if=/dev/zero of=/mnt/testfile bs=4k count=16
  xfs_io -c "fpunch 4k 48k" -c "fiemap -v" /mnt/testfile

[wrong result]
 EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
   0: [0..7]:          274560..274567       8 0x1000
   1: [8..95]:         hole                88
   2: [96..127]:       274656..274687      32 0x1001

[expected result]
 EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
   0: [0..7]:          278528..278535       8 0x1000
   1: [8..103]:        hole                96
   2: [104..127]:      278632..278655      24 0x1001

Moreover this commit calls ext4_es_remove_extents() to remove some
delayed extents from extent status tree because now extent status tree
has been applied in the lastest kernel.  Meanwhile we never disable
dioread_nolock in ext4_ind_punch_hole() because this feature couldn't be
enabled for a indirect-based file, and we don't flush any unwritten io
because for a indirect-based file it needn't convert unwritten extents.

Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
---
[This patch bases against dev branch]

 fs/ext4/indirect.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
Theodore Ts'o - Jan. 28, 2013, 2:22 p.m.
On Sun, Jan 27, 2013 at 05:44:27PM -0000, Zheng Liu wrote:
> From: Zheng Liu <wenqing.lz@taobao.com>
> 
> This commit fixes a bug in ext4_free_hole_blocks that iff the number of
> hole blocks is equal to EXT4_NDIR_BLOCKS and the first block of the hole
> is a direct block, the number of deallocated blocks is wrong.

Thanks!  I've merged this into the indirect punch hole patch, which is
in the dev branch and queued in the ext4 patch queue.

	     	    	      	  	   - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 3a33ca2..96a5c1f 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -1582,7 +1582,7 @@  static int ext4_free_hole_blocks(handle_t *handle, struct inode *inode,
 					       level, first, count, num);
 			if (ret)
 				goto err;
-			if (count > max)
+			if (count > max - first)
 				count -= max - first;
 			else
 				break;
@@ -1661,15 +1661,11 @@  int ext4_ind_punch_hole(struct file *file, loff_t offset, loff_t length)
 	}
 
 	/* Wait all existing dio works, newcomers will block on i_mutex */
-	ext4_inode_block_unlocked_dio(inode);
-	err = ext4_flush_unwritten_io(inode);
-	if (err)
-		goto out_dio;
 	inode_dio_wait(inode);
 
 	handle = start_transaction(inode);
 	if (IS_ERR(handle))
-		goto out_dio;
+		goto out_mutex;
 
 	/*
 	 * Now we need to zero out the non-page-aligned data in the
@@ -1738,6 +1734,8 @@  int ext4_ind_punch_hole(struct file *file, loff_t offset, loff_t length)
 	down_write(&EXT4_I(inode)->i_data_sem);
 	ext4_discard_preallocations(inode);
 
+	err = ext4_es_remove_extent(inode, first_block,
+				    stop_block - first_block);
 	err = ext4_free_hole_blocks(handle, inode, first_block, stop_block);
 
 	ext4_discard_preallocations(inode);
@@ -1752,8 +1750,6 @@  out:
 	ext4_mark_inode_dirty(handle, inode);
 	ext4_journal_stop(handle);
 
-out_dio:
-	ext4_inode_resume_unlocked_dio(inode);
 out_mutex:
 	mutex_unlock(&inode->i_mutex);