public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
From: libaokun@huaweicloud.com
To: linux-ext4@vger.kernel.org
Cc: tytso@mit.edu, adilger.kernel@dilger.ca, jack@suse.cz,
	ritesh.list@gmail.com, ojaswin@linux.ibm.com,
	linux-kernel@vger.kernel.org, yi.zhang@huawei.com,
	yangerkun@huawei.com, libaokun@huaweicloud.com,
	Baokun Li <libaokun1@huawei.com>
Subject: [PATCH v2 20/25] ext4: get rid of ppath in ext4_ext_convert_to_initialized()
Date: Thu, 22 Aug 2024 10:35:40 +0800	[thread overview]
Message-ID: <20240822023545.1994557-21-libaokun@huaweicloud.com> (raw)
In-Reply-To: <20240822023545.1994557-1-libaokun@huaweicloud.com>

From: Baokun Li <libaokun1@huawei.com>

The use of path and ppath is now very confusing, so to make the code more
readable, pass path between functions uniformly, and get rid of ppath.

To get rid of the ppath in ext4_ext_convert_to_initialized(), the following
is done here:

 * Free the extents path when an error is encountered.
 * Its caller needs to update ppath if it uses ppath.
 * The 'allocated' is changed from passing a value to passing an address.

No functional changes.

Signed-off-by: Baokun Li <libaokun1@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Tested-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 fs/ext4/extents.c | 73 +++++++++++++++++++++++------------------------
 1 file changed, 35 insertions(+), 38 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index f0c9c53452d2..efc078c2124e 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3438,13 +3438,11 @@ static struct ext4_ext_path *ext4_split_extent(handle_t *handle,
  *    that are allocated and initialized.
  *    It is guaranteed to be >= map->m_len.
  */
-static int ext4_ext_convert_to_initialized(handle_t *handle,
-					   struct inode *inode,
-					   struct ext4_map_blocks *map,
-					   struct ext4_ext_path **ppath,
-					   int flags)
+static struct ext4_ext_path *
+ext4_ext_convert_to_initialized(handle_t *handle, struct inode *inode,
+			struct ext4_map_blocks *map, struct ext4_ext_path *path,
+			int flags, unsigned int *allocated)
 {
-	struct ext4_ext_path *path = *ppath;
 	struct ext4_sb_info *sbi;
 	struct ext4_extent_header *eh;
 	struct ext4_map_blocks split_map;
@@ -3454,7 +3452,6 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 	unsigned int ee_len, depth, map_len = map->m_len;
 	int err = 0;
 	int split_flag = EXT4_EXT_DATA_VALID2;
-	int allocated = 0;
 	unsigned int max_zeroout = 0;
 
 	ext_debug(inode, "logical block %llu, max_blocks %u\n",
@@ -3495,6 +3492,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 	 *  - L2: we only attempt to merge with an extent stored in the
 	 *    same extent tree node.
 	 */
+	*allocated = 0;
 	if ((map->m_lblk == ee_block) &&
 		/* See if we can merge left */
 		(map_len < ee_len) &&		/*L1*/
@@ -3524,7 +3522,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 			(prev_len < (EXT_INIT_MAX_LEN - map_len))) {	/*C4*/
 			err = ext4_ext_get_access(handle, inode, path + depth);
 			if (err)
-				goto out;
+				goto errout;
 
 			trace_ext4_ext_convert_to_initialized_fastpath(inode,
 				map, ex, abut_ex);
@@ -3539,7 +3537,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 			abut_ex->ee_len = cpu_to_le16(prev_len + map_len);
 
 			/* Result: number of initialized blocks past m_lblk */
-			allocated = map_len;
+			*allocated = map_len;
 		}
 	} else if (((map->m_lblk + map_len) == (ee_block + ee_len)) &&
 		   (map_len < ee_len) &&	/*L1*/
@@ -3570,7 +3568,7 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 		    (next_len < (EXT_INIT_MAX_LEN - map_len))) {	/*C4*/
 			err = ext4_ext_get_access(handle, inode, path + depth);
 			if (err)
-				goto out;
+				goto errout;
 
 			trace_ext4_ext_convert_to_initialized_fastpath(inode,
 				map, ex, abut_ex);
@@ -3585,18 +3583,20 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 			abut_ex->ee_len = cpu_to_le16(next_len + map_len);
 
 			/* Result: number of initialized blocks past m_lblk */
-			allocated = map_len;
+			*allocated = map_len;
 		}
 	}
-	if (allocated) {
+	if (*allocated) {
 		/* Mark the block containing both extents as dirty */
 		err = ext4_ext_dirty(handle, inode, path + depth);
 
 		/* Update path to point to the right extent */
 		path[depth].p_ext = abut_ex;
+		if (err)
+			goto errout;
 		goto out;
 	} else
-		allocated = ee_len - (map->m_lblk - ee_block);
+		*allocated = ee_len - (map->m_lblk - ee_block);
 
 	WARN_ON(map->m_lblk < ee_block);
 	/*
@@ -3623,21 +3623,21 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 	split_map.m_lblk = map->m_lblk;
 	split_map.m_len = map->m_len;
 
-	if (max_zeroout && (allocated > split_map.m_len)) {
-		if (allocated <= max_zeroout) {
+	if (max_zeroout && (*allocated > split_map.m_len)) {
+		if (*allocated <= max_zeroout) {
 			/* case 3 or 5 */
 			zero_ex1.ee_block =
 				 cpu_to_le32(split_map.m_lblk +
 					     split_map.m_len);
 			zero_ex1.ee_len =
-				cpu_to_le16(allocated - split_map.m_len);
+				cpu_to_le16(*allocated - split_map.m_len);
 			ext4_ext_store_pblock(&zero_ex1,
 				ext4_ext_pblock(ex) + split_map.m_lblk +
 				split_map.m_len - ee_block);
 			err = ext4_ext_zeroout(inode, &zero_ex1);
 			if (err)
 				goto fallback;
-			split_map.m_len = allocated;
+			split_map.m_len = *allocated;
 		}
 		if (split_map.m_lblk - ee_block + split_map.m_len <
 								max_zeroout) {
@@ -3655,27 +3655,24 @@ static int ext4_ext_convert_to_initialized(handle_t *handle,
 
 			split_map.m_len += split_map.m_lblk - ee_block;
 			split_map.m_lblk = ee_block;
-			allocated = map->m_len;
+			*allocated = map->m_len;
 		}
 	}
 
 fallback:
 	path = ext4_split_extent(handle, inode, path, &split_map, split_flag,
 				 flags, NULL);
-	if (IS_ERR(path)) {
-		err = PTR_ERR(path);
-		*ppath = NULL;
-		goto out;
-	}
-	err = 0;
-	*ppath = path;
+	if (IS_ERR(path))
+		return path;
 out:
 	/* If we have gotten a failure, don't zero out status tree */
-	if (!err) {
-		ext4_zeroout_es(inode, &zero_ex1);
-		ext4_zeroout_es(inode, &zero_ex2);
-	}
-	return err ? err : allocated;
+	ext4_zeroout_es(inode, &zero_ex1);
+	ext4_zeroout_es(inode, &zero_ex2);
+	return path;
+
+errout:
+	ext4_free_ext_path(path);
+	return ERR_PTR(err);
 }
 
 /*
@@ -3897,7 +3894,6 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
 			struct ext4_ext_path **ppath, int flags,
 			unsigned int allocated, ext4_fsblk_t newblock)
 {
-	int ret = 0;
 	int err = 0;
 
 	ext_debug(inode, "logical block %llu, max_blocks %u, flags 0x%x, allocated %u\n",
@@ -3977,23 +3973,24 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
 	 * For buffered writes, at writepage time, etc.  Convert a
 	 * discovered unwritten extent to written.
 	 */
