On Thu, Dec 10, 2009 at 09:28:28AM -0800, Curt Wohlgemuth wrote:
> This fixes a bug in which new blocks returned from an extent created with
> ext4_ext_zeroout() can have dirty metadata still associated with them.
>
> Signed-off-by: Curt Wohlgemuth <curtw@xxxxxxxxxx>
> ---
>
> This is for the problem I reported on 23 Nov ("Bug in extent zeroout: blocks
> not marked as new"). I'm not seeing the corruption with this fix that I was
> seeing without it.
>
> diff -uprN orig/fs/ext4/extents.c new/fs/ext4/extents.c
> --- orig/fs/ext4/extents.c 2009-12-09 15:09:25.000000000 -0800
> +++ new/fs/ext4/extents.c 2009-12-09 15:09:37.000000000 -0800
> @@ -2474,9 +2474,21 @@ static int ext4_ext_zeroout(struct inode
> submit_bio(WRITE, bio);
> wait_for_completion(&event);
>
> - if (test_bit(BIO_UPTODATE, &bio->bi_flags))
> + /* On success, we need to insure all metadata associated
> + * with each of these blocks is unmapped. */
> + if (test_bit(BIO_UPTODATE, &bio->bi_flags)) {
> + sector_t block = ee_pblock;
> +
> ret = 0;
> - else {
> + done = 0;
> + while (done < len) {
> + unmap_underlying_metadata(inode->i_sb->s_bdev,
> + block);
> +
> + done++;
> + block++;
> + }
> + } else {
> ret = -EIO;
> break;
> }
a) We are zeroing out 'done' blocks but you are unmapping 'len' blocks ?
b) We are already doing a unmap in mpage_da_map_blocks so i guess
what you want is to unmap the extra block allocated
c) ee_pblock is in 512 byte units so the block number is wrong.
how about the patch below ?
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 3a7928f..f9a735f 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3023,6 +3023,14 @@ out:
return err;
}
+static void unmap_underlying_metadata_blocks(struct block_device *bdev,
+ sector_t block, int count)
+{
+ int i;
+ for (i = 0; i < count; i++)
+ unmap_underlying_metadata(bdev, block + i);
+}
+
static int
ext4_ext_handle_uninitialized_extents(handle_t *handle, struct inode *inode,
ext4_lblk_t iblock, unsigned int max_blocks,
@@ -3098,6 +3106,18 @@ out:
} else
allocated = ret;
set_buffer_new(bh_result);
+ /*
+ * if we allocated more blocks than requested
+ * we need to make sure we unmap the extra block
+ * allocated. The actual needed block will get
+ * unmapped later when we find the buffer_head marked
+ * new.
+ */
+ if (allocated > max_blocks) {
+ unmap_underlying_metadata_blocks(inode->i_sb->s_bdev,
+ newblock + max_blocks,
+ allocated - max_blocks);
+ }
map_out:
set_buffer_mapped(bh_result);
out1:
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
