* [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