* [PATCH v4 0/2] fix extents need to be restored when ext4_ext_insert_extent failed
@ 2023-02-13 4:05 zhanchengbin
2023-02-13 4:05 ` [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at zhanchengbin
2023-02-13 4:05 ` [PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error zhanchengbin
0 siblings, 2 replies; 9+ messages in thread
From: zhanchengbin @ 2023-02-13 4:05 UTC (permalink / raw)
To: tytso, jack; +Cc: linux-ext4, yi.zhang, linfeilong, liuzhiqiang26, 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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at 2023-02-13 4:05 [PATCH v4 0/2] fix extents need to be restored when ext4_ext_insert_extent failed zhanchengbin @ 2023-02-13 4:05 ` zhanchengbin 2023-02-14 11:48 ` Jan Kara 2023-02-13 4:05 ` [PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error zhanchengbin 1 sibling, 1 reply; 9+ messages in thread From: zhanchengbin @ 2023-02-13 4:05 UTC (permalink / raw) To: tytso, jack; +Cc: linux-ext4, yi.zhang, linfeilong, liuzhiqiang26, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at 2023-02-13 4:05 ` [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at zhanchengbin @ 2023-02-14 11:48 ` Jan Kara 2023-02-15 8:51 ` zhanchengbin 0 siblings, 1 reply; 9+ messages in thread From: Jan Kara @ 2023-02-14 11:48 UTC (permalink / raw) To: zhanchengbin; +Cc: tytso, jack, linux-ext4, yi.zhang, linfeilong, liuzhiqiang26 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at 2023-02-14 11:48 ` Jan Kara @ 2023-02-15 8:51 ` zhanchengbin 2023-02-16 13:07 ` Jan Kara 2023-02-19 3:35 ` Theodore Ts'o 0 siblings, 2 replies; 9+ messages in thread From: zhanchengbin @ 2023-02-15 8:51 UTC (permalink / raw) To: Jan Kara; +Cc: tytso, jack, linux-ext4, yi.zhang, linfeilong, liuzhiqiang26 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 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at 2023-02-15 8:51 ` zhanchengbin @ 2023-02-16 13:07 ` Jan Kara 2023-02-19 3:35 ` Theodore Ts'o 1 sibling, 0 replies; 9+ messages in thread From: Jan Kara @ 2023-02-16 13:07 UTC (permalink / raw) To: zhanchengbin Cc: Jan Kara, tytso, jack, linux-ext4, yi.zhang, linfeilong, liuzhiqiang26 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at 2023-02-15 8:51 ` zhanchengbin 2023-02-16 13:07 ` Jan Kara @ 2023-02-19 3:35 ` Theodore Ts'o 1 sibling, 0 replies; 9+ messages in thread From: Theodore Ts'o @ 2023-02-19 3:35 UTC (permalink / raw) To: zhanchengbin Cc: Jan Kara, jack, linux-ext4, yi.zhang, linfeilong, liuzhiqiang26 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error 2023-02-13 4:05 [PATCH v4 0/2] fix extents need to be restored when ext4_ext_insert_extent failed zhanchengbin 2023-02-13 4:05 ` [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at zhanchengbin @ 2023-02-13 4:05 ` zhanchengbin 2023-02-13 6:18 ` kernel test robot 2023-02-13 6:19 ` kernel test robot 1 sibling, 2 replies; 9+ messages in thread From: zhanchengbin @ 2023-02-13 4:05 UTC (permalink / raw) To: tytso, jack; +Cc: linux-ext4, yi.zhang, linfeilong, liuzhiqiang26, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error 2023-02-13 4:05 ` [PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error zhanchengbin @ 2023-02-13 6:18 ` kernel test robot 2023-02-13 6:19 ` kernel test robot 1 sibling, 0 replies; 9+ messages in thread From: kernel test robot @ 2023-02-13 6:18 UTC (permalink / raw) To: zhanchengbin, tytso, jack Cc: llvm, oe-kbuild-all, linux-ext4, yi.zhang, linfeilong, liuzhiqiang26, zhanchengbin 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error 2023-02-13 4:05 ` [PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error zhanchengbin 2023-02-13 6:18 ` kernel test robot @ 2023-02-13 6:19 ` kernel test robot 1 sibling, 0 replies; 9+ messages in thread From: kernel test robot @ 2023-02-13 6:19 UTC (permalink / raw) To: zhanchengbin, tytso, jack Cc: llvm, oe-kbuild-all, linux-ext4, yi.zhang, linfeilong, liuzhiqiang26, zhanchengbin 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-02-19 3:36 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-13 4:05 [PATCH v4 0/2] fix extents need to be restored when ext4_ext_insert_extent failed zhanchengbin 2023-02-13 4:05 ` [PATCH v4 1/2] ext4: fix inode tree inconsistency caused by ENOMEM in ext4_split_extent_at zhanchengbin 2023-02-14 11:48 ` Jan Kara 2023-02-15 8:51 ` zhanchengbin 2023-02-16 13:07 ` Jan Kara 2023-02-19 3:35 ` Theodore Ts'o 2023-02-13 4:05 ` [PATCH v4 2/2] ext4: clear the verified flag of the modified leaf or idx if error zhanchengbin 2023-02-13 6:18 ` kernel test robot 2023-02-13 6:19 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox