[PATCH v4 0/2] fix extents need to be restored when ext4_ext_insert_extent failed

[PATCH v4 0/2] fix extents need to be restored when ext4_ext_insert_extent failed

[zhanchengbin]

Inside the ext4_ext_insert_extent function, every error returned will
not destroy the consistency of the tree. Even if it fails after changing
half of the tree, can also ensure that the tree is self-consistent, like
function ext4_ext_create_new_leaf.
After ext4_ext_insert_extent fails, update extent status tree depends on
the incoming split_flag. So restore the len of extent to be split when
ext4_ext_insert_extent return failed in ext4_split_extent_at.

Diff v2 Vs v1:
1) return directly after inserting successfully
2) restore the length of extent in memory after inserting unsuccessfully

Diff v3 Vs v2:
Sorry for not taking into account the successful extent insertion. and I
reanalyzed the ext4_ext_insert_extent function and modified the conditions
for restoring the length.

Diff v4 Vs v3:
Clear the verified flag from the modified bh when failed inext4_ext_rm_idx
or ext4_ext_correct_indexes.

zhanchengbin (2):
  ext4: fix inode tree inconsistency caused by ENOMEM in
    ext4_split_extent_at
  ext4: clear the verified flag of the modified leaf or idx if error

 fs/ext4/extents.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

-- 
2.31.1

[PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at

[zhanchengbin]

If ENOMEM fails when the extent is splitting, we need to restore the length
of the split extent.
In the call stack of the ext4_split_extent_at function, only in
ext4_ext_create_new_leaf will it alloc memory and change the shape of the
extent tree,even if an ENOMEM is returned at this time, the extent tree is
still self-consistent, Just restore the split extent lens in the function
ext4_split_extent_at.

ext4_split_extent_at
 ext4_ext_insert_extent
  ext4_ext_create_new_leaf
   1)ext4_ext_split
     ext4_find_extent
   2)ext4_ext_grow_indepth
     ext4_find_extent

Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
---
 fs/ext4/extents.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 9de1c9d1a13d..0f95e857089e 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -935,6 +935,7 @@ ext4_find_extent(struct inode *inode, ext4_lblk_t block,
 
 		bh = read_extent_tree_block(inode, path[ppos].p_idx, --i, flags);
 		if (IS_ERR(bh)) {
+			EXT4_ERROR_INODE(inode, "IO error reading extent block");
 			ret = PTR_ERR(bh);
 			goto err;
 		}
@@ -3251,7 +3252,7 @@ static int ext4_split_extent_at(handle_t *handle,
 		ext4_ext_mark_unwritten(ex2);
 
 	err = ext4_ext_insert_extent(handle, inode, ppath, &newex, flags);
-	if (err != -ENOSPC && err != -EDQUOT)
+	if (err != -ENOSPC && err != -EDQUOT && err != -ENOMEM)
 		goto out;
 
 	if (EXT4_EXT_MAY_ZEROOUT & split_flag) {
-- 
2.31.1

[PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error

[zhanchengbin]

Clear the verified flag from the modified bh when failed in ext4_ext_rm_idx
or ext4_ext_correct_indexes.
In this way, the start value of the logical block itself and its
parents' will be checked in ext4_valid_extent_entries.

Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
---
 fs/ext4/extents.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 0f95e857089e..9013a05f524b 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -1756,6 +1756,8 @@ static int ext4_ext_correct_indexes(handle_t *handle, struct inode *inode,
 		if (err)
 			break;
 	}
+	while (!(k < 0) && k++ < depth)
+		clear_buffer_verified(path[k]->p_bh);
 
 	return err;
 }
@@ -2304,6 +2306,7 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode,
 {
 	int err;
 	ext4_fsblk_t leaf;
+	int b_depth = depth;
 
 	/* free index block */
 	depth--;
@@ -2345,6 +2348,9 @@ static int ext4_ext_rm_idx(handle_t *handle, struct inode *inode,
 		if (err)
 			break;
 	}
+	while (!(depth < 0) && depth++ < b_depth - 1)
+		clear_buffer_verified(path[depth]->p_bh);
+
 	return err;
 }
 
-- 
2.31.1

Re: [PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error

[kernel test robot]

Hi zhanchengbin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on jack-fs/for_next linus/master v6.2-rc8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/zhanchengbin/ext4-fix-inode-tree-inconsistency-caused-by-ENOMEM-in-ext4_split_extent_at/20230213-114334
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20230213040522.3339406-3-zhanchengbin1%40huawei.com
patch subject: [PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error
config: arm-randconfig-r024-20230213 (https://download.01.org/0day-ci/archive/20230213/202302131414.5RKeHgAZ-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project db0e6591612b53910a1b366863348bdb9d7d2fb1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/c6de5d67952addd5ffa288574ed55ebe7aeba755
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review zhanchengbin/ext4-fix-inode-tree-inconsistency-caused-by-ENOMEM-in-ext4_split_extent_at/20230213-114334
        git checkout c6de5d67952addd5ffa288574ed55ebe7aeba755
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302131414.5RKeHgAZ-lkp@intel.com/

All errors (new ones prefixed by >>):

>> fs/ext4/extents.c:1760:32: error: member reference type 'struct ext4_ext_path' is not a pointer; did you mean to use '.'?
                   clear_buffer_verified(path[k]->p_bh);
                                         ~~~~~~~^~
                                                .
   fs/ext4/extents.c:2352:36: error: member reference type 'struct ext4_ext_path' is not a pointer; did you mean to use '.'?
                   clear_buffer_verified(path[depth]->p_bh);
                                         ~~~~~~~~~~~^~
                                                    .
   2 errors generated.


vim +1760 fs/ext4/extents.c

  1699	
  1700	/*
  1701	 * ext4_ext_correct_indexes:
  1702	 * if leaf gets modified and modified extent is first in the leaf,
  1703	 * then we have to correct all indexes above.
  1704	 * TODO: do we need to correct tree in all cases?
  1705	 */
  1706	static int ext4_ext_correct_indexes(handle_t *handle, struct inode *inode,
  1707					struct ext4_ext_path *path)
  1708	{
  1709		struct ext4_extent_header *eh;
  1710		int depth = ext_depth(inode);
  1711		struct ext4_extent *ex;
  1712		__le32 border;
  1713		int k, err = 0;
  1714	
  1715		eh = path[depth].p_hdr;
  1716		ex = path[depth].p_ext;
  1717	
  1718		if (unlikely(ex == NULL || eh == NULL)) {
  1719			EXT4_ERROR_INODE(inode,
  1720					 "ex %p == NULL or eh %p == NULL", ex, eh);
  1721			return -EFSCORRUPTED;
  1722		}
  1723	
  1724		if (depth == 0) {
  1725			/* there is no tree at all */
  1726			return 0;
  1727		}
  1728	
  1729		if (ex != EXT_FIRST_EXTENT(eh)) {
  1730			/* we correct tree if first leaf got modified only */
  1731			return 0;
  1732		}
  1733	
  1734		/*
  1735		 * TODO: we need correction if border is smaller than current one
  1736		 */
  1737		k = depth - 1;
  1738		border = path[depth].p_ext->ee_block;
  1739		err = ext4_ext_get_access(handle, inode, path + k);
  1740		if (err)
  1741			return err;
  1742		path[k].p_idx->ei_block = border;
  1743		err = ext4_ext_dirty(handle, inode, path + k);
  1744		if (err)
  1745			return err;
  1746	
  1747		while (k--) {
  1748			/* change all left-side indexes */
  1749			if (path[k+1].p_idx != EXT_FIRST_INDEX(path[k+1].p_hdr))
  1750				break;
  1751			err = ext4_ext_get_access(handle, inode, path + k);
  1752			if (err)
  1753				break;
  1754			path[k].p_idx->ei_block = border;
  1755			err = ext4_ext_dirty(handle, inode, path + k);
  1756			if (err)
  1757				break;
  1758		}
  1759		while (!(k < 0) && k++ < depth)
> 1760			clear_buffer_verified(path[k]->p_bh);
  1761	
  1762		return err;
  1763	}
  1764	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error

[kernel test robot]

Hi zhanchengbin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on jack-fs/for_next linus/master v6.2-rc8 next-20230210]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/zhanchengbin/ext4-fix-inode-tree-inconsistency-caused-by-ENOMEM-in-ext4_split_extent_at/20230213-114334
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20230213040522.3339406-3-zhanchengbin1%40huawei.com
patch subject: [PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error
config: i386-randconfig-a004-20230213 (https://download.01.org/0day-ci/archive/20230213/202302131407.XrieHNuN-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/c6de5d67952addd5ffa288574ed55ebe7aeba755
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review zhanchengbin/ext4-fix-inode-tree-inconsistency-caused-by-ENOMEM-in-ext4_split_extent_at/20230213-114334
        git checkout c6de5d67952addd5ffa288574ed55ebe7aeba755
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash fs/ext4/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302131407.XrieHNuN-lkp@intel.com/

All errors (new ones prefixed by >>):

>> fs/ext4/extents.c:1760:32: error: member reference type 'struct ext4_ext_path' is not a pointer; did you mean to use '.'?
                   clear_buffer_verified(path[k]->p_bh);
                                         ~~~~~~~^~
                                                .
   fs/ext4/extents.c:2352:36: error: member reference type 'struct ext4_ext_path' is not a pointer; did you mean to use '.'?
                   clear_buffer_verified(path[depth]->p_bh);
                                         ~~~~~~~~~~~^~
                                                    .
   2 errors generated.


vim +1760 fs/ext4/extents.c

  1699	
  1700	/*
  1701	 * ext4_ext_correct_indexes:
  1702	 * if leaf gets modified and modified extent is first in the leaf,
  1703	 * then we have to correct all indexes above.
  1704	 * TODO: do we need to correct tree in all cases?
  1705	 */
  1706	static int ext4_ext_correct_indexes(handle_t *handle, struct inode *inode,
  1707					struct ext4_ext_path *path)
  1708	{
  1709		struct ext4_extent_header *eh;
  1710		int depth = ext_depth(inode);
  1711		struct ext4_extent *ex;
  1712		__le32 border;
  1713		int k, err = 0;
  1714	
  1715		eh = path[depth].p_hdr;
  1716		ex = path[depth].p_ext;
  1717	
  1718		if (unlikely(ex == NULL || eh == NULL)) {
  1719			EXT4_ERROR_INODE(inode,
  1720					 "ex %p == NULL or eh %p == NULL", ex, eh);
  1721			return -EFSCORRUPTED;
  1722		}
  1723	
  1724		if (depth == 0) {
  1725			/* there is no tree at all */
  1726			return 0;
  1727		}
  1728	
  1729		if (ex != EXT_FIRST_EXTENT(eh)) {
  1730			/* we correct tree if first leaf got modified only */
  1731			return 0;
  1732		}
  1733	
  1734		/*
  1735		 * TODO: we need correction if border is smaller than current one
  1736		 */
  1737		k = depth - 1;
  1738		border = path[depth].p_ext->ee_block;
  1739		err = ext4_ext_get_access(handle, inode, path + k);
  1740		if (err)
  1741			return err;
  1742		path[k].p_idx->ei_block = border;
  1743		err = ext4_ext_dirty(handle, inode, path + k);
  1744		if (err)
  1745			return err;
  1746	
  1747		while (k--) {
  1748			/* change all left-side indexes */
  1749			if (path[k+1].p_idx != EXT_FIRST_INDEX(path[k+1].p_hdr))
  1750				break;
  1751			err = ext4_ext_get_access(handle, inode, path + k);
  1752			if (err)
  1753				break;
  1754			path[k].p_idx->ei_block = border;
  1755			err = ext4_ext_dirty(handle, inode, path + k);
  1756			if (err)
  1757				break;
  1758		}
  1759		while (!(k < 0) && k++ < depth)
> 1760			clear_buffer_verified(path[k]->p_bh);
  1761	
  1762		return err;
  1763	}
  1764	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at

[Jan Kara]

On Mon 13-02-23 12:05:21, zhanchengbin wrote:
> If ENOMEM fails when the extent is splitting, we need to restore the length
> of the split extent.
> In the call stack of the ext4_split_extent_at function, only in
> ext4_ext_create_new_leaf will it alloc memory and change the shape of the
> extent tree,even if an ENOMEM is returned at this time, the extent tree is
> still self-consistent, Just restore the split extent lens in the function
> ext4_split_extent_at.
> 
> ext4_split_extent_at
>  ext4_ext_insert_extent
>   ext4_ext_create_new_leaf
>    1)ext4_ext_split
>      ext4_find_extent
>    2)ext4_ext_grow_indepth
>      ext4_find_extent
> 
> Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
> ---
>  fs/ext4/extents.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 9de1c9d1a13d..0f95e857089e 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -935,6 +935,7 @@ ext4_find_extent(struct inode *inode, ext4_lblk_t block,
>  
>  		bh = read_extent_tree_block(inode, path[ppos].p_idx, --i, flags);
>  		if (IS_ERR(bh)) {
> +			EXT4_ERROR_INODE(inode, "IO error reading extent block");

Why have you added this? Usually we don't log any additional errors for IO
errors because the storage layer already reports it... Furthermore this
would potentialy panic the system / remount the fs RO which we also usually
don't do in case of IO errors, only in case of FS corruption.

								Honza

>  			ret = PTR_ERR(bh);
>  			goto err;
>  		}
> @@ -3251,7 +3252,7 @@ static int ext4_split_extent_at(handle_t *handle,
>  		ext4_ext_mark_unwritten(ex2);
>  
>  	err = ext4_ext_insert_extent(handle, inode, ppath, &newex, flags);
> -	if (err != -ENOSPC && err != -EDQUOT)
> +	if (err != -ENOSPC && err != -EDQUOT && err != -ENOMEM)
>  		goto out;
>  
>  	if (EXT4_EXT_MAY_ZEROOUT & split_flag) {
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at

[zhanchengbin]

On 2023/2/14 19:48, Jan Kara wrote:
> On Mon 13-02-23 12:05:21, zhanchengbin wrote:
>> If ENOMEM fails when the extent is splitting, we need to restore the length
>> of the split extent.
>> In the call stack of the ext4_split_extent_at function, only in
>> ext4_ext_create_new_leaf will it alloc memory and change the shape of the
>> extent tree,even if an ENOMEM is returned at this time, the extent tree is
>> still self-consistent, Just restore the split extent lens in the function
>> ext4_split_extent_at.
>>
>> ext4_split_extent_at
>>   ext4_ext_insert_extent
>>    ext4_ext_create_new_leaf
>>     1)ext4_ext_split
>>       ext4_find_extent
>>     2)ext4_ext_grow_indepth
>>       ext4_find_extent
>>
>> Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
>> ---
>>   fs/ext4/extents.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>> index 9de1c9d1a13d..0f95e857089e 100644
>> --- a/fs/ext4/extents.c
>> +++ b/fs/ext4/extents.c
>> @@ -935,6 +935,7 @@ ext4_find_extent(struct inode *inode, ext4_lblk_t block,
>>   
>>   		bh = read_extent_tree_block(inode, path[ppos].p_idx, --i, flags);
>>   		if (IS_ERR(bh)) {
>> +			EXT4_ERROR_INODE(inode, "IO error reading extent block");
> 
> Why have you added this? Usually we don't log any additional errors for IO
> errors because the storage layer already reports it... Furthermore this
> would potentialy panic the system / remount the fs RO which we also usually
> don't do in case of IO errors, only in case of FS corruption.
> 
> 								Honza

Because failure of read_extent_tree_block indirectly leads to filesystem
inconsistency in ext4_split_extent_at, I want the filesystem to become
read-only after failure.

  - bin.

> 
>>   			ret = PTR_ERR(bh);
>>   			goto err;
>>   		}
>> @@ -3251,7 +3252,7 @@ static int ext4_split_extent_at(handle_t *handle,
>>   		ext4_ext_mark_unwritten(ex2);
>>   
>>   	err = ext4_ext_insert_extent(handle, inode, ppath, &newex, flags);
>> -	if (err != -ENOSPC && err != -EDQUOT)
>> +	if (err != -ENOSPC && err != -EDQUOT && err != -ENOMEM)
>>   		goto out;
>>   
>>   	if (EXT4_EXT_MAY_ZEROOUT & split_flag) {
>> -- 
>> 2.31.1
>>

Re: [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at

[Jan Kara]

On Wed 15-02-23 16:51:23, zhanchengbin wrote:
> 
> On 2023/2/14 19:48, Jan Kara wrote:
> > On Mon 13-02-23 12:05:21, zhanchengbin wrote:
> > > If ENOMEM fails when the extent is splitting, we need to restore the length
> > > of the split extent.
> > > In the call stack of the ext4_split_extent_at function, only in
> > > ext4_ext_create_new_leaf will it alloc memory and change the shape of the
> > > extent tree,even if an ENOMEM is returned at this time, the extent tree is
> > > still self-consistent, Just restore the split extent lens in the function
> > > ext4_split_extent_at.
> > > 
> > > ext4_split_extent_at
> > >   ext4_ext_insert_extent
> > >    ext4_ext_create_new_leaf
> > >     1)ext4_ext_split
> > >       ext4_find_extent
> > >     2)ext4_ext_grow_indepth
> > >       ext4_find_extent
> > > 
> > > Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com>
> > > ---
> > >   fs/ext4/extents.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> > > index 9de1c9d1a13d..0f95e857089e 100644
> > > --- a/fs/ext4/extents.c
> > > +++ b/fs/ext4/extents.c
> > > @@ -935,6 +935,7 @@ ext4_find_extent(struct inode *inode, ext4_lblk_t block,
> > >   		bh = read_extent_tree_block(inode, path[ppos].p_idx, --i, flags);
> > >   		if (IS_ERR(bh)) {
> > > +			EXT4_ERROR_INODE(inode, "IO error reading extent block");
> > 
> > Why have you added this? Usually we don't log any additional errors for IO
> > errors because the storage layer already reports it... Furthermore this
> > would potentialy panic the system / remount the fs RO which we also usually
> > don't do in case of IO errors, only in case of FS corruption.
> > 
> > 								Honza
> 
> Because failure of read_extent_tree_block indirectly leads to filesystem
> inconsistency in ext4_split_extent_at, I want the filesystem to become
> read-only after failure.

Can you please describe how exactly? Because I'd rather declare the error
directly in ext4_split_extent_at() than in ext4_find_extent() unless it
gets too complicated...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at

[Theodore Ts'o]

On Wed, Feb 15, 2023 at 04:51:23PM +0800, zhanchengbin wrote:
> > > 
> > > 
> 
> Because failure of read_extent_tree_block indirectly leads to filesystem
> inconsistency in ext4_split_extent_at, I want the filesystem to become
> read-only after failure.

If I understand your concern correctly, the problem you're trying to
solve is that in ext4_ext_create_new_leaf() we can't easily unwind the
file system mutation in process if ext4_find_extent() fails here:

> > > ext4_split_extent_at
> > >   ext4_ext_insert_extent
> > >    ext4_ext_create_new_leaf
> > >     1)ext4_ext_split
> > >       ext4_find_extent
> > >     2)ext4_ext_grow_indepth
> > >       ext4_find_extent      <=======

Is that your concern?

If so, it seems to me that there are two reasons why
ext4_find_extent() could fail.  The first is that it could be because
there is a memory allocation failure.  The second is that there is an
I/O error when it actually tries to read the extent block.

The memory allocation failure case can be solved by passing in
EXT4_EX_NOFAIL to ext4_find_extent() in those cases where we can't
back out safely, and that certainly includes the this code path.

As far as the I/O failure, we shouldn't be forcing a file system error
in ext4_find_extent(), as you have in this patch:

> > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> > > index 9de1c9d1a13d..0f95e857089e 100644
> > > --- a/fs/ext4/extents.c
> > > +++ b/fs/ext4/extents.c
> > > @@ -935,6 +935,7 @@ ext4_find_extent(struct inode *inode, ext4_lblk_t block,
> > >   		bh = read_extent_tree_block(inode, path[ppos].p_idx, --i, flags);
> > >   		if (IS_ERR(bh)) {
> > > +			EXT4_ERROR_INODE(inode, "IO error reading extent block");

The reason for that is in the case where we are *not* modifying the
file system, and the I/O error is a transient one (for example, maybe
there is a network hiccup in an iSCSI or Fibre Channel attached disk)
we do *not* want to mark the file system as corrupted.

Now, if the *caller* of ext4_find_extent() is in the middle of making
a change to the file system, and we can't easily back out, at that
point, it's totaly fair to mark the file system as inconsistent.  In
the ideal world, we'd try to figure out a way to pre-read in the
necessary bloccks before starting the file system mutation, to reduce
the chances of failing in the middle of the update operation.

Of course, the world is not perfect, and case where we are splitting a
leaf node, and it turns out we need to grow the depth of the tree is a
relatively rare case, and if it turns out we have an unlucky read
operation right when this happens, if we need to stop the system by
calling EXT4_ERROR*, I'm OK with that.  But we should *only* be doing
this particular case, and not in other cases when we might be calling
ext4_find_extent() is a read-only operation (for example, while
looking up a logical to physical block assignment).  After, all the
*vast* majority of calls to ext4_find_extent() will be in read-only
contexts, and so calling EXT4_ERROR_INODE() any time
read_extent_tree_block() might fail is not appropriate.

Does that make sense to you?

Cheers,

						- Ted