-	ret = ext4_ext_convert_to_initialized(handle, inode, map, ppath, flags);
-	if (ret < 0) {
-		err = ret;
+	*ppath = ext4_ext_convert_to_initialized(handle, inode, map, *ppath,
+						 flags, &allocated);
+	if (IS_ERR(*ppath)) {
+		err = PTR_ERR(*ppath);
+		*ppath = NULL;
 		goto out2;
 	}
 	ext4_update_inode_fsync_trans(handle, inode, 1);
 	/*
-	 * shouldn't get a 0 return when converting an unwritten extent
+	 * shouldn't get a 0 allocated when converting an unwritten extent
 	 * unless m_len is 0 (bug) or extent has been corrupted
 	 */
-	if (unlikely(ret == 0)) {
-		EXT4_ERROR_INODE(inode, "unexpected ret == 0, m_len = %u",
+	if (unlikely(allocated == 0)) {
+		EXT4_ERROR_INODE(inode, "unexpected allocated == 0, m_len = %u",
 				 map->m_len);
 		err = -EFSCORRUPTED;
 		goto out2;
 	}
-	allocated = ret;
 
 out:
 	map->m_flags |= EXT4_MAP_NEW;
-- 
2.39.2


  parent reply	other threads:[~2024-08-22  2:40 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-22  2:35 [PATCH v2 00/25] ext4: some bugfixes and cleanups for ext4 extents path libaokun
2024-08-22  2:35 ` [PATCH v2 01/25] ext4: refactor ext4_ext_rm_idx() to index 'path' libaokun
2024-08-22  2:35 ` [PATCH v2 02/25] ext4: prevent partial update of the extents path libaokun
2024-08-22  2:35 ` [PATCH v2 03/25] ext4: fix slab-use-after-free in ext4_split_extent_at() libaokun
2024-08-22  2:35 ` [PATCH v2 04/25] ext4: avoid use-after-free in ext4_ext_show_leaf() libaokun
2024-08-22  2:35 ` [PATCH v2 05/25] ext4: update orig_path in ext4_find_extent() libaokun
2024-08-27 18:04   ` Jan Kara
2024-08-22  2:35 ` [PATCH v2 06/25] ext4: aovid use-after-free in ext4_ext_insert_extent() libaokun
2024-08-27 18:07   ` Jan Kara
2024-08-22  2:35 ` [PATCH v2 07/25] ext4: drop ppath from ext4_ext_replay_update_ex() to avoid double-free libaokun
2024-08-22  2:35 ` [PATCH v2 08/25] ext4: fix double brelse() the buffer of the extents path libaokun
2024-08-22  2:35 ` [PATCH v2 09/25] ext4: add new ext4_ext_path_brelse() helper libaokun
2024-08-22  2:35 ` [PATCH v2 10/25] ext4: propagate errors from ext4_find_extent() in ext4_insert_range() libaokun
2024-08-22  2:35 ` [PATCH v2 11/25] ext4: get rid of ppath in ext4_find_extent() libaokun
2024-08-22  2:35 ` [PATCH v2 12/25] ext4: get rid of ppath in get_ext_path() libaokun
2024-08-22  2:35 ` [PATCH v2 13/25] ext4: get rid of ppath in ext4_ext_create_new_leaf() libaokun
2024-08-22  2:35 ` [PATCH v2 14/25] ext4: get rid of ppath in ext4_ext_insert_extent() libaokun
2024-08-22  2:35 ` [PATCH v2 15/25] ext4: get rid of ppath in ext4_split_extent_at() libaokun
2024-08-22  2:35 ` [PATCH v2 16/25] ext4: get rid of ppath in ext4_force_split_extent_at() libaokun
2024-08-22  2:35 ` [PATCH v2 17/25] ext4: get rid of ppath in ext4_split_extent() libaokun
2024-08-22  2:35 ` [PATCH v2 18/25] ext4: get rid of ppath in ext4_split_convert_extents() libaokun
2024-08-22  2:35 ` [PATCH v2 19/25] ext4: get rid of ppath in ext4_convert_unwritten_extents_endio() libaokun
2024-08-22  2:35 ` libaokun [this message]
2024-08-22  2:35 ` [PATCH v2 21/25] ext4: get rid of ppath in ext4_ext_handle_unwritten_extents() libaokun
2024-08-22  2:35 ` [PATCH v2 22/25] ext4: get rid of ppath in convert_initialized_extent() libaokun
2024-08-22  2:35 ` [PATCH v2 23/25] ext4: refactor ext4_swap_extents() to reuse extents path libaokun
2024-08-22  2:35 ` [PATCH v2 24/25] ext4: make some fast commit functions " libaokun
2024-08-22  2:35 ` [PATCH v2 25/25] ext4: save unnecessary indentation in ext4_ext_create_new_leaf() libaokun
2024-08-27 17:16   ` Jan Kara
2024-09-05 14:53 ` [PATCH v2 00/25] ext4: some bugfixes and cleanups for ext4 extents path Theodore Ts'o

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240822023545.1994557-21-libaokun@huaweicloud.com \
    --to=libaokun@huaweicloud.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=jack@suse.cz \
    --cc=libaokun1@huawei.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojaswin@linux.ibm.com \
    --cc=ritesh.list@gmail.com \
    --cc=tytso@mit.edu \
    --cc=yangerkun@huawei.com \
    --cc=yi.zhang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox