linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] ext4: clean up ext4_ext_find_extent()
@ 2014-09-01  4:06 Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 01/10] ext4: teach ext4_ext_find_extent() to free path on error Theodore Ts'o
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Theodore Ts'o @ 2014-09-01  4:06 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

This patch set fixes a potential memory leak in the error path handling
for ext4_fill_fiemap_extents().  More importantly, it makes the code
clearer so that easier to understand and audit.  The resulting code is
also tighter, both in lines of codes and bytes of compiled object code.
It also avoids some unnecessarily memory allocation and free operations.

Theodore Ts'o (10):
  ext4: teach ext4_ext_find_extent() to free path on error
  ext4: collapse ext4_convert_initialized_extents()
  ext4: drop EXT4_EX_NOFREE_ON_ERR in convert_initialized_extent()
  ext4: drop EXT4_EX_NOFREE_ON_ERR from rest of extents handling code
  ext4: call ext4_ext_drop_refs() from ext4_ext_find_extent()
  ext4: allow a NULL argument to ext4_ext_drop_refs()
  ext4: teach ext4_ext_find_extent() to realloc path if necessary
  ext4: reuse path object in ext4_ext_shift_extents()
  ext4: reuse path object in ext4_move_extents()
  ext4: rename ext4_ext_find_extent() to ext4_find_extent()

 fs/ext4/ext4.h           |  10 +-
 fs/ext4/ext4_extents.h   |   1 +
 fs/ext4/extents.c        | 360 +++++++++++++++++++++--------------------------
 fs/ext4/extents_status.c |   8 +-
 fs/ext4/migrate.c        |  11 +-
 fs/ext4/move_extent.c    |  43 +++---
 6 files changed, 194 insertions(+), 239 deletions(-)

-- 
2.1.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 01/10] ext4: teach ext4_ext_find_extent() to free path on error
  2014-09-01  4:06 [PATCH 00/10] ext4: clean up ext4_ext_find_extent() Theodore Ts'o
@ 2014-09-01  4:06 ` Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 02/10] ext4: collapse ext4_convert_initialized_extents() Theodore Ts'o
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Theodore Ts'o @ 2014-09-01  4:06 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Right now, there are a places where it is all to easy to leak memory
on an error path, via a usage like this:

	struct ext4_ext_path *path = NULL

	while (...) {
		...
		path = ext4_ext_find_extent(inode, block, path, 0);
		if (IS_ERR(path)) {
			/* oops, if path was non-NULL before the call to
			   ext4_ext_find_extent, we've leaked it!  :-(  */
			...
			return PTR_ERR(path);
		}
		...
	}

Unfortunately, there some code paths where we are doing the following
instead:

	path = ext4_ext_find_extent(inode, block, orig_path, 0);

and where it's important that we _not_ free orig_path in the case
where ext4_ext_find_extent() returns an error.

So change the function signature of ext4_ext_find_extent() so that it
takes a struct ext4_ext_path ** for its third argument, and by
default, on an error, it will free the struct ext4_ext_path, and then
zero out the struct ext4_ext_path * pointer.  In order to avoid
causing problems, we add a flag EXT4_EX_NOFREE_ON_ERR which causes
ext4_ext_find_extent() to use the original behavior of forcing the
caller to deal with freeing the original path pointer on the error
case.

The goal is to get rid of EXT4_EX_NOFREE_ON_ERR entirely, but this
allows for a gentle transition and makes the patches easier to verify.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/ext4.h        |  3 ++-
 fs/ext4/extents.c     | 28 ++++++++++++++++++----------
 fs/ext4/move_extent.c |  2 +-
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 550b4f9..696e51a 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -582,6 +582,7 @@ enum {
  */
 #define EXT4_EX_NOCACHE				0x0800
 #define EXT4_EX_FORCE_CACHE			0x1000
+#define EXT4_EX_NOFREE_ON_ERR			0x2000
 
 /*
  * Flags used by ext4_free_blocks
@@ -2733,7 +2734,7 @@ extern int ext4_ext_insert_extent(handle_t *, struct inode *,
 				  struct ext4_ext_path *,
 				  struct ext4_extent *, int);
 extern struct ext4_ext_path *ext4_ext_find_extent(struct inode *, ext4_lblk_t,
-						  struct ext4_ext_path *,
+						  struct ext4_ext_path **,
 						  int flags);
 extern void ext4_ext_drop_refs(struct ext4_ext_path *);
 extern int ext4_ext_check_inode(struct inode *inode);
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index bc3b49f..fc76be8 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -848,11 +848,13 @@ int ext4_ext_tree_init(handle_t *handle, struct inode *inode)
 
 struct ext4_ext_path *
 ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block,
-		     struct ext4_ext_path *path, int flags)
+		     struct ext4_ext_path **orig_path, int flags)
 {
 	struct ext4_extent_header *eh;
 	struct buffer_head *bh;
-	short int depth, i, ppos = 0, alloc = 0;
+	struct ext4_ext_path *path = orig_path ? *orig_path : NULL;
+	short int depth, i, ppos = 0;
+	short free_on_err = (flags & EXT4_EX_NOFREE_ON_ERR) == 0;
 	int ret;
 
 	eh = ext_inode_hdr(inode);
@@ -864,7 +866,7 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block,
 				GFP_NOFS);
 		if (unlikely(!path))
 			return ERR_PTR(-ENOMEM);
-		alloc = 1;
+		free_on_err = 1;
 	}
 	path[0].p_hdr = eh;
 	path[0].p_bh = NULL;
@@ -916,8 +918,11 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block,
 
 err:
 	ext4_ext_drop_refs(path);
-	if (alloc)
+	if (free_on_err) {
 		kfree(path);
+		if (orig_path)
+			*orig_path = NULL;
+	}
 	return ERR_PTR(ret);
 }
 
@@ -1349,7 +1354,7 @@ repeat:
 		ext4_ext_drop_refs(path);
 		path = ext4_ext_find_extent(inode,
 				    (ext4_lblk_t)le32_to_cpu(newext->ee_block),
-				    path, gb_flags);
+				    &path, gb_flags | EXT4_EX_NOFREE_ON_ERR);
 		if (IS_ERR(path))
 			err = PTR_ERR(path);
 	} else {
@@ -1362,7 +1367,7 @@ repeat:
 		ext4_ext_drop_refs(path);
 		path = ext4_ext_find_extent(inode,
 				   (ext4_lblk_t)le32_to_cpu(newext->ee_block),
-				    path, gb_flags);
+				    &path, gb_flags | EXT4_EX_NOFREE_ON_ERR);
 		if (IS_ERR(path)) {
 			err = PTR_ERR(path);
 			goto out;
@@ -2145,7 +2150,7 @@ static int ext4_fill_fiemap_extents(struct inode *inode,
 			path = NULL;
 		}
 
-		path = ext4_ext_find_extent(inode, block, path, 0);
+		path = ext4_ext_find_extent(inode, block, &path, 0);
 		if (IS_ERR(path)) {
 			up_read(&EXT4_I(inode)->i_data_sem);
 			err = PTR_ERR(path);
@@ -3319,7 +3324,8 @@ static int ext4_split_extent(handle_t *handle,
 	 * result in split of original leaf or extent zeroout.
 	 */
 	ext4_ext_drop_refs(path);
-	path = ext4_ext_find_extent(inode, map->m_lblk, path, 0);
+	path = ext4_ext_find_extent(inode, map->m_lblk, &path,
+				    EXT4_EX_NOFREE_ON_ERR);
 	if (IS_ERR(path))
 		return PTR_ERR(path);
 	depth = ext_depth(inode);
@@ -3703,7 +3709,8 @@ static int ext4_convert_initialized_extents(handle_t *handle,
 		if (err < 0)
 			goto out;
 		ext4_ext_drop_refs(path);
-		path = ext4_ext_find_extent(inode, map->m_lblk, path, 0);
+		path = ext4_ext_find_extent(inode, map->m_lblk, &path,
+					    EXT4_EX_NOFREE_ON_ERR);
 		if (IS_ERR(path)) {
 			err = PTR_ERR(path);
 			goto out;
@@ -3775,7 +3782,8 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle,
 		if (err < 0)
 			goto out;
 		ext4_ext_drop_refs(path);
-		path = ext4_ext_find_extent(inode, map->m_lblk, path, 0);
+		path = ext4_ext_find_extent(inode, map->m_lblk, &path,
+					    EXT4_EX_NOFREE_ON_ERR);
 		if (IS_ERR(path)) {
 			err = PTR_ERR(path);
 			goto out;
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index c8f895b..5e2465a 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -37,7 +37,7 @@ get_ext_path(struct inode *inode, ext4_lblk_t lblock,
 	int ret = 0;
 	struct ext4_ext_path *path;
 
-	path = ext4_ext_find_extent(inode, lblock, *orig_path, EXT4_EX_NOCACHE);
+	path = ext4_ext_find_extent(inode, lblock, orig_path, EXT4_EX_NOCACHE);
 	if (IS_ERR(path))
 		ret = PTR_ERR(path);
 	else if (path[ext_depth(inode)].p_ext == NULL)
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 02/10] ext4: collapse ext4_convert_initialized_extents()
  2014-09-01  4:06 [PATCH 00/10] ext4: clean up ext4_ext_find_extent() Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 01/10] ext4: teach ext4_ext_find_extent() to free path on error Theodore Ts'o
@ 2014-09-01  4:06 ` Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 03/10] ext4: drop EXT4_EX_NOFREE_ON_ERR in convert_initialized_extent() Theodore Ts'o
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Theodore Ts'o @ 2014-09-01  4:06 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

