2008-03-04 12:43:52

by Aneesh Kumar K.V

[permalink] [raw]
Subject: [PATCH] ext4: Convert uninitialized extent to initialized extent in case of file system full

A write to prealloc area cause the split of unititalized extent
into a initialized and uninitialized extent. If we don't have
space to add new extent information instead of returning error
convert the existing uninitialized extent to initialized one. We
need to zero out the blocks corresponding to the extent to prevent
wrong data reaching userspace.

Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
fs/ext4/extents.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 99 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index d315cc1..839caf2 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2136,6 +2136,80 @@ void ext4_ext_release(struct super_block *sb)
#endif
}

+static void bi_complete(struct bio *bio, int error)
+{
+ complete((struct completion *)bio->bi_private);
+}
+
+/* FIXME!! we need to try to merge to left or right after zerout */
+static int ext4_ext_zeroout(struct inode *inode, struct ext4_extent *ex)
+{
+ int ret = -EIO;
+ struct bio *bio;
+ int blkbits, blocksize;
+ sector_t ee_pblock;
+ struct completion event;
+ unsigned int ee_len, len, done, offset;
+
+
+ blkbits = inode->i_blkbits;
+ blocksize = inode->i_sb->s_blocksize;
+ ee_len = ext4_ext_get_actual_len(ex);
+ ee_pblock = ext_pblock(ex);
+
+ /* convert ee_pblock in 512 byte sector */
+ ee_pblock = ee_pblock << (blkbits >> 9);
+
+
+ while (ee_len > 0) {
+
+ if (ee_len > BIO_MAX_PAGES)
+ len = BIO_MAX_PAGES;
+ else
+ len = ee_len;
+
+ bio = bio_alloc(GFP_NOIO, len);
+ if (!bio)
+ return -ENOMEM;
+ bio->bi_sector = ee_pblock;
+ bio->bi_bdev = inode->i_sb->s_bdev;
+
+ done = 0;
+ offset = 0;
+ while (done < len) {
+ ret = bio_add_page(bio, ZERO_PAGE(0),
+ blocksize, offset);
+ if (ret != blocksize) {
+ /* We can't add any more page because of
+ * hardware limitation. Start a new bio
+ */
+ break;
+ }
+ done++;
+ offset += blocksize;
+ if (offset >= PAGE_CACHE_SIZE)
+ offset = 0;
+ }
+
+ init_completion(&event);
+ bio->bi_private = &event;
+ bio->bi_end_io = bi_complete;
+ submit_bio(WRITE, bio);
+ wait_for_completion(&event);
+
+ if (test_bit(BIO_UPTODATE, &bio->bi_flags))
+ ret = 0;
+ else {
+ ret = -EIO;
+ break;
+ }
+ bio_put(bio);
+ ee_len -= done;
+ ee_pblock += done << (blkbits - 9);
+ }
+ return ret;
+}
+
/*
* This function is called by ext4_ext_get_blocks() if someone tries to write
* to an uninitialized extent. It may result in splitting the uninitialized
@@ -2202,14 +2276,19 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
ex3->ee_len = cpu_to_le16(allocated - max_blocks);
ext4_ext_mark_uninitialized(ex3);
err = ext4_ext_insert_extent(handle, inode, path, ex3);
- if (err) {
+ if (err == -ENOSPC) {
+ err = ext4_ext_zeroout(inode, &orig_ex);
+ if (err)
+ goto fix_extent_len;
+ /* update the extent length and mark as initialized */
ex->ee_block = orig_ex.ee_block;
ex->ee_len = orig_ex.ee_len;
ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
- ext4_ext_mark_uninitialized(ex);
ext4_ext_dirty(handle, inode, path + depth);
- goto out;
- }
+ return le16_to_cpu(ex->ee_len);
+
+ } else if (err)
+ goto fix_extent_len;
/*
* The depth, and hence eh & ex might change
* as part of the insert above.
@@ -2295,15 +2374,28 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
goto out;
insert:
err = ext4_ext_insert_extent(handle, inode, path, &newex);
- if (err) {
+ if (err == -ENOSPC) {
+ err = ext4_ext_zeroout(inode, &orig_ex);
+ if (err)
+ goto fix_extent_len;
+ /* update the extent length and mark as initialized */
ex->ee_block = orig_ex.ee_block;
ex->ee_len = orig_ex.ee_len;
ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
- ext4_ext_mark_uninitialized(ex);
ext4_ext_dirty(handle, inode, path + depth);
- }
+ return le16_to_cpu(ex->ee_len);
+ } else if (err)
+ goto fix_extent_len;
out:
return err ? err : allocated;
+
+fix_extent_len:
+ ex->ee_block = orig_ex.ee_block;
+ ex->ee_len = orig_ex.ee_len;
+ ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
+ ext4_ext_mark_uninitialized(ex);
+ ext4_ext_dirty(handle, inode, path + depth);
+ return err;
}

