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 != depth)
+ {
Use
if (newdepth != depth) {
Ok.
+ depth=newdepth;
spaces
Ok.
+ path = ext4_ext_find_extent(inode, iblock, NULL);
+ if (IS_ERR(path)) {
+
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 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_uninitialized(ex3);
+ err = ext4_ext_insert_extent(handle, inode, path, ex3);
+ if (err)
+ goto