The function ext4_convert_initialized_extents() is only called by a
single function --- ext4_ext_convert_initalized_extents().  Inline the
code and get rid of the unnecessary bits in order to simplify the code.

Rename ext4_ext_convert_initalized_extents() to
convert_initalized_extents() since it's a static function that is
actually only used in a single caller, ext4_ext_map_blocks().

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/extents.c | 136 +++++++++++++++++++++++-------------------------------
 1 file changed, 59 insertions(+), 77 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index fc76be8..985848d 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3683,67 +3683,6 @@ static int ext4_split_convert_extents(handle_t *handle,
 	return ext4_split_extent(handle, inode, path, map, split_flag, flags);
 }
 
-static int ext4_convert_initialized_extents(handle_t *handle,
-					    struct inode *inode,
-					    struct ext4_map_blocks *map,
-					    struct ext4_ext_path *path)
-{
-	struct ext4_extent *ex;
-	ext4_lblk_t ee_block;
-	unsigned int ee_len;
-	int depth;
-	int err = 0;
-
-	depth = ext_depth(inode);
-	ex = path[depth].p_ext;
-	ee_block = le32_to_cpu(ex->ee_block);
-	ee_len = ext4_ext_get_actual_len(ex);
-
-	ext_debug("%s: inode %lu, logical"
-		"block %llu, max_blocks %u\n", __func__, inode->i_ino,
-		  (unsigned long long)ee_block, ee_len);
-
-	if (ee_block != map->m_lblk || ee_len > map->m_len) {
-		err = ext4_split_convert_extents(handle, inode, map, path,
-				EXT4_GET_BLOCKS_CONVERT_UNWRITTEN);
-		if (err < 0)
-			goto out;
-		ext4_ext_drop_refs(path);
-		path = ext4_ext_find_extent(inode, map->m_lblk, &path,
-					    EXT4_EX_NOFREE_ON_ERR);
-		if (IS_ERR(path)) {
-			err = PTR_ERR(path);
-			goto out;
-		}
-		depth = ext_depth(inode);
-		ex = path[depth].p_ext;
-		if (!ex) {
-			EXT4_ERROR_INODE(inode, "unexpected hole at %lu",
-					 (unsigned long) map->m_lblk);
-			err = -EIO;
-			goto out;
-		}
-	}
-
-	err = ext4_ext_get_access(handle, inode, path + depth);
-	if (err)
-		goto out;
-	/* first mark the extent as unwritten */
-	ext4_ext_mark_unwritten(ex);
-
-	/* note: ext4_ext_correct_indexes() isn't needed here because
-	 * borders are not changed
-	 */
-	ext4_ext_try_to_merge(handle, inode, path, ex);
-
-	/* Mark modified extent as dirty */
-	err = ext4_ext_dirty(handle, inode, path + path->p_depth);
-out:
-	ext4_ext_show_leaf(inode, path);
-	return err;
-}
-
-
 static int ext4_convert_unwritten_extents_endio(handle_t *handle,
 						struct inode *inode,
 						struct ext4_map_blocks *map,
@@ -3980,12 +3919,15 @@ get_reserved_cluster_alloc(struct inode *inode, ext4_lblk_t lblk_start,
 }
 
 static int
-ext4_ext_convert_initialized_extent(handle_t *handle, struct inode *inode,
-			struct ext4_map_blocks *map,
-			struct ext4_ext_path *path, int flags,
-			unsigned int allocated, ext4_fsblk_t newblock)
+convert_initialized_extent(handle_t *handle, struct inode *inode,
+			   struct ext4_map_blocks *map,
+			   struct ext4_ext_path *path, int flags,
+			   unsigned int allocated, ext4_fsblk_t newblock)
 {
-	int ret = 0;
+	struct ext4_extent *ex;
+	ext4_lblk_t ee_block;
+	unsigned int ee_len;
+	int depth;
 	int err = 0;
 
 	/*
@@ -3995,20 +3937,60 @@ ext4_ext_convert_initialized_extent(handle_t *handle, struct inode *inode,
 	if (map->m_len > EXT_UNWRITTEN_MAX_LEN)
 		map->m_len = EXT_UNWRITTEN_MAX_LEN / 2;
 
-	ret = ext4_convert_initialized_extents(handle, inode, map,
-						path);
-	if (ret >= 0) {
-		ext4_update_inode_fsync_trans(handle, inode, 1);
-		err = check_eofblocks_fl(handle, inode, map->m_lblk,
-					 path, map->m_len);
-	} else
-		err = ret;
+	depth = ext_depth(inode);
+	ex = path[depth].p_ext;
+	ee_block = le32_to_cpu(ex->ee_block);
+	ee_len = ext4_ext_get_actual_len(ex);
+
+	ext_debug("%s: inode %lu, logical"
+		"block %llu, max_blocks %u\n", __func__, inode->i_ino,
+		  (unsigned long long)ee_block, ee_len);
+
+	if (ee_block != map->m_lblk || ee_len > map->m_len) {
+		err = ext4_split_convert_extents(handle, inode, map, path,
+				EXT4_GET_BLOCKS_CONVERT_UNWRITTEN);
+		if (err < 0)
+			return err;
+		ext4_ext_drop_refs(path);
+		path = ext4_ext_find_extent(inode, map->m_lblk, &path,
+					    EXT4_EX_NOFREE_ON_ERR);
+		if (IS_ERR(path))
+			return PTR_ERR(path);
+		depth = ext_depth(inode);
+		ex = path[depth].p_ext;
+		if (!ex) {
+			EXT4_ERROR_INODE(inode, "unexpected hole at %lu",
+					 (unsigned long) map->m_lblk);
+			return -EIO;
+		}
+	}
+
+	err = ext4_ext_get_access(handle, inode, path + depth);
+	if (err)
+		return err;
+	/* first mark the extent as unwritten */
+	ext4_ext_mark_unwritten(ex);
+
+	/* note: ext4_ext_correct_indexes() isn't needed here because
+	 * borders are not changed
+	 */
+	ext4_ext_try_to_merge(handle, inode, path, ex);
+
+	/* Mark modified extent as dirty */
+	err = ext4_ext_dirty(handle, inode, path + path->p_depth);
+	if (err)
+		return err;
+	ext4_ext_show_leaf(inode, path);
+
+	ext4_update_inode_fsync_trans(handle, inode, 1);
+	err = check_eofblocks_fl(handle, inode, map->m_lblk, path, map->m_len);
+	if (err)
+		return err;
 	map->m_flags |= EXT4_MAP_UNWRITTEN;
 	if (allocated > map->m_len)
 		allocated = map->m_len;
 	map->m_len = allocated;

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 03/10] ext4: drop EXT4_EX_NOFREE_ON_ERR in convert_initialized_extent()
  2014-09-01  4:06 [PATCH 00/10] ext4: clean up ext4_ext_find_extent() Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 01/10] ext4: teach ext4_ext_find_extent() to free path on error Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 02/10] ext4: collapse ext4_convert_initialized_extents() Theodore Ts'o