/*
--
1.5.4.3.422.g34cd6.dirty



2008-03-05 00:47:46

by Mingming Cao

[permalink] [raw]
Subject: Re: [PATCH] ext4: Convert uninitialized extent to initialized extent in case of file system full

On Tue, 2008-03-04 at 18:13 +0530, Aneesh Kumar K.V wrote:
> A write to prealloc area cause the split of unititalized extent
> into a initialized and uninitialized extent. If we don't have
> space to add new extent information instead of returning error
> convert the existing uninitialized extent to initialized one. We
> need to zero out the blocks corresponding to the extent to prevent
> wrong data reaching userspace.
>

Looks good!

Added to patch queue. I modified the summary slightly.

Mingming
> Signed-off-by: Aneesh Kumar K.V <[email protected]>
> ---
> fs/ext4/extents.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 99 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index d315cc1..839caf2 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2136,6 +2136,80 @@ void ext4_ext_release(struct super_block *sb)
> #endif
> }
>
> +static void bi_complete(struct bio *bio, int error)
> +{
> + complete((struct completion *)bio->bi_private);
> +}
> +
> +/* FIXME!! we need to try to merge to left or right after zerout */
> +static int ext4_ext_zeroout(struct inode *inode, struct ext4_extent *ex)
> +{
> + int ret = -EIO;
> + struct bio *bio;
> + int blkbits, blocksize;
> + sector_t ee_pblock;
> + struct completion event;
> + unsigned int ee_len, len, done, offset;
> +
> +
> + blkbits = inode->i_blkbits;
> + blocksize = inode->i_sb->s_blocksize;
> + ee_len = ext4_ext_get_actual_len(ex);
> + ee_pblock = ext_pblock(ex);
> +
> + /* convert ee_pblock in 512 byte sector */
> + ee_pblock = ee_pblock << (blkbits >> 9);
> +
> +
> + while (ee_len > 0) {
> +
> + if (ee_len > BIO_MAX_PAGES)
> + len = BIO_MAX_PAGES;
> + else
> + len = ee_len;
> +
> + bio = bio_alloc(GFP_NOIO, len);
> + if (!bio)
> + return -ENOMEM;
> + bio->bi_sector = ee_pblock;
> + bio->bi_bdev = inode->i_sb->s_bdev;
> +
> + done = 0;
> + offset = 0;
> + while (done < len) {
> + ret = bio_add_page(bio, ZERO_PAGE(0),
> + blocksize, offset);
> + if (ret != blocksize) {
> + /* We can't add any more page because of
> + * hardware limitation. Start a new bio
> + */
> + break;
> + }
> + done++;
> + offset += blocksize;
> + if (offset >= PAGE_CACHE_SIZE)
> + offset = 0;
> + }
> +
> + init_completion(&event);
> + bio->bi_private = &event;
> + bio->bi_end_io = bi_complete;
> + submit_bio(WRITE, bio);
> + wait_for_completion(&event);
> +
> + if (test_bit(BIO_UPTODATE, &bio->bi_flags))
> + ret = 0;
> + else {
> + ret = -EIO;
> + break;
> + }
> + bio_put(bio);
> + ee_len -= done;
> + ee_pblock += done << (blkbits - 9);
> + }
> + return ret;
> +}
> +
> /*
> * This function is called by ext4_ext_get_blocks() if someone tries to write
> * to an uninitialized extent. It may result in splitting the uninitialized
> @@ -2202,14 +2276,19 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> ex3->ee_len = cpu_to_le16(allocated - max_blocks);
> ext4_ext_mark_uninitialized(ex3);
> err = ext4_ext_insert_extent(handle, inode, path, ex3);
> - if (err) {
> + if (err == -ENOSPC) {
> + err = ext4_ext_zeroout(inode, &orig_ex);
> + if (err)
> + goto fix_extent_len;
> + /* update the extent length and mark as initialized */
> ex->ee_block = orig_ex.ee_block;
> ex->ee_len = orig_ex.ee_len;
> ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
> - ext4_ext_mark_uninitialized(ex);
> ext4_ext_dirty(handle, inode, path + depth);
> - goto out;
> - }
> + return le16_to_cpu(ex->ee_len);
> +
> + } else if (err)
> + goto fix_extent_len;
> /*
> * The depth, and hence eh & ex might change
> * as part of the insert above.
> @@ -2295,15 +2374,28 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
> goto out;
> insert:
> err = ext4_ext_insert_extent(handle, inode, path, &newex);
> - if (err) {
> + if (err == -ENOSPC) {
> + err = ext4_ext_zeroout(inode, &orig_ex);
> + if (err)
> + goto fix_extent_len;
> + /* update the extent length and mark as initialized */
> ex->ee_block = orig_ex.ee_block;
> ex->ee_len = orig_ex.ee_len;
> ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
> - ext4_ext_mark_uninitialized(ex);
> ext4_ext_dirty(handle, inode, path + depth);
> - }
> + return le16_to_cpu(ex->ee_len);
> + } else if (err)
> + goto fix_extent_len;
> out:
> return err ? err : allocated;
> +
> +fix_extent_len:
> + ex->ee_block = orig_ex.ee_block;
> + ex->ee_len = orig_ex.ee_len;
> + ext4_ext_store_pblock(ex, ext_pblock(&orig_ex));
> + ext4_ext_mark_uninitialized(ex);
> + ext4_ext_dirty(handle, inode, path + depth);
> + return err;
> }
>
> /*