Re: [PATCH 5/5] ext4: write support for preallocated blocks/extents
On Mon, May 07, 2007 at 03:40:26PM +0300, Pekka Enberg wrote:
> On 4/26/07, Amit K. Arora <[EMAIL PROTECTED]> wrote:
> > /*
> >+ * ext4_ext_try_to_merge:
> >+ * tries to merge the "ex" extent to the next extent in the tree.
> >+ * It always tries to merge towards right. If you want to merge towards
> >+ * left, pass "ex - 1" as argument instead of "ex".
> >+ * Returns 0 if the extents (ex and ex+1) were _not_ merged and returns
> >+ * 1 if they got merged.
> >+ */
> >+int ext4_ext_try_to_merge(struct inode *inode,
> >+ struct ext4_ext_path *path,
> >+ struct ext4_extent *ex)
> >+{
>
> Please either use proper kerneldoc format or drop
> "ext4_ext_try_to_merge" from the comment.
Ok, Thanks.
--
Regards,
Amit Arora
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] ext4: write support for preallocated blocks/extents
On 4/26/07, Amit K. Arora <[EMAIL PROTECTED]> wrote:
/*
+ * ext4_ext_try_to_merge:
+ * tries to merge the "ex" extent to the next extent in the tree.
+ * It always tries to merge towards right. If you want to merge towards
+ * left, pass "ex - 1" as argument instead of "ex".
+ * Returns 0 if the extents (ex and ex+1) were _not_ merged and returns
+ * 1 if they got merged.
+ */
+int ext4_ext_try_to_merge(struct inode *inode,
+ struct ext4_ext_path *path,
+ struct ext4_extent *ex)
+{
Please either use proper kerneldoc format or drop
"ext4_ext_try_to_merge" from the comment.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] ext4: write support for preallocated blocks/extents
On Thu, May 03, 2007 at 09:32:38PM -0700, Andrew Morton wrote:
> On Thu, 26 Apr 2007 23:46:23 +0530 "Amit K. Arora" <[EMAIL PROTECTED]> wrote:
> > + */
> > +int ext4_ext_try_to_merge(struct inode *inode,
> > + struct ext4_ext_path *path,
> > + struct ext4_extent *ex)
> > +{
> > + struct ext4_extent_header *eh;
> > + unsigned int depth, len;
> > + int merge_done=0, uninitialized = 0;
>
> space around "=", please.
>
> Many people prefer not to do the multiple-definitions-per-line, btw:
>
> int merge_done = 0;
> int uninitialized = 0;
Ok. Will make the change.
>
> reasons:
>
> - If gives you some space for a nice comment
>
> - It makes patches much more readable, and it makes rejects easier to fix
>
> - standardisation.
>
> > + depth = ext_depth(inode);
> > + BUG_ON(path[depth].p_hdr == NULL);
> > + eh = path[depth].p_hdr;
> > +
> > + while (ex < EXT_LAST_EXTENT(eh)) {
> > + if (!ext4_can_extents_be_merged(inode, ex, ex + 1))
> > + break;
> > + /* merge with next extent! */
> > + if (ext4_ext_is_uninitialized(ex))
> > + uninitialized = 1;
> > + ex->ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex)
> > + + ext4_ext_get_actual_len(ex + 1));
> > + if (uninitialized)
> > + ext4_ext_mark_uninitialized(ex);
> > +
> > + if (ex + 1 < EXT_LAST_EXTENT(eh)) {
> > + len = (EXT_LAST_EXTENT(eh) - ex - 1)
> > + * sizeof(struct ext4_extent);
> > + memmove(ex + 1, ex + 2, len);
> > + }
> > + eh->eh_entries = cpu_to_le16(le16_to_cpu(eh->eh_entries)-1);
>
> Kenrel convention is to put spaces around "-"
Will fix this.
>
> > + merge_done = 1;
> > + BUG_ON(eh->eh_entries == 0);
>
> eek, scary BUG_ON. Do we really need to be that severe? Would it be
> better to warn and run ext4_error() here?
Ok.
>
> > + }
> > +
> > + return merge_done;
> > +}
> > +
> > +
> >
> > ...
> >
> > +/*
> > + * ext4_ext_convert_to_initialized:
> > + * 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
> > + * extent into multiple extents (upto three). Atleast one initialized
> > extent
> > + * and atmost two uninitialized extents can result.
>
> There are some typos here
>
> > + * There are three possibilities:
> > + * a> No split required: Entire extent should be initialized.
> > + * b> Split into two extents: Only one end of the extent is being
> > written to.
> > + * c> Split into three extents: Somone is writing in middle of the
> > extent.
>
> and here
>
Ok. Will fix them.
> > + */
> > +int ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
> > + struct ext4_ext_path *path,
> > + ext4_fsblk_t iblock,
> > + unsigned long max_blocks)
> > +{
> > + struct ext4_extent *ex, *ex1 = NULL, *ex2 = NULL, *ex3 = NULL, newex;
> > + struct ext4_extent_header *eh;
> > + unsigned int allocated, ee_block, ee_len, depth;
> > + ext4_fsblk_t newblock;
> > + int err = 0, ret = 0;
> > +
> > + depth = ext_depth(inode);
> > + eh = path[depth].p_hdr;
> > + ex = path[depth].p_ext;
> > + ee_block = le32_to_cpu(ex->ee_block);
> > + ee_len = ext4_ext_get_actual_len(ex);
> > + allocated = ee_len - (iblock - ee_block);
> > + newblock = iblock - ee_block + ext_pblock(ex);
> > + ex2 = ex;
> > +
> > + /* ex1: ee_block to iblock - 1 : uninitialized */
> > + if (iblock > ee_block) {
> > + ex1 = ex;
> > + ex1->ee_len = cpu_to_le16(iblock - ee_block);
> > + ext4_ext_mark_uninitialized(ex1);
> > + ex2 = &newex;
> > + }
> > + /* for sanity, update the length of the ex2 extent before
> > +* we insert ex3, if ex1 is NULL. This is to avoid temporary
> > +* overlap of blocks.
> > +*/
> > + if (!ex1 && allocated > max_blocks)
> > + ex2->ee_len = cpu_to_le16(max_blocks);
> > + /* ex3: to ee_block + ee_len : uninitialised */
> > + if (allocated > max_blocks) {
> > + unsigned int newdepth;
> > + ex3 = &newex;
> > + ex3->ee_block = cpu_to_le32(iblock + max_blocks);
> > + ext4_ext_store_pblock(ex3, newblock + max_blocks);
> > + 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)
> > + goto out;
> > + /* The depth, and hence eh & ex might change
> > +* as part of the insert above.
> > +*/
> > + newdepth = ext_depth(inode);
> > + if (newdepth != dep
Re: [PATCH 5/5] ext4: write support for preallocated blocks/extents
On Thu, 26 Apr 2007 23:46:23 +0530 "Amit K. Arora" <[EMAIL PROTECTED]> wrote:
> This patch adds write support for preallocated (using fallocate system
> call) blocks/extents. The preallocated extents in ext4 are marked
> "uninitialized", hence they need special handling especially while
> writing to them. This patch takes care of that.
>
> ...
>
> /*
> + * ext4_ext_try_to_merge:
> + * tries to merge the "ex" extent to the next extent in the tree.
> + * It always tries to merge towards right. If you want to merge towards
> + * left, pass "ex - 1" as argument instead of "ex".
> + * Returns 0 if the extents (ex and ex+1) were _not_ merged and returns
> + * 1 if they got merged.
OK.
> + */
> +int ext4_ext_try_to_merge(struct inode *inode,
> + struct ext4_ext_path *path,
> + struct ext4_extent *ex)
> +{
> + struct ext4_extent_header *eh;
> + unsigned int depth, len;
> + int merge_done=0, uninitialized = 0;
space around "=", please.
Many people prefer not to do the multiple-definitions-per-line, btw:
int merge_done = 0;
int uninitialized = 0;
reasons:
- If gives you some space for a nice comment
- It makes patches much more readable, and it makes rejects easier to fix
- standardisation.
> + depth = ext_depth(inode);
> + BUG_ON(path[depth].p_hdr == NULL);
> + eh = path[depth].p_hdr;
> +
> + while (ex < EXT_LAST_EXTENT(eh)) {
> + if (!ext4_can_extents_be_merged(inode, ex, ex + 1))
> + break;
> + /* merge with next extent! */
> + if (ext4_ext_is_uninitialized(ex))
> + uninitialized = 1;
> + ex->ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex)
> + + ext4_ext_get_actual_len(ex + 1));
> + if (uninitialized)
> + ext4_ext_mark_uninitialized(ex);
> +
> + if (ex + 1 < EXT_LAST_EXTENT(eh)) {
> + len = (EXT_LAST_EXTENT(eh) - ex - 1)
> + * sizeof(struct ext4_extent);
> + memmove(ex + 1, ex + 2, len);
> + }
> + eh->eh_entries = cpu_to_le16(le16_to_cpu(eh->eh_entries)-1);
Kenrel convention is to put spaces around "-"
> + merge_done = 1;
> + BUG_ON(eh->eh_entries == 0);
eek, scary BUG_ON. Do we really need to be that severe? Would it be
better to warn and run ext4_error() here?
> + }
> +
> + return merge_done;
> +}
> +
> +
>
> ...
>
> +/*
> + * ext4_ext_convert_to_initialized:
> + * 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
> + * extent into multiple extents (upto three). Atleast one initialized extent
> + * and atmost two uninitialized extents can result.
There are some typos here
> + * There are three possibilities:
> + * a> No split required: Entire extent should be initialized.
> + * b> Split into two extents: Only one end of the extent is being written
> to.
> + * c> Split into three extents: Somone is writing in middle of the extent.
and here
> + */
> +int ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
> + struct ext4_ext_path *path,
> + ext4_fsblk_t iblock,
> + unsigned long max_blocks)
> +{
> + struct ext4_extent *ex, *ex1 = NULL, *ex2 = NULL, *ex3 = NULL, newex;
> + struct ext4_extent_header *eh;
> + unsigned int allocated, ee_block, ee_len, depth;
> + ext4_fsblk_t newblock;
> + int err = 0, ret = 0;
> +
> + depth = ext_depth(inode);
> + eh = path[depth].p_hdr;
> + ex = path[depth].p_ext;
> + ee_block = le32_to_cpu(ex->ee_block);
> + ee_len = ext4_ext_get_actual_len(ex);
> + allocated = ee_len - (iblock - ee_block);
> + newblock = iblock - ee_block + ext_pblock(ex);
> + ex2 = ex;
> +
> + /* ex1: ee_block to iblock - 1 : uninitialized */
> + if (iblock > ee_block) {
> + ex1 = ex;
> + ex1->ee_len = cpu_to_le16(iblock - ee_block);
> + ext4_ext_mark_uninitialized(ex1);
> + ex2 = &newex;
> + }
> + /* for sanity, update the length of the ex2 extent before
> + * we insert ex3, if ex1 is NULL. This is to avoid temporary
> + * overlap of blocks.
> + */
> + if (!ex1 && allocated > max_blocks)
> + ex2->ee_len = cpu_to_le16(max_blocks);
> + /* ex3: to ee_block + ee_len : uninitialised */
> + if (allocated > max_blocks) {
> + unsigned int newdepth;
> + ex3 = &newex;
> + ex3->ee_block = cpu_to_le32(iblock + max_blocks);
> + ext4_ext_store_pblock(ex3, newblock + max_blocks);
> + ex3->ee_len = cpu_to_le16(allocated - max_blocks);
> + ext4_ext_mark_uni
[PATCH 5/5] ext4: write support for preallocated blocks/extents
This patch adds write support for preallocated (using fallocate system
call) blocks/extents. The preallocated extents in ext4 are marked
"uninitialized", hence they need special handling especially while
writing to them. This patch takes care of that.
Signed-off-by: Amit Arora <[EMAIL PROTECTED]>
---
fs/ext4/extents.c | 228 +++-
include/linux/ext4_fs_extents.h |1
2 files changed, 202 insertions(+), 27 deletions(-)
Index: linux-2.6.21/fs/ext4/extents.c
===
--- linux-2.6.21.orig/fs/ext4/extents.c
+++ linux-2.6.21/fs/ext4/extents.c
@@ -1141,6 +1141,51 @@ ext4_can_extents_be_merged(struct inode
}
/*
+ * ext4_ext_try_to_merge:
+ * tries to merge the "ex" extent to the next extent in the tree.
+ * It always tries to merge towards right. If you want to merge towards
+ * left, pass "ex - 1" as argument instead of "ex".
+ * Returns 0 if the extents (ex and ex+1) were _not_ merged and returns
+ * 1 if they got merged.
+ */
+int ext4_ext_try_to_merge(struct inode *inode,
+ struct ext4_ext_path *path,
+ struct ext4_extent *ex)
+{
+ struct ext4_extent_header *eh;
+ unsigned int depth, len;
+ int merge_done=0, uninitialized = 0;
+
+ depth = ext_depth(inode);
+ BUG_ON(path[depth].p_hdr == NULL);
+ eh = path[depth].p_hdr;
+
+ while (ex < EXT_LAST_EXTENT(eh)) {
+ if (!ext4_can_extents_be_merged(inode, ex, ex + 1))
+ break;
+ /* merge with next extent! */
+ if (ext4_ext_is_uninitialized(ex))
+ uninitialized = 1;
+ ex->ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex)
+ + ext4_ext_get_actual_len(ex + 1));
+ if (uninitialized)
+ ext4_ext_mark_uninitialized(ex);
+
+ if (ex + 1 < EXT_LAST_EXTENT(eh)) {
+ len = (EXT_LAST_EXTENT(eh) - ex - 1)
+ * sizeof(struct ext4_extent);
+ memmove(ex + 1, ex + 2, len);
+ }
+ eh->eh_entries = cpu_to_le16(le16_to_cpu(eh->eh_entries)-1);
+ merge_done = 1;
+ BUG_ON(eh->eh_entries == 0);
+ }
+
+ return merge_done;
+}
+
+
+/*
* ext4_ext_check_overlap:
* check if a portion of the "newext" extent overlaps with an
* existing extent.
@@ -1316,25 +1361,7 @@ has_space:
merge:
/* try to merge extents to the right */
- while (nearex < EXT_LAST_EXTENT(eh)) {
- if (!ext4_can_extents_be_merged(inode, nearex, nearex + 1))
- break;
- /* merge with next extent! */
- if (ext4_ext_is_uninitialized(nearex))
- uninitialized = 1;
- nearex->ee_len = cpu_to_le16(ext4_ext_get_actual_len(nearex)
- + ext4_ext_get_actual_len(nearex + 1));
- if (uninitialized)
- ext4_ext_mark_uninitialized(nearex);
-
- if (nearex + 1 < EXT_LAST_EXTENT(eh)) {
- len = (EXT_LAST_EXTENT(eh) - nearex - 1)
- * sizeof(struct ext4_extent);
- memmove(nearex + 1, nearex + 2, len);
- }
- eh->eh_entries = cpu_to_le16(le16_to_cpu(eh->eh_entries)-1);
- BUG_ON(eh->eh_entries == 0);
- }
+ ext4_ext_try_to_merge(inode, path, nearex);
/* try to merge extents to the left */
@@ -1999,15 +2026,149 @@ void ext4_ext_release(struct super_block
#endif
}
+/*
+ * ext4_ext_convert_to_initialized:
+ * 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
+ * extent into multiple extents (upto three). Atleast one initialized extent
+ * and atmost two uninitialized extents can result.
+ * There are three possibilities:
+ * a> No split required: Entire extent should be initialized.
+ * b> Split into two extents: Only one end of the extent is being written to.
+ * c> Split into three extents: Somone is writing in middle of the extent.
+ */
+int ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
+ struct ext4_ext_path *path,
+ ext4_fsblk_t iblock,
+ unsigned long max_blocks)
+{
+ struct ext4_extent *ex, *ex1 = NULL, *ex2 = NULL, *ex3 = NULL, newex;
+ struct ext4_extent_header *eh;
+ unsigned int allocated, ee_block, ee_len, depth;
+ ext4_fsblk_t newblock;
+ int err = 0, ret = 0;
+
+ depth = ext_depth(inode);
+ eh = path[depth].p_hdr;
+ ex = path[depth].p_ext;
+ ee_block = le3