@ 2014-09-01  4:06 ` Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 04/10] ext4: drop EXT4_EX_NOFREE_ON_ERR from rest of extents handling code Theodore Ts'o
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Theodore Ts'o @ 2014-09-01  4:06 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Transfer responsibility of freeing struct ext4_ext_path on error to
ext4_ext_find_extent().

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/extents.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 985848d..ceab095 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3921,9 +3921,10 @@ get_reserved_cluster_alloc(struct inode *inode, ext4_lblk_t lblk_start,
 static int
 convert_initialized_extent(handle_t *handle, struct inode *inode,
 			   struct ext4_map_blocks *map,
-			   struct ext4_ext_path *path, int flags,
+			   struct ext4_ext_path **ppath, int flags,
 			   unsigned int allocated, ext4_fsblk_t newblock)
 {
+	struct ext4_ext_path *path = *ppath;
 	struct ext4_extent *ex;
 	ext4_lblk_t ee_block;
 	unsigned int ee_len;
@@ -3952,8 +3953,7 @@ convert_initialized_extent(handle_t *handle, struct inode *inode,
 		if (err < 0)
 			return err;
 		ext4_ext_drop_refs(path);
-		path = ext4_ext_find_extent(inode, map->m_lblk, &path,
-					    EXT4_EX_NOFREE_ON_ERR);
+		path = ext4_ext_find_extent(inode, map->m_lblk, ppath, 0);
 		if (IS_ERR(path))
 			return PTR_ERR(path);
 		depth = ext_depth(inode);
@@ -4331,8 +4331,8 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 			if ((!ext4_ext_is_unwritten(ex)) &&
 			    (flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN)) {
 				allocated = convert_initialized_extent(
-						handle, inode, map, path, flags,
-						allocated, newblock);
+						handle, inode, map, &path,
+						flags, allocated, newblock);
 				goto out2;
 			} else if (!ext4_ext_is_unwritten(ex))
 				goto out;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 04/10] ext4: drop EXT4_EX_NOFREE_ON_ERR from rest of extents handling code
  2014-09-01  4:06 [PATCH 00/10] ext4: clean up ext4_ext_find_extent() Theodore Ts'o
                   ` (2 preceding siblings ...)
  2014-09-01  4:06 ` [PATCH 03/10] ext4: drop EXT4_EX_NOFREE_ON_ERR in convert_initialized_extent() Theodore Ts'o
@ 2014-09-01  4:06 ` Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 05/10] ext4: call ext4_ext_drop_refs() from ext4_ext_find_extent() Theodore Ts'o
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Theodore Ts'o @ 2014-09-01  4:06 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Drop EXT4_EX_NOFREE_ON_ERR from ext4_ext_create_new_leaf(),
ext4_split_extent(), ext4_convert_unwritten_extents_endio().

This requires fixing all of their callers to potentially
ext4_ext_find_extent() to free the struct ext4_ext_path object in case
of an error, and there are interlocking dependencies all the way up to
ext4_ext_map_blocks(), ext4_swap_extents(), and
ext4_ext_remove_space().

Once this is done, we can drop the EXT4_EX_NOFREE_ON_ERR flag since it
is no longer necessary.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/ext4.h    |   3 +-
 fs/ext4/extents.c | 114 +++++++++++++++++++++++++++---------------------------
 fs/ext4/migrate.c |   2 +-
 3 files changed, 60 insertions(+), 59 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 696e51a..4a5a6b9 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -582,7 +582,6 @@ enum {
  */
 #define EXT4_EX_NOCACHE				0x0800
 #define EXT4_EX_FORCE_CACHE			0x1000
-#define EXT4_EX_NOFREE_ON_ERR			0x2000
 
 /*
  * Flags used by ext4_free_blocks
@@ -2731,7 +2730,7 @@ extern int ext4_can_extents_be_merged(struct inode *inode,
 				      struct ext4_extent *ex1,
 				      struct ext4_extent *ex2);
 extern int ext4_ext_insert_extent(handle_t *, struct inode *,
-				  struct ext4_ext_path *,
+				  struct ext4_ext_path **,
 				  struct ext4_extent *, int);
 extern struct ext4_ext_path *ext4_ext_find_extent(struct inode *, ext4_lblk_t,
 						  struct ext4_ext_path **,
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index ceab095..f6d8b00 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -98,21 +98,21 @@ static void ext4_extent_block_csum_set(struct inode *inode,
 
 static int ext4_split_extent(handle_t *handle,
 				struct inode *inode,
-				struct ext4_ext_path *path,
+				struct ext4_ext_path **path,
 				struct ext4_map_blocks *map,
 				int split_flag,
 				int flags);
 
 static int ext4_split_extent_at(handle_t *handle,
 			     struct inode *inode,
-			     struct ext4_ext_path *path,
+			     struct ext4_ext_path **ppath,
 			     ext4_lblk_t split,
 			     int split_flag,
 			     int flags);
 
 static int ext4_force_split_extent_at(handle_t *handle,
 				      struct inode *inode,
-				      struct ext4_ext_path *path,
+				      struct ext4_ext_path **ppath,
 				      ext4_lblk_t lblk,
 				      int nofail);
 
@@ -854,7 +854,6 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block,
 	struct buffer_head *bh;
 	struct ext4_ext_path *path = orig_path ? *orig_path : NULL;
 	short int depth, i, ppos = 0;
-	short free_on_err = (flags & EXT4_EX_NOFREE_ON_ERR) == 0;
 	int ret;
 
 	eh = ext_inode_hdr(inode);
@@ -866,7 +865,6 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block,
 				GFP_NOFS);
 		if (unlikely(!path))
 			return ERR_PTR(-ENOMEM);
-		free_on_err = 1;
 	}
 	path[0].p_hdr = eh;
 	path[0].p_bh = NULL;
@@ -918,11 +916,9 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block,
 
 err:
 	ext4_ext_drop_refs(path);
-	if (free_on_err) {
-		kfree(path);
-		if (orig_path)
-			*orig_path = NULL;
-	}
+	kfree(path);
+	if (orig_path)
+		*orig_path = NULL;
 	return ERR_PTR(ret);
 }
 
@@ -1325,9 +1321,10 @@ out:
 static int ext4_ext_create_new_leaf(handle_t *handle, struct inode *inode,
 				    unsigned int mb_flags,
 				    unsigned int gb_flags,
-				    struct ext4_ext_path *path,
+				    struct ext4_ext_path **ppath,
 				    struct ext4_extent *newext)
 {
+	struct ext4_ext_path *path = *ppath;
 	struct ext4_ext_path *curp;
 	int depth, i, err = 0;
 
@@ -1354,7 +1351,7 @@ repeat:
 		ext4_ext_drop_refs(path);
 		path = ext4_ext_find_extent(inode,
 				    (ext4_lblk_t)le32_to_cpu(newext->ee_block),
-				    &path, gb_flags | EXT4_EX_NOFREE_ON_ERR);
+				    ppath, gb_flags);
 		if (IS_ERR(path))
 			err = PTR_ERR(path);
 	} else {
@@ -1367,7 +1364,7 @@ repeat:
 		ext4_ext_drop_refs(path);
 		path = ext4_ext_find_extent(inode,
 				   (ext4_lblk_t)le32_to_cpu(newext->ee_block),
-				    &path, gb_flags | EXT4_EX_NOFREE_ON_ERR);
+				    ppath, gb_flags);
 		if (IS_ERR(path)) {
 			err = PTR_ERR(path);
 			goto out;
@@ -1907,9 +1904,10 @@ out:
  * creating new leaf in the no-space case.
  */
 int ext4_ext_insert_extent(handle_t *handle, struct inode *inode,
-				struct ext4_ext_path *path,
+				struct ext4_ext_path **ppath,
 				struct ext4_extent *newext, int gb_flags)
 {
+	struct ext4_ext_path *path = *ppath;
 	struct ext4_extent_header *eh;
 	struct ext4_extent *ex, *fex;
 	struct ext4_extent *nearex; /* nearest extent */
@@ -2041,7 +2039,7 @@ prepend:
 	if (gb_flags & EXT4_GET_BLOCKS_METADATA_NOFAIL)
 		mb_flags = EXT4_MB_USE_RESERVED;
 	err = ext4_ext_create_new_leaf(handle, inode, mb_flags, gb_flags,
-				       path, newext);
+				       ppath, newext);
 	if (err)
 		goto cleanup;
 	depth = ext_depth(inode);
@@ -2871,7 +2869,7 @@ again:
 			 * fail removing space due to ENOSPC so try to use
 			 * reserved block if that happens.
 			 */
-			err = ext4_force_split_extent_at(handle, inode, path,
+			err = ext4_force_split_extent_at(handle, inode, &path,
 							 end + 1, 1);
 			if (err < 0)
 				goto out;
@@ -3012,12 +3010,13 @@ again:
 		}
 	}
 out:
-	ext4_ext_drop_refs(path);
-	kfree(path);
-	if (err == -EAGAIN) {
+	if (path) {
+		ext4_ext_drop_refs(path);
+		kfree(path);
 		path = NULL;
-		goto again;
 	}
+	if (err == -EAGAIN)
+		goto again;
 	ext4_journal_stop(handle);
 
 	return err;
@@ -3131,11 +3130,12 @@ static int ext4_ext_zeroout(struct inode *inode, struct ext4_extent *ex)
  */
 static int ext4_split_extent_at(handle_t *handle,
 			     struct inode *inode,
-			     struct ext4_ext_path *path,
+			     struct ext4_ext_path **ppath,
 			     ext4_lblk_t split,
 			     int split_flag,
 			     int flags)
 {
+	struct ext4_ext_path *path = *ppath;
 	ext4_fsblk_t newblock;
 	ext4_lblk_t ee_block;
 	struct ext4_extent *ex, newex, orig_ex, zero_ex;
@@ -3206,7 +3206,7 @@ static int ext4_split_extent_at(handle_t *handle,
 	if (split_flag & EXT4_EXT_MARK_UNWRIT2)
 		ext4_ext_mark_unwritten(ex2);
 
-	err = ext4_ext_insert_extent(handle, inode, path, &newex, flags);
+	err = ext4_ext_insert_extent(handle, inode, ppath, &newex, flags);
 	if (err == -ENOSPC && (EXT4_EXT_MAY_ZEROOUT & split_flag)) {
 		if (split_flag & (EXT4_EXT_DATA_VALID1|EXT4_EXT_DATA_VALID2)) {
 			if (split_flag & EXT4_EXT_DATA_VALID1) {
@@ -3261,12 +3261,13 @@ fix_extent_len:
 
 static inline int
 ext4_force_split_extent_at(handle_t *handle, struct inode *inode,
-			   struct ext4_ext_path *path, ext4_lblk_t lblk,
+			   struct ext4_ext_path **ppath, ext4_lblk_t lblk,
 			   int nofail)
 {
+	struct ext4_ext_path *path = *ppath;
 	int unwritten = ext4_ext_is_unwritten(path[path->p_depth].p_ext);
 
-	return ext4_split_extent_at(handle, inode, path, lblk, unwritten ?
+	return ext4_split_extent_at(handle, inode, ppath, lblk, unwritten ?
 			EXT4_EXT_MARK_UNWRIT1|EXT4_EXT_MARK_UNWRIT2 : 0,
 			EXT4_EX_NOCACHE | EXT4_GET_BLOCKS_PRE_IO |
 			(nofail ? EXT4_GET_BLOCKS_METADATA_NOFAIL:0));
@@ -3285,11 +3286,12 @@ ext4_force_split_extent_at(handle_t *handle, struct inode *inode,
  */
 static int ext4_split_extent(handle_t *handle,
 			      struct inode *inode,
-			      struct ext4_ext_path *path,
+			      struct ext4_ext_path **ppath,
 			      struct ext4_map_blocks *map,
 			      int split_flag,
 			      int flags)
 {
+	struct ext4_ext_path *path = *ppath;
 	ext4_lblk_t ee_block;
 	struct ext4_extent *ex;
 	unsigned int ee_len, depth;
@@ -3312,7 +3314,7 @@ static int ext4_split_extent(handle_t *handle,
 				       EXT4_EXT_MARK_UNWRIT2;
 		if (split_flag & EXT4_EXT_DATA_VALID2)
 			split_flag1 |= EXT4_EXT_DATA_VALID1;
-		err = ext4_split_extent_at(handle, inode, path,
+		err = ext4_split_extent_at(handle, inode, ppath,
 				map->m_lblk + map->m_len, split_flag1, flags1);
 		if (err)
 			goto out;
@@ -3324,8 +3326,7 @@ static int ext4_split_extent(handle_t *handle,
 	 * result in split of original leaf or extent zeroout.
 	 */
 	ext4_ext_drop_refs(path);
-	path = ext4_ext_find_extent(inode, map->m_lblk, &path,
-				    EXT4_EX_NOFREE_ON_ERR);
+	path = ext4_ext_find_extent(inode, map->m_lblk, ppath, 0);
 	if (IS_ERR(path))
 		return PTR_ERR(path);
 	depth = ext_depth(inode);
@@ -3345,7 +3346,7 @@ static int ext4_split_extent(handle_t *handle,
 			split_flag1 |= split_flag & (EXT4_EXT_MAY_ZEROOUT |
 						     EXT4_EXT_MARK_UNWRIT2);
 		}
-		err = ext4_split_extent_at(handle, inode, path,
+		err = ext4_split_extent_at(handle, inode, ppath,
 				map->m_lblk, split_flag1, flags);
 		if (err)
 			goto out;
@@ -3379,9 +3380,10 @@ out:
 static int ext4_ext_convert_to_initialized(handle_t *handle,
 					   struct inode *inode,
 					   struct ext4_map_blocks *map,
-					   struct ext4_ext_path *path,
+					   struct ext4_ext_path **ppath,
 					   int flags)
 {
+	struct ext4_ext_path *path = *ppath;
 	struct ext4_sb_info *sbi;
 	struct ext4_extent_header *eh;
 	struct ext4_map_blocks split_map;
@@ -3605,7 +3607,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 		}
 	}
 
-	allocated = ext4_split_extent(handle, inode, path,
+	allocated = ext4_split_extent(handle, inode, ppath,
 				      &split_map, split_flag, flags);
 	if (allocated < 0)
 		err = allocated;
@@ -3644,9 +3646,10 @@ out:
 static int ext4_split_convert_extents(handle_t *handle,
 					struct inode *inode,
 					struct ext4_map_blocks *map,
-					struct ext4_ext_path *path,
+					struct ext4_ext_path **ppath,
 					int flags)
 {
+	struct ext4_ext_path *path = *ppath;
 	ext4_lblk_t eof_block;
 	ext4_lblk_t ee_block;
 	struct ext4_extent *ex;
@@ -3680,14 +3683,15 @@ static int ext4_split_convert_extents(handle_t *handle,
 		split_flag |= (EXT4_EXT_MARK_UNWRIT2 | EXT4_EXT_DATA_VALID2);
 	}
 	flags |= EXT4_GET_BLOCKS_PRE_IO;
-	return ext4_split_extent(handle, inode, path, map, split_flag, flags);
+	return ext4_split_extent(handle, inode, ppath, map, split_flag, flags);
 }
 
 static int ext4_convert_unwritten_extents_endio(handle_t *handle,
 						struct inode *inode,
 						struct ext4_map_blocks *map,
-						struct ext4_ext_path *path)
+						struct ext4_ext_path **ppath)
 {
+	struct ext4_ext_path *path = *ppath;
 	struct ext4_extent *ex;
 	ext4_lblk_t ee_block;
 	unsigned int ee_len;
@@ -3716,17 +3720,14 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle,
 			     inode->i_ino, (unsigned long long)ee_block, ee_len,
 			     (unsigned long long)map->m_lblk, map->m_len);
 #endif
-		err = ext4_split_convert_extents(handle, inode, map, path,
+		err = ext4_split_convert_extents(handle, inode, map, ppath,
 						 EXT4_GET_BLOCKS_CONVERT);
 		if (err < 0)
-			goto out;
+			return err;
 		ext4_ext_drop_refs(path);
-		path = ext4_ext_find_extent(inode, map->m_lblk, &path,
-					    EXT4_EX_NOFREE_ON_ERR);
-		if (IS_ERR(path)) {
-			err = PTR_ERR(path);
-			goto out;
-		}
+		path = ext4_ext_find_extent(inode, map->m_lblk, ppath, 0);
+		if (IS_ERR(path))
+			return PTR_ERR(path);
 		depth = ext_depth(inode);
 		ex = path[depth].p_ext;
 	}
@@ -3948,7 +3949,7 @@ convert_initialized_extent(handle_t *handle, struct inode *inode,
 		  (unsigned long long)ee_block, ee_len);
 
 	if (ee_block != map->m_lblk || ee_len > map->m_len) {
-		err = ext4_split_convert_extents(handle, inode, map, path,
+		err = ext4_split_convert_extents(handle, inode, map, ppath,
 				EXT4_GET_BLOCKS_CONVERT_UNWRITTEN);
 		if (err < 0)
 			return err;
@@ -3996,9 +3997,10 @@ convert_initialized_extent(handle_t *handle, struct inode *inode,
 static int
 ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
 			struct ext4_map_blocks *map,
-			struct ext4_ext_path *path, int flags,
+			struct ext4_ext_path **ppath, int flags,
 			unsigned int allocated, ext4_fsblk_t newblock)
 {
+	struct ext4_ext_path *path = *ppath;
 	int ret = 0;
 	int err = 0;
 	ext4_io_end_t *io = ext4_inode_aio(inode);
@@ -4020,8 +4022,8 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
 
 	/* get_block() before submit the IO, split the extent */
 	if (flags & EXT4_GET_BLOCKS_PRE_IO) {
-		ret = ext4_split_convert_extents(handle, inode, map,
-					 path, flags | EXT4_GET_BLOCKS_CONVERT);
+		ret = ext4_split_convert_extents(handle, inode, map, ppath,
+					 flags | EXT4_GET_BLOCKS_CONVERT);
 		if (ret <= 0)
 			goto out;
 		/*
@@ -4039,7 +4041,7 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
 	/* IO end_io complete, convert the filled extent to written */
 	if (flags & EXT4_GET_BLOCKS_CONVERT) {
 		ret = ext4_convert_unwritten_extents_endio(handle, inode, map,
-							path);
+							   ppath);
 		if (ret >= 0) {
 			ext4_update_inode_fsync_trans(handle, inode, 1);
 			err = check_eofblocks_fl(handle, inode, map->m_lblk,
@@ -4077,7 +4079,7 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
 	}
 
 	/* buffered write, writepage time, convert*/
-	ret = ext4_ext_convert_to_initialized(handle, inode, map, path, flags);
+	ret = ext4_ext_convert_to_initialized(handle, inode, map, ppath, flags);
 	if (ret >= 0)
 		ext4_update_inode_fsync_trans(handle, inode, 1);
 out:
@@ -4338,7 +4340,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 				goto out;
 
 			ret = ext4_ext_handle_unwritten_extents(
-				handle, inode, map, path, flags,
+				handle, inode, map, &path, flags,
 				allocated, newblock);
 			if (ret < 0)
 				err = ret;
@@ -4485,7 +4487,7 @@ got_allocated_blocks:
 		err = check_eofblocks_fl(handle, inode, map->m_lblk,
 					 path, ar.len);
 	if (!err)
-		err = ext4_ext_insert_extent(handle, inode, path,
+		err = ext4_ext_insert_extent(handle, inode, &path,
 					     &newex, flags);
 
 	if (!err && set_unwritten) {
@@ -5610,18 +5612,18 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
 		if (e1_blk < lblk1) {
 			split = 1;
 			*erp = ext4_force_split_extent_at(handle, inode1,
-						path1, lblk1, 0);
+						&path1, lblk1, 0);
 			if (unlikely(*erp))
 				goto finish;
 		}
 		if (e2_blk < lblk2) {
 			split = 1;
 			*erp = ext4_force_split_extent_at(handle, inode2,
-						path2,  lblk2, 0);
+						&path2,  lblk2, 0);
 			if (unlikely(*erp))
 				goto finish;
 		}
-		/* ext4_split_extent_at() may retult in leaf extent split,
+		/* ext4_split_extent_at() may result in leaf extent split,
 		 * path must to be revalidated. */
 		if (split)
 			goto repeat;
@@ -5636,18 +5638,18 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
 		if (len != e1_len) {
 			split = 1;
 			*erp = ext4_force_split_extent_at(handle, inode1,
-						path1, lblk1 + len, 0);
+						&path1, lblk1 + len, 0);
 			if (unlikely(*erp))
 				goto finish;
 		}
 		if (len != e2_len) {
 			split = 1;
 			*erp = ext4_force_split_extent_at(handle, inode2,
-						path2, lblk2 + len, 0);
+						&path2, lblk2 + len, 0);
 			if (*erp)
 				goto finish;
 		}
-		/* ext4_split_extent_at() may retult in leaf extent split,
+		/* ext4_split_extent_at() may result in leaf extent split,
 		 * path must to be revalidated. */
 		if (split)
 			goto repeat;
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index d3567f2..aff7bdf 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -81,7 +81,7 @@ static int finish_range(handle_t *handle, struct inode *inode,
 				goto err_out;
 		}
 	}
-	retval = ext4_ext_insert_extent(handle, inode, path, &newext, 0);
+	retval = ext4_ext_insert_extent(handle, inode, &path, &newext, 0);
 err_out:
 	up_write((&EXT4_I(inode)->i_data_sem));
 	if (path) {
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 05/10] ext4: call ext4_ext_drop_refs() from ext4_ext_find_extent()
  2014-09-01  4:06 [PATCH 00/10] ext4: clean up ext4_ext_find_extent() Theodore Ts'o
                   ` (3 preceding siblings ...)
  2014-09-01  4:06 ` [PATCH 04/10] ext4: drop EXT4_EX_NOFREE_ON_ERR from rest of extents handling code Theodore Ts'o
@ 2014-09-01  4:06 ` Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 06/10] ext4: allow a NULL argument to ext4_ext_drop_refs() Theodore Ts'o
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Theodore Ts'o @ 2014-09-01  4:06 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

In nearly all of the calls to ext4_ext_find_extent() where the caller
is trying to recycle the path object, ext4_ext_drop_refs() gets called
to release the buffer heads before the path object gets overwritten.
To simplify things for the callers, and to avoid the possibility of a
memory leak, make ext4_ext_find_extent() responsible for dropping the
buffers.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/extents.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index f6d8b00..62aa508 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -859,8 +859,10 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block,
 	eh = ext_inode_hdr(inode);
 	depth = ext_depth(inode);
 
-	/* account possible depth increase */
-	if (!path) {
+	if (path)
+		ext4_ext_drop_refs(path);
+	else {
+		/* account possible depth increase */
 		path = kzalloc(sizeof(struct ext4_ext_path) * (depth + 2),
 				GFP_NOFS);
 		if (unlikely(!path))
@@ -1348,7 +1350,6 @@ repeat:
 			goto out;
 
 		/* refill path */
-		ext4_ext_drop_refs(path);
 		path = ext4_ext_find_extent(inode,
 				    (ext4_lblk_t)le32_to_cpu(newext->ee_block),
 				    ppath, gb_flags);
@@ -1361,7 +1362,6 @@ repeat:
 			goto out;
 
 		/* refill path */
-		ext4_ext_drop_refs(path);
 		path = ext4_ext_find_extent(inode,
 				   (ext4_lblk_t)le32_to_cpu(newext->ee_block),
 				    ppath, gb_flags);
@@ -3325,7 +3325,6 @@ static int ext4_split_extent(handle_t *handle,
 	 * Update path is required because previous ext4_split_extent_at() may
 	 * result in split of original leaf or extent zeroout.
 	 */
-	ext4_ext_drop_refs(path);
 	path = ext4_ext_find_extent(inode, map->m_lblk, ppath, 0);
 	if (IS_ERR(path))
 		return PTR_ERR(path);
@@ -3724,7 +3723,6 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle,
 						 EXT4_GET_BLOCKS_CONVERT);
 		if (err < 0)
 			return err;
-		ext4_ext_drop_refs(path);
 		path = ext4_ext_find_extent(inode, map->m_lblk, ppath, 0);
 		if (IS_ERR(path))
 			return PTR_ERR(path);
@@ -3953,7 +3951,6 @@ convert_initialized_extent(handle_t *handle, struct inode *inode,
 				EXT4_GET_BLOCKS_CONVERT_UNWRITTEN);
 		if (err < 0)
 			return err;
-		ext4_ext_drop_refs(path);
 		path = ext4_ext_find_extent(inode, map->m_lblk, ppath, 0);
 		if (IS_ERR(path))
 			return PTR_ERR(path);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 06/10] ext4: allow a NULL argument to ext4_ext_drop_refs()
  2014-09-01  4:06 [PATCH 00/10] ext4: clean up ext4_ext_find_extent() Theodore Ts'o
                   ` (4 preceding siblings ...)
  2014-09-01  4:06 ` [PATCH 05/10] ext4: call ext4_ext_drop_refs() from ext4_ext_find_extent() Theodore Ts'o
@ 2014-09-01  4:06 ` Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 07/10] ext4: teach ext4_ext_find_extent() to realloc path if necessary Theodore Ts'o
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Theodore Ts'o @ 2014-09-01  4:06 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Teach ext4_ext_drop_refs() to accept a NULL argument, much like
kfree().  This allows us to drop a lot of checks to make sure path is
non-NULL before calling ext4_ext_drop_refs().

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/extents.c        | 48 ++++++++++++++++++------------------------------
 fs/ext4/extents_status.c |  6 ++----
 fs/ext4/migrate.c        |  6 ++----
 fs/ext4/move_extent.c    | 20 +++++++-------------
 4 files changed, 29 insertions(+), 51 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 62aa508..17a1f6f 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -701,9 +701,11 @@ static void ext4_ext_show_move(struct inode *inode, struct ext4_ext_path *path,
 
 void ext4_ext_drop_refs(struct ext4_ext_path *path)
 {
-	int depth = path->p_depth;
-	int i;
+	int depth, i;
 
+	if (!path)
+		return;
+	depth = path->p_depth;
 	for (i = 0; i <= depth; i++, path++)
 		if (path->p_bh) {
 			brelse(path->p_bh);
@@ -2117,10 +2119,8 @@ merge:
 	err = ext4_ext_dirty(handle, inode, path + path->p_depth);
 
 cleanup:
-	if (npath) {
-		ext4_ext_drop_refs(npath);
-		kfree(npath);
-	}
+	ext4_ext_drop_refs(npath);
+	kfree(npath);
 	return err;
 }
 
@@ -2275,11 +2275,8 @@ static int ext4_fill_fiemap_extents(struct inode *inode,
 		block = es.es_lblk + es.es_len;
 	}
 
-	if (path) {
-		ext4_ext_drop_refs(path);
-		kfree(path);
-	}
-
+	ext4_ext_drop_refs(path);
+	kfree(path);
 	return err;
 }
 
@@ -3010,11 +3007,9 @@ again:
 		}
 	}
 out:
-	if (path) {
-		ext4_ext_drop_refs(path);
-		kfree(path);
-		path = NULL;
-	}
+	ext4_ext_drop_refs(path);
+	kfree(path);
+	path = NULL;
 	if (err == -EAGAIN)
 		goto again;
 	ext4_journal_stop(handle);
@@ -4617,10 +4612,8 @@ out:
 	map->m_pblk = newblock;
 	map->m_len = allocated;
 out2:
-	if (path) {
-		ext4_ext_drop_refs(path);
-		kfree(path);
-	}
+	ext4_ext_drop_refs(path);
+	kfree(path);
 
 	trace_ext4_ext_map_blocks_exit(inode, flags, map,
 				       err ? err : allocated);
@@ -5692,16 +5685,11 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
 		count -= len;
 
 	repeat:
-		if (path1) {
-			ext4_ext_drop_refs(path1);
-			kfree(path1);
-			path1 = NULL;
-		}
-		if (path2) {
-			ext4_ext_drop_refs(path2);
-			kfree(path2);
-			path2 = NULL;
-		}
+		ext4_ext_drop_refs(path1);
+		kfree(path1);
+		ext4_ext_drop_refs(path2);
+		kfree(path2);
+		path1 = path2 = NULL;
 	}
 	return replaced_count;
 }
diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 0b7e28e..8ffff96 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -499,10 +499,8 @@ static void ext4_es_insert_extent_ext_check(struct inode *inode,
 		}
 	}
 out:
-	if (path) {
-		ext4_ext_drop_refs(path);
-		kfree(path);
-	}
+	ext4_ext_drop_refs(path);
+	kfree(path);
 }
 
 static void ext4_es_insert_extent_ind_check(struct inode *inode,
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index aff7bdf..061c300 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -84,10 +84,8 @@ static int finish_range(handle_t *handle, struct inode *inode,
 	retval = ext4_ext_insert_extent(handle, inode, &path, &newext, 0);
 err_out:
 	up_write((&EXT4_I(inode)->i_data_sem));
-	if (path) {
-		ext4_ext_drop_refs(path);
-		kfree(path);
-	}
+	ext4_ext_drop_refs(path);
+	kfree(path);
 	lb->first_pblock = 0;
 	return retval;
 }
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 5e2465a..a34c076 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -113,10 +113,8 @@ mext_check_coverage(struct inode *inode, ext4_lblk_t from, ext4_lblk_t count,
 	}
 	ret = 1;
 out:
-	if (path) {
-		ext4_ext_drop_refs(path);
-		kfree(path);
-	}
+	ext4_ext_drop_refs(path);
+	kfree(path);
 	return ret;
 }
 
@@ -711,11 +709,9 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk,
 		o_start += cur_len;
 		d_start += cur_len;
 	repeat:
-		if (path) {
-			ext4_ext_drop_refs(path);
-			kfree(path);
-			path = NULL;
-		}
+		ext4_ext_drop_refs(path);
+		kfree(path);
+		path = NULL;
 	}
 	*moved_len = o_start - orig_blk;
 	if (*moved_len > len)
@@ -727,10 +723,8 @@ out:
 		ext4_discard_preallocations(donor_inode);
 	}
 
-	if (path) {
-		ext4_ext_drop_refs(path);
-		kfree(path);
-	}
+	ext4_ext_drop_refs(path);
+	kfree(path);
 	ext4_double_up_write_data_sem(orig_inode, donor_inode);
 	ext4_inode_resume_unlocked_dio(orig_inode);
 	ext4_inode_resume_unlocked_dio(donor_inode);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 07/10] ext4: teach ext4_ext_find_extent() to realloc path if necessary
  2014-09-01  4:06 [PATCH 00/10] ext4: clean up ext4_ext_find_extent() Theodore Ts'o
                   ` (5 preceding siblings ...)
  2014-09-01  4:06 ` [PATCH 06/10] ext4: allow a NULL argument to ext4_ext_drop_refs() Theodore Ts'o
@ 2014-09-01  4:06 ` Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 08/10] ext4: reuse path object in ext4_ext_shift_extents() Theodore Ts'o
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Theodore Ts'o @ 2014-09-01  4:06 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

This adds additional safety in case for some reason we end reusing a
path structure which isn't big enough for current depth of the inode.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/ext4_extents.h |  1 +
 fs/ext4/extents.c      | 20 ++++++++++----------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/ext4_extents.h b/fs/ext4/ext4_extents.h
index a867f5c..3c93815 100644
--- a/fs/ext4/ext4_extents.h
+++ b/fs/ext4/ext4_extents.h
@@ -123,6 +123,7 @@ find_ext4_extent_tail(struct ext4_extent_header *eh)
 struct ext4_ext_path {
 	ext4_fsblk_t			p_block;
 	__u16				p_depth;
+	__u16				p_maxdepth;
 	struct ext4_extent		*p_ext;
 	struct ext4_extent_idx		*p_idx;
 	struct ext4_extent_header	*p_hdr;
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 17a1f6f..fd840e9 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -861,14 +861,20 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block,
 	eh = ext_inode_hdr(inode);
 	depth = ext_depth(inode);
 
-	if (path)
+	if (path) {
 		ext4_ext_drop_refs(path);
-	else {
+		if (depth > path[0].p_maxdepth) {
+			kfree(path);
+			*orig_path = path = NULL;
+		}
+	}
+	if (!path) {
 		/* account possible depth increase */
 		path = kzalloc(sizeof(struct ext4_ext_path) * (depth + 2),
 				GFP_NOFS);
 		if (unlikely(!path))
 			return ERR_PTR(-ENOMEM);
+		path[0].p_maxdepth = depth + 1;
 	}
 	path[0].p_hdr = eh;
 	path[0].p_bh = NULL;
@@ -1812,6 +1818,7 @@ static void ext4_ext_try_to_merge_up(handle_t *handle,
 		sizeof(struct ext4_extent_idx);
 	s += sizeof(struct ext4_extent_header);
 
+	path[1].p_maxdepth = path[0].p_maxdepth;
 	memcpy(path[0].p_hdr, path[1].p_hdr, s);
 	path[0].p_depth = 0;
 	path[0].p_ext = EXT_FIRST_EXTENT(path[0].p_hdr) +
@@ -2142,12 +2149,6 @@ static int ext4_fill_fiemap_extents(struct inode *inode,
 		/* find extent for this block */
 		down_read(&EXT4_I(inode)->i_data_sem);
 
-		if (path && ext_depth(inode) != depth) {
-			/* depth was changed. we have to realloc path */
-			kfree(path);
-			path = NULL;
-		}

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 08/10] ext4: reuse path object in ext4_ext_shift_extents()
  2014-09-01  4:06 [PATCH 00/10] ext4: clean up ext4_ext_find_extent() Theodore Ts'o
                   ` (6 preceding siblings ...)
  2014-09-01  4:06 ` [PATCH 07/10] ext4: teach ext4_ext_find_extent() to realloc path if necessary Theodore Ts'o
@ 2014-09-01  4:06 ` Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 09/10] ext4: reuse path object in ext4_move_extents() Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 10/10] ext4: rename ext4_ext_find_extent() to ext4_find_extent() Theodore Ts'o
  9 siblings, 0 replies; 11+ messages in thread
From: Theodore Ts'o @ 2014-09-01  4:06 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Now that the semantics of ext4_ext_find_extent() are much cleaner,
it's safe and more efficient to reuse the path object across the
multiple calls to ext4_ext_find_extent() in ext4_ext_shift_extents().

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/extents.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index fd840e9..bfae63b 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5305,26 +5305,21 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
 
 	depth = path->p_depth;
 	extent = path[depth].p_ext;
-	if (!extent) {
-		ext4_ext_drop_refs(path);
-		kfree(path);
-		return ret;
-	}
+	if (!extent)
+		goto out;
 
 	stop_block = le32_to_cpu(extent->ee_block) +
 			ext4_ext_get_actual_len(extent);
-	ext4_ext_drop_refs(path);
-	kfree(path);
 
 	/* Nothing to shift, if hole is at the end of file */
 	if (start >= stop_block)
-		return ret;
+		goto out;
 
 	/*
 	 * Don't start shifting extents until we make sure the hole is big
 	 * enough to accomodate the shift.
 	 */
-	path = ext4_ext_find_extent(inode, start - 1, NULL, 0);
+	path = ext4_ext_find_extent(inode, start - 1, &path, 0);
 	if (IS_ERR(path))
 		return PTR_ERR(path);
 	depth = path->p_depth;
@@ -5337,8 +5332,6 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
 		ex_start = 0;
 		ex_end = 0;
 	}
-	ext4_ext_drop_refs(path);
-	kfree(path);
 
 	if ((start == ex_start && shift > ex_start) ||
 	    (shift > start - ex_end))
@@ -5346,7 +5339,7 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
 
 	/* Its safe to start updating extents */
 	while (start < stop_block) {
-		path = ext4_ext_find_extent(inode, start, NULL, 0);
+		path = ext4_ext_find_extent(inode, start, &path, 0);
 		if (IS_ERR(path))
 			return PTR_ERR(path);
 		depth = path->p_depth;
@@ -5362,19 +5355,17 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
 				path[depth].p_ext++;
 			} else {
 				start = ext4_ext_next_allocated_block(path);
-				ext4_ext_drop_refs(path);
-				kfree(path);
 				continue;
 			}
 		}
 		ret = ext4_ext_shift_path_extents(path, shift, inode,
 				handle, &start);
-		ext4_ext_drop_refs(path);
-		kfree(path);
 		if (ret)
 			break;
 	}

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 09/10] ext4: reuse path object in ext4_move_extents()
  2014-09-01  4:06 [PATCH 00/10] ext4: clean up ext4_ext_find_extent() Theodore Ts'o
                   ` (7 preceding siblings ...)
  2014-09-01  4:06 ` [PATCH 08/10] ext4: reuse path object in ext4_ext_shift_extents() Theodore Ts'o
@ 2014-09-01  4:06 ` Theodore Ts'o
  2014-09-01  4:06 ` [PATCH 10/10] ext4: rename ext4_ext_find_extent() to ext4_find_extent() Theodore Ts'o
  9 siblings, 0 replies; 11+ messages in thread
From: Theodore Ts'o @ 2014-09-01  4:06 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Reuse the path object in ext4_move_extents() so we don't unnecessarily
free and reallocate it.

Also clean up the get_ext_path() wrapper so that it has the same
semantics of freeing the path object on error as ext4_ext_find_extent().

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/move_extent.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index a34c076..7bf970d 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -32,20 +32,21 @@
  */
 static inline int
 get_ext_path(struct inode *inode, ext4_lblk_t lblock,
-		struct ext4_ext_path **orig_path)
+		struct ext4_ext_path **ppath)
 {
-	int ret = 0;
 	struct ext4_ext_path *path;
 
-	path = ext4_ext_find_extent(inode, lblock, orig_path, EXT4_EX_NOCACHE);
+	path = ext4_ext_find_extent(inode, lblock, ppath, EXT4_EX_NOCACHE);
 	if (IS_ERR(path))
-		ret = PTR_ERR(path);
-	else if (path[ext_depth(inode)].p_ext == NULL)
-		ret = -ENODATA;
-	else
-		*orig_path = path;
-
-	return ret;
+		return PTR_ERR(path);
+	if (path[ext_depth(inode)].p_ext == NULL) {
+		ext4_ext_drop_refs(path);
+		kfree(path);
+		*ppath = NULL;
+		return -ENODATA;
+	}
+	*ppath = path;
+	return 0;
 }
 
 /**
@@ -667,7 +668,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk,
 			}
 			d_start += next_blk - o_start;
 			o_start = next_blk;
-			goto repeat;
+			continue;
 		/* Check hole after the start pos */
 		} else if (cur_blk > o_start) {
 			/* Skip hole */
@@ -708,10 +709,6 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk,
 			break;
 		o_start += cur_len;
 		d_start += cur_len;
-	repeat:
-		ext4_ext_drop_refs(path);
-		kfree(path);
-		path = NULL;
 	}
 	*moved_len = o_start - orig_blk;
 	if (*moved_len > len)
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 10/10] ext4: rename ext4_ext_find_extent() to ext4_find_extent()
  2014-09-01  4:06 [PATCH 00/10] ext4: clean up ext4_ext_find_extent() Theodore Ts'o
                   ` (8 preceding siblings ...)
  2014-09-01  4:06 ` [PATCH 09/10] ext4: reuse path object in ext4_move_extents() Theodore Ts'o
@ 2014-09-01  4:06 ` Theodore Ts'o
  9 siblings, 0 replies; 11+ messages in thread
From: Theodore Ts'o @ 2014-09-01  4:06 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Make the function name less redundant.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/ext4.h           |  8 ++++----
 fs/ext4/extents.c        | 38 +++++++++++++++++++-------------------
 fs/ext4/extents_status.c |  2 +-
 fs/ext4/migrate.c        |  3 +--
 fs/ext4/move_extent.c    |  4 ++--
 5 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 4a5a6b9..c07f43f 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -573,7 +573,7 @@ enum {
 
 /*
  * The bit position of these flags must not overlap with any of the
- * EXT4_GET_BLOCKS_*.  They are used by ext4_ext_find_extent(),
+ * EXT4_GET_BLOCKS_*.  They are used by ext4_find_extent(),
  * read_extent_tree_block(), ext4_split_extent_at(),
  * ext4_ext_insert_extent(), and ext4_ext_create_new_leaf().
  * EXT4_EX_NOCACHE is used to indicate that the we shouldn't be
@@ -2732,9 +2732,9 @@ extern int ext4_can_extents_be_merged(struct inode *inode,
 extern int ext4_ext_insert_extent(handle_t *, struct inode *,
 				  struct ext4_ext_path **,
 				  struct ext4_extent *, int);
-extern struct ext4_ext_path *ext4_ext_find_extent(struct inode *, ext4_lblk_t,
-						  struct ext4_ext_path **,
-						  int flags);
+extern struct ext4_ext_path *ext4_find_extent(struct inode *, ext4_lblk_t,
+					      struct ext4_ext_path **,
+					      int flags);
 extern void ext4_ext_drop_refs(struct ext4_ext_path *);
 extern int ext4_ext_check_inode(struct inode *inode);
 extern int ext4_find_delalloc_range(struct inode *inode,
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index bfae63b..47b9aa1 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -849,8 +849,8 @@ int ext4_ext_tree_init(handle_t *handle, struct inode *inode)
 }
 
 struct ext4_ext_path *
-ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block,
-		     struct ext4_ext_path **orig_path, int flags)
+ext4_find_extent(struct inode *inode, ext4_lblk_t block,
+		 struct ext4_ext_path **orig_path, int flags)
 {
 	struct ext4_extent_header *eh;
 	struct buffer_head *bh;
@@ -1358,7 +1358,7 @@ repeat:
 			goto out;
 
 		/* refill path */
-		path = ext4_ext_find_extent(inode,
+		path = ext4_find_extent(inode,
 				    (ext4_lblk_t)le32_to_cpu(newext->ee_block),
 				    ppath, gb_flags);
 		if (IS_ERR(path))
@@ -1370,7 +1370,7 @@ repeat:
 			goto out;
 
 		/* refill path */
-		path = ext4_ext_find_extent(inode,
+		path = ext4_find_extent(inode,
 				   (ext4_lblk_t)le32_to_cpu(newext->ee_block),
 				    ppath, gb_flags);
 		if (IS_ERR(path)) {
@@ -1943,7 +1943,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode,
 		/*
 		 * Try to see whether we should rather test the extent on
 		 * right from ex, or from the left of ex. This is because
-		 * ext4_ext_find_extent() can return either extent on the
+		 * ext4_find_extent() can return either extent on the
 		 * left, or on the right from the searched position. This
 		 * will make merging more effective.
 		 */
@@ -2026,7 +2026,7 @@ prepend:
 	if (next != EXT_MAX_BLOCKS) {
 		ext_debug("next leaf block - %u\n", next);
 		BUG_ON(npath != NULL);
-		npath = ext4_ext_find_extent(inode, next, NULL, 0);
+		npath = ext4_find_extent(inode, next, NULL, 0);
 		if (IS_ERR(npath))
 			return PTR_ERR(npath);
 		BUG_ON(npath->p_depth != path->p_depth);
@@ -2149,7 +2149,7 @@ static int ext4_fill_fiemap_extents(struct inode *inode,
 		/* find extent for this block */
 		down_read(&EXT4_I(inode)->i_data_sem);
 
-		path = ext4_ext_find_extent(inode, block, &path, 0);
+		path = ext4_find_extent(inode, block, &path, 0);
 		if (IS_ERR(path)) {
 			up_read(&EXT4_I(inode)->i_data_sem);
 			err = PTR_ERR(path);
@@ -2832,7 +2832,7 @@ again:
 		ext4_lblk_t ee_block;
 
 		/* find extent for this block */
-		path = ext4_ext_find_extent(inode, end, NULL, EXT4_EX_NOCACHE);
+		path = ext4_find_extent(inode, end, NULL, EXT4_EX_NOCACHE);
 		if (IS_ERR(path)) {
 			ext4_journal_stop(handle);
 			return PTR_ERR(path);
@@ -3320,7 +3320,7 @@ static int ext4_split_extent(handle_t *handle,
 	 * Update path is required because previous ext4_split_extent_at() may
 	 * result in split of original leaf or extent zeroout.
 	 */
-	path = ext4_ext_find_extent(inode, map->m_lblk, ppath, 0);
+	path = ext4_find_extent(inode, map->m_lblk, ppath, 0);
 	if (IS_ERR(path))
 		return PTR_ERR(path);
 	depth = ext_depth(inode);
@@ -3718,7 +3718,7 @@ static int ext4_convert_unwritten_extents_endio(handle_t *handle,
 						 EXT4_GET_BLOCKS_CONVERT);
 		if (err < 0)
 			return err;
-		path = ext4_ext_find_extent(inode, map->m_lblk, ppath, 0);
+		path = ext4_find_extent(inode, map->m_lblk, ppath, 0);
 		if (IS_ERR(path))
 			return PTR_ERR(path);
 		depth = ext_depth(inode);
@@ -3946,7 +3946,7 @@ convert_initialized_extent(handle_t *handle, struct inode *inode,
 				EXT4_GET_BLOCKS_CONVERT_UNWRITTEN);
 		if (err < 0)
 			return err;
-		path = ext4_ext_find_extent(inode, map->m_lblk, ppath, 0);
+		path = ext4_find_extent(inode, map->m_lblk, ppath, 0);
 		if (IS_ERR(path))
 			return PTR_ERR(path);
 		depth = ext_depth(inode);
@@ -4272,7 +4272,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 	trace_ext4_ext_map_blocks_enter(inode, map->m_lblk, map->m_len, flags);
 
 	/* find extent for this block */
-	path = ext4_ext_find_extent(inode, map->m_lblk, NULL, 0);
+	path = ext4_find_extent(inode, map->m_lblk, NULL, 0);
 	if (IS_ERR(path)) {
 		err = PTR_ERR(path);
 		path = NULL;
@@ -4284,7 +4284,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 	/*
 	 * consistent leaf must not be empty;
 	 * this situation is possible, though, _during_ tree modification;
-	 * this is why assert can't be put in ext4_ext_find_extent()
+	 * this is why assert can't be put in ext4_find_extent()
 	 */
 	if (unlikely(path[depth].p_ext == NULL && depth != 0)) {
 		EXT4_ERROR_INODE(inode, "bad extent address "
@@ -4369,7 +4369,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 
 	/*
 	 * If we are doing bigalloc, check to see if the extent returned
-	 * by ext4_ext_find_extent() implies a cluster we can use.
+	 * by ext4_find_extent() implies a cluster we can use.
 	 */
 	if (cluster_offset && ex &&
 	    get_implied_cluster_alloc(inode->i_sb, map, ex, path)) {
@@ -5299,7 +5299,7 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
 	ext4_lblk_t ex_start, ex_end;
 
 	/* Let path point to the last extent */
-	path = ext4_ext_find_extent(inode, EXT_MAX_BLOCKS - 1, NULL, 0);
+	path = ext4_find_extent(inode, EXT_MAX_BLOCKS - 1, NULL, 0);
 	if (IS_ERR(path))
 		return PTR_ERR(path);
 
@@ -5319,7 +5319,7 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
 	 * Don't start shifting extents until we make sure the hole is big
 	 * enough to accomodate the shift.
 	 */
-	path = ext4_ext_find_extent(inode, start - 1, &path, 0);
+	path = ext4_find_extent(inode, start - 1, &path, 0);
 	if (IS_ERR(path))
 		return PTR_ERR(path);
 	depth = path->p_depth;
@@ -5339,7 +5339,7 @@ ext4_ext_shift_extents(struct inode *inode, handle_t *handle,
 
 	/* Its safe to start updating extents */
 	while (start < stop_block) {
-		path = ext4_ext_find_extent(inode, start, &path, 0);
+		path = ext4_find_extent(inode, start, &path, 0);
 		if (IS_ERR(path))
 			return PTR_ERR(path);
 		depth = path->p_depth;
@@ -5536,7 +5536,7 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
 		int e1_len, e2_len, len;
 		int split = 0;
 
-		path1 = ext4_ext_find_extent(inode1, lblk1, NULL, EXT4_EX_NOCACHE);
+		path1 = ext4_find_extent(inode1, lblk1, NULL, EXT4_EX_NOCACHE);
 		if (unlikely(IS_ERR(path1))) {
 			*erp = PTR_ERR(path1);
 			path1 = NULL;
@@ -5544,7 +5544,7 @@ ext4_swap_extents(handle_t *handle, struct inode *inode1,
 			count = 0;
 			goto repeat;
 		}
-		path2 = ext4_ext_find_extent(inode2, lblk2, NULL, EXT4_EX_NOCACHE);
+		path2 = ext4_find_extent(inode2, lblk2, NULL, EXT4_EX_NOCACHE);
 		if (unlikely(IS_ERR(path2))) {
 			*erp = PTR_ERR(path2);
 			path2 = NULL;
diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 8ffff96..bdd400c 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -426,7 +426,7 @@ static void ext4_es_insert_extent_ext_check(struct inode *inode,
 	unsigned short ee_len;
 	int depth, ee_status, es_status;
 
-	path = ext4_ext_find_extent(inode, es->es_lblk, NULL, EXT4_EX_NOCACHE);
+	path = ext4_find_extent(inode, es->es_lblk, NULL, EXT4_EX_NOCACHE);
 	if (IS_ERR(path))
 		return;
 
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 061c300..a432634 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -41,8 +41,7 @@ static int finish_range(handle_t *handle, struct inode *inode,
 	ext4_ext_store_pblock(&newext, lb->first_pblock);
 	/* Locking only for convinience since we are operating on temp inode */
 	down_write(&EXT4_I(inode)->i_data_sem);
-	path = ext4_ext_find_extent(inode, lb->first_block, NULL, 0);
-
+	path = ext4_find_extent(inode, lb->first_block, NULL, 0);
 	if (IS_ERR(path)) {
 		retval = PTR_ERR(path);
 		path = NULL;
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 7bf970d..5d78063 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -27,7 +27,7 @@
  * @lblock:	logical block number to find an extent path
  * @path:	pointer to an extent path pointer (for output)
  *
- * ext4_ext_find_extent wrapper. Return 0 on success, or a negative error value
+ * ext4_find_extent wrapper. Return 0 on success, or a negative error value
  * on failure.
  */
 static inline int
@@ -36,7 +36,7 @@ get_ext_path(struct inode *inode, ext4_lblk_t lblock,
 {
 	struct ext4_ext_path *path;
 
-	path = ext4_ext_find_extent(inode, lblock, ppath, EXT4_EX_NOCACHE);
+	path = ext4_find_extent(inode, lblock, ppath, EXT4_EX_NOCACHE);
 	if (IS_ERR(path))
 		return PTR_ERR(path);
 	if (path[ext_depth(inode)].p_ext == NULL) {
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-09-01  4:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-01  4:06 [PATCH 00/10] ext4: clean up ext4_ext_find_extent() Theodore Ts'o
2014-09-01  4:06 ` [PATCH 01/10] ext4: teach ext4_ext_find_extent() to free path on error Theodore Ts'o
2014-09-01  4:06 ` [PATCH 02/10] ext4: collapse ext4_convert_initialized_extents() Theodore Ts'o
2014-09-01  4:06 ` [PATCH 03/10] ext4: drop EXT4_EX_NOFREE_ON_ERR in convert_initialized_extent() Theodore Ts'o
2014-09-01  4:06 ` [PATCH 04/10] ext4: drop EXT4_EX_NOFREE_ON_ERR from rest of extents handling code Theodore Ts'o
2014-09-01  4:06 ` [PATCH 05/10] ext4: call ext4_ext_drop_refs() from ext4_ext_find_extent() Theodore Ts'o
2014-09-01  4:06 ` [PATCH 06/10] ext4: allow a NULL argument to ext4_ext_drop_refs() Theodore Ts'o
2014-09-01  4:06 ` [PATCH 07/10] ext4: teach ext4_ext_find_extent() to realloc path if necessary Theodore Ts'o
2014-09-01  4:06 ` [PATCH 08/10] ext4: reuse path object in ext4_ext_shift_extents() Theodore Ts'o
2014-09-01  4:06 ` [PATCH 09/10] ext4: reuse path object in ext4_move_extents() Theodore Ts'o
2014-09-01  4:06 ` [PATCH 10/10] ext4: rename ext4_ext_find_extent() to ext4_find_extent() Theodore Ts'o

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).