[PATCH v3 00/12] ext4: simplify the counting and management of delalloc reserved blocks

[PATCH v3 00/12] ext4: simplify the counting and management of delalloc reserved blocks

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

Changes since v2:
 - In patch 3, update the chang log as Jan suggested.
 - In patch 5 and 6, when moving reserved blocks count updating to
   ext4_es_insert_extent(), chang the condition for determining quota
   claim by passing allocation information instead of counting used
   reserved blocks as Jan suggested.
 - Add patch 9, drop an unused helper ext4_es_store_status().
 - Add patch 10, make extent status type exclusive, add assertion and
   commtents as Jan suggested.

Changes since v1:
 - Just rebase to v6.11-rc1.

This patch series is the part 3 prepartory changes of the buffered IO
iomap conversion, it simplify the counting and updating logic of
delalloc reserved blocks. I picked them out from my buffered IO iomap
conversion RFC series v4[1], and did some minor improvements of commit
messages. This series is based on 6.11-rc3, after it we could save a lot
of code.

Patch 1-3 simplify the delalloc extent management logic by changes to
always set EXT4_GET_BLOCKS_DELALLOC_RESERVE flag when allocating
preallocated blocks, and don't add EXTENT_STATUS_DELAYED flag to an
unwritten extent, which means ext4_es_is_delayed() is equal to
ext4_es_is_delonly().

Patch 4-6 simplify the reserved blocks updating logic by moves the
reserved blocks updating from ext4_{ind|ext}_map_blocks() to
ext4_es_insert_extent().

Patch 7-12 make extent status type exclusive and drop the unused code
(e.g. ext4_es_is_delonly()), update comments and do some cleanup.

[1] https://lore.kernel.org/linux-ext4/20240410142948.2817554-1-yi.zhang@huaweicloud.com/

Thanks,
Yi.

Zhang Yi (12):
  ext4: factor out ext4_map_create_blocks() to allocate new blocks
  ext4: optimize the EXT4_GET_BLOCKS_DELALLOC_RESERVE flag set
  ext4: don't set EXTENT_STATUS_DELAYED on allocated blocks
  ext4: let __revise_pending() return newly inserted pendings
  ext4: passing block allocation information to ext4_es_insert_extent()
  ext4: update delalloc data reserve spcae in ext4_es_insert_extent()
  ext4: drop ext4_es_delayed_clu()
  ext4: use ext4_map_query_blocks() in ext4_map_blocks()
  ext4: drop unused ext4_es_store_status()
  ext4: make extent status types exclusive
  ext4: drop ext4_es_is_delonly()
  ext4: drop all delonly descriptions

 fs/ext4/extents.c        |  42 +------
 fs/ext4/extents_status.c | 240 ++++++++++++++-------------------------
 fs/ext4/extents_status.h |  28 ++---
 fs/ext4/indirect.c       |   7 --
 fs/ext4/inode.c          | 201 ++++++++++++++------------------
 5 files changed, 189 insertions(+), 329 deletions(-)

-- 
2.39.2

[PATCH v3 01/12] ext4: factor out ext4_map_create_blocks() to allocate new blocks

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

Factor out a common helper ext4_map_create_blocks() from
ext4_map_blocks() to do a real blocks allocation, no logic changes.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/inode.c | 157 +++++++++++++++++++++++++-----------------------
 1 file changed, 81 insertions(+), 76 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 941c1c0d5c6e..112aec171ee9 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -482,6 +482,86 @@ static int ext4_map_query_blocks(handle_t *handle, struct inode *inode,
 	return retval;
 }
 
+static int ext4_map_create_blocks(handle_t *handle, struct inode *inode,
+				  struct ext4_map_blocks *map, int flags)
+{
+	struct extent_status es;
+	unsigned int status;
+	int err, retval = 0;
+
+	/*
+	 * Here we clear m_flags because after allocating an new extent,
+	 * it will be set again.
+	 */
+	map->m_flags &= ~EXT4_MAP_FLAGS;
+
+	/*
+	 * We need to check for EXT4 here because migrate could have
+	 * changed the inode type in between.
+	 */
+	if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
+		retval = ext4_ext_map_blocks(handle, inode, map, flags);
+	} else {
+		retval = ext4_ind_map_blocks(handle, inode, map, flags);
+
+		/*
+		 * We allocated new blocks which will result in i_data's
+		 * format changing. Force the migrate to fail by clearing
+		 * migrate flags.
+		 */
+		if (retval > 0 && map->m_flags & EXT4_MAP_NEW)
+			ext4_clear_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
+	}
+	if (retval <= 0)
+		return retval;
+
+	if (unlikely(retval != map->m_len)) {
+		ext4_warning(inode->i_sb,
+			     "ES len assertion failed for inode %lu: "
+			     "retval %d != map->m_len %d",
+			     inode->i_ino, retval, map->m_len);
+		WARN_ON(1);
+	}
+
+	/*
+	 * We have to zeroout blocks before inserting them into extent
+	 * status tree. Otherwise someone could look them up there and
+	 * use them before they are really zeroed. We also have to
+	 * unmap metadata before zeroing as otherwise writeback can
+	 * overwrite zeros with stale data from block device.
+	 */
+	if (flags & EXT4_GET_BLOCKS_ZERO &&
+	    map->m_flags & EXT4_MAP_MAPPED && map->m_flags & EXT4_MAP_NEW) {
+		err = ext4_issue_zeroout(inode, map->m_lblk, map->m_pblk,
+					 map->m_len);
+		if (err)
+			return err;
+	}
+
+	/*
+	 * If the extent has been zeroed out, we don't need to update
+	 * extent status tree.
+	 */
+	if (flags & EXT4_GET_BLOCKS_PRE_IO &&
+	    ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es)) {
+		if (ext4_es_is_written(&es))
+			return retval;
+	}
+
+	status = map->m_flags & EXT4_MAP_UNWRITTEN ?
+			EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
+	if (!(flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE) &&
+	    !(status & EXTENT_STATUS_WRITTEN) &&
+	    ext4_es_scan_range(inode, &ext4_es_is_delayed, map->m_lblk,
+			       map->m_lblk + map->m_len - 1))
+		status |= EXTENT_STATUS_DELAYED;
+
+	ext4_es_insert_extent(inode, map->m_lblk, map->m_len,
+			      map->m_pblk, status);
+
+	return retval;
+}
+
 /*
  * The ext4_map_blocks() function tries to look up the requested blocks,
  * and returns if the blocks are already mapped.
@@ -630,12 +710,6 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
 		if (!(flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN))
 			return retval;
 
-	/*
-	 * Here we clear m_flags because after allocating an new extent,
-	 * it will be set again.
-	 */
-	map->m_flags &= ~EXT4_MAP_FLAGS;
-
 	/*
 	 * New blocks allocate and/or writing to unwritten extent
 	 * will possibly result in updating i_data, so we take
@@ -643,76 +717,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
 	 * with create == 1 flag.
 	 */
 	down_write(&EXT4_I(inode)->i_data_sem);
-
-	/*
-	 * We need to check for EXT4 here because migrate
-	 * could have changed the inode type in between
-	 */
-	if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
-		retval = ext4_ext_map_blocks(handle, inode, map, flags);
-	} else {
-		retval = ext4_ind_map_blocks(handle, inode, map, flags);
-
-		if (retval > 0 && map->m_flags & EXT4_MAP_NEW) {
-			/*
-			 * We allocated new blocks which will result in
-			 * i_data's format changing.  Force the migrate
-			 * to fail by clearing migrate flags
-			 */
-			ext4_clear_inode_state(inode, EXT4_STATE_EXT_MIGRATE);
-		}
-	}
-
-	if (retval > 0) {
-		unsigned int status;
-
-		if (unlikely(retval != map->m_len)) {
-			ext4_warning(inode->i_sb,
-				     "ES len assertion failed for inode "
-				     "%lu: retval %d != map->m_len %d",
-				     inode->i_ino, retval, map->m_len);
-			WARN_ON(1);
-		}
-
-		/*
-		 * We have to zeroout blocks before inserting them into extent
-		 * status tree. Otherwise someone could look them up there and
-		 * use them before they are really zeroed. We also have to
-		 * unmap metadata before zeroing as otherwise writeback can
-		 * overwrite zeros with stale data from block device.
-		 */
-		if (flags & EXT4_GET_BLOCKS_ZERO &&
-		    map->m_flags & EXT4_MAP_MAPPED &&
-		    map->m_flags & EXT4_MAP_NEW) {
-			ret = ext4_issue_zeroout(inode, map->m_lblk,
-						 map->m_pblk, map->m_len);
-			if (ret) {
-				retval = ret;
-				goto out_sem;
-			}
-		}
-
-		/*
-		 * If the extent has been zeroed out, we don't need to update
-		 * extent status tree.
-		 */
-		if ((flags & EXT4_GET_BLOCKS_PRE_IO) &&
-		    ext4_es_lookup_extent(inode, map->m_lblk, NULL, &es)) {
-			if (ext4_es_is_written(&es))
-				goto out_sem;
-		}
-		status = map->m_flags & EXT4_MAP_UNWRITTEN ?
-				EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
-		if (!(flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE) &&
-		    !(status & EXTENT_STATUS_WRITTEN) &&
-		    ext4_es_scan_range(inode, &ext4_es_is_delayed, map->m_lblk,
-				       map->m_lblk + map->m_len - 1))
-			status |= EXTENT_STATUS_DELAYED;
-		ext4_es_insert_extent(inode, map->m_lblk, map->m_len,
-				      map->m_pblk, status);
-	}
-
-out_sem:
+	retval = ext4_map_create_blocks(handle, inode, map, flags);
 	up_write((&EXT4_I(inode)->i_data_sem));
 	if (retval > 0 && map->m_flags & EXT4_MAP_MAPPED) {
 		ret = check_block_validity(inode, map);
-- 
2.39.2

[PATCH v3 02/12] ext4: optimize the EXT4_GET_BLOCKS_DELALLOC_RESERVE flag set

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

When doing block allocation, magic EXT4_GET_BLOCKS_DELALLOC_RESERVE
means the allocating range covers a range of delayed allocated clusters,
the blocks and quotas have already been reserved in ext4_da_map_blocks(),
we should update the reserved space and don't need to claim them again.

At the moment, we only set this magic in mpage_map_one_extent() when
allocating a range of delayed allocated clusters in the write back path,
it makes things complicated since we have to notice and deal with the
case of allocating non-delayed allocated clusters separately in
ext4_ext_map_blocks(). For example, it we fallocate some blocks that
have been delayed allocated, free space would be claimed again in
ext4_mb_new_blocks() (this is wrong exactily), and we can't claim quota
space again, we have to release the quota reservations made for that
previously delayed allocated clusters.

Move the position thats set the EXT4_GET_BLOCKS_DELALLOC_RESERVE to
where we actually do block allocation, it could simplify above handling
a lot, it means that we always set this magic once the allocation range
covers delalloc blocks, no need to take care of the allocation path.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/inode.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 112aec171ee9..91b2610a6dc5 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -489,6 +489,14 @@ static int ext4_map_create_blocks(handle_t *handle, struct inode *inode,
 	unsigned int status;
 	int err, retval = 0;
 
+	/*
+	 * We pass in the magic EXT4_GET_BLOCKS_DELALLOC_RESERVE
+	 * indicates that the blocks and quotas has already been
+	 * checked when the data was copied into the page cache.
+	 */
+	if (map->m_flags & EXT4_MAP_DELAYED)
+		flags |= EXT4_GET_BLOCKS_DELALLOC_RESERVE;
+
 	/*
 	 * Here we clear m_flags because after allocating an new extent,
 	 * it will be set again.
@@ -2224,11 +2232,6 @@ static int mpage_map_one_extent(handle_t *handle, struct mpage_da_data *mpd)
 	 * writeback and there is nothing we can do about it so it might result
 	 * in data loss.  So use reserved blocks to allocate metadata if
 	 * possible.
-	 *
-	 * We pass in the magic EXT4_GET_BLOCKS_DELALLOC_RESERVE if
-	 * the blocks in question are delalloc blocks.  This indicates
-	 * that the blocks and quotas has already been checked when
-	 * the data was copied into the page cache.
 	 */
 	get_blocks_flags = EXT4_GET_BLOCKS_CREATE |
 			   EXT4_GET_BLOCKS_METADATA_NOFAIL |
@@ -2236,8 +2239,6 @@ static int mpage_map_one_extent(handle_t *handle, struct mpage_da_data *mpd)
 	dioread_nolock = ext4_should_dioread_nolock(inode);
 	if (dioread_nolock)
 		get_blocks_flags |= EXT4_GET_BLOCKS_IO_CREATE_EXT;
-	if (map->m_flags & BIT(BH_Delay))
-		get_blocks_flags |= EXT4_GET_BLOCKS_DELALLOC_RESERVE;
 
 	err = ext4_map_blocks(handle, inode, map, get_blocks_flags);
 	if (err < 0)
-- 
2.39.2

[PATCH v3 03/12] ext4: don't set EXTENT_STATUS_DELAYED on allocated blocks

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

Currently, we release delayed allocation reservation when removing
delayed extent from extent status tree (which also happens when
overwriting one extent with another one). When we allocated unwritten
extent under some delayed allocated extent, we don't need the
reservation anymore and hence we don't need to preserve the
EXT4_MAP_DELAYED status bit. Allocating the new extent blocks will
properly release the reservation.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/extents_status.c |  9 +--------
 fs/ext4/inode.c          | 11 -----------
 2 files changed, 1 insertion(+), 19 deletions(-)

diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 17dcf13adde2..024cd37d53b3 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -869,14 +869,7 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 		return;
 
 	BUG_ON(end < lblk);
-
-	if ((status & EXTENT_STATUS_DELAYED) &&
-	    (status & EXTENT_STATUS_WRITTEN)) {
-		ext4_warning(inode->i_sb, "Inserting extent [%u/%u] as "
-				" delayed and written which can potentially "
-				" cause data loss.", lblk, len);
-		WARN_ON(1);
-	}
+	WARN_ON_ONCE(status & EXTENT_STATUS_DELAYED);
 
 	newes.es_lblk = lblk;
 	newes.es_len = len;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 91b2610a6dc5..e9ce1e4e6acb 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -558,12 +558,6 @@ static int ext4_map_create_blocks(handle_t *handle, struct inode *inode,
 
 	status = map->m_flags & EXT4_MAP_UNWRITTEN ?
 			EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
-	if (!(flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE) &&
-	    !(status & EXTENT_STATUS_WRITTEN) &&
-	    ext4_es_scan_range(inode, &ext4_es_is_delayed, map->m_lblk,
-			       map->m_lblk + map->m_len - 1))
-		status |= EXTENT_STATUS_DELAYED;
-
 	ext4_es_insert_extent(inode, map->m_lblk, map->m_len,
 			      map->m_pblk, status);
 
@@ -682,11 +676,6 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
 
 		status = map->m_flags & EXT4_MAP_UNWRITTEN ?
 				EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
-		if (!(flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE) &&
-		    !(status & EXTENT_STATUS_WRITTEN) &&
-		    ext4_es_scan_range(inode, &ext4_es_is_delayed, map->m_lblk,
-				       map->m_lblk + map->m_len - 1))
-			status |= EXTENT_STATUS_DELAYED;
 		ext4_es_insert_extent(inode, map->m_lblk, map->m_len,
 				      map->m_pblk, status);
 	}
-- 
2.39.2

[PATCH v3 04/12] ext4: let __revise_pending() return newly inserted pendings

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

Let __insert_pending() return 1 after successfully inserting a new
pending cluster, and also let __revise_pending() to return the number of
of newly inserted pendings.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 fs/ext4/extents_status.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 024cd37d53b3..4d24b56cfaf0 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -887,7 +887,7 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 		es1 = __es_alloc_extent(true);
 	if ((err1 || err2) && !es2)
 		es2 = __es_alloc_extent(true);
-	if ((err1 || err2 || err3) && revise_pending && !pr)
+	if ((err1 || err2 || err3 < 0) && revise_pending && !pr)
 		pr = __alloc_pending(true);
 	write_lock(&EXT4_I(inode)->i_es_lock);
 
@@ -915,7 +915,7 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 
 	if (revise_pending) {
 		err3 = __revise_pending(inode, lblk, len, &pr);
-		if (err3 != 0)
+		if (err3 < 0)
 			goto error;
 		if (pr) {
 			__free_pending(pr);
@@ -924,7 +924,7 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 	}
 error:
 	write_unlock(&EXT4_I(inode)->i_es_lock);
-	if (err1 || err2 || err3)
+	if (err1 || err2 || err3 < 0)
 		goto retry;
 
 	ext4_es_print_tree(inode);
@@ -1933,7 +1933,7 @@ static struct pending_reservation *__get_pending(struct inode *inode,
  * @lblk - logical block in the cluster to be added
  * @prealloc - preallocated pending entry
  *
- * Returns 0 on successful insertion and -ENOMEM on failure.  If the
+ * Returns 1 on successful insertion and -ENOMEM on failure.  If the
  * pending reservation is already in the set, returns successfully.
  */
 static int __insert_pending(struct inode *inode, ext4_lblk_t lblk,
@@ -1977,6 +1977,7 @@ static int __insert_pending(struct inode *inode, ext4_lblk_t lblk,
 
 	rb_link_node(&pr->rb_node, parent, p);
 	rb_insert_color(&pr->rb_node, &tree->root);
+	ret = 1;
 
 out:
 	return ret;
@@ -2098,7 +2099,7 @@ void ext4_es_insert_delayed_extent(struct inode *inode, ext4_lblk_t lblk,
 		es1 = __es_alloc_extent(true);
 	if ((err1 || err2) && !es2)
 		es2 = __es_alloc_extent(true);
-	if (err1 || err2 || err3) {
+	if (err1 || err2 || err3 < 0) {
 		if (lclu_allocated && !pr1)
 			pr1 = __alloc_pending(true);
 		if (end_allocated && !pr2)
@@ -2128,7 +2129,7 @@ void ext4_es_insert_delayed_extent(struct inode *inode, ext4_lblk_t lblk,
 
 	if (lclu_allocated) {
 		err3 = __insert_pending(inode, lblk, &pr1);
-		if (err3 != 0)
+		if (err3 < 0)
 			goto error;
 		if (pr1) {
 			__free_pending(pr1);
@@ -2137,7 +2138,7 @@ void ext4_es_insert_delayed_extent(struct inode *inode, ext4_lblk_t lblk,
 	}
 	if (end_allocated) {
 		err3 = __insert_pending(inode, end, &pr2);
-		if (err3 != 0)
+		if (err3 < 0)
 			goto error;
 		if (pr2) {
 			__free_pending(pr2);
@@ -2146,7 +2147,7 @@ void ext4_es_insert_delayed_extent(struct inode *inode, ext4_lblk_t lblk,
 	}
 error:
 	write_unlock(&EXT4_I(inode)->i_es_lock);
-	if (err1 || err2 || err3)
+	if (err1 || err2 || err3 < 0)
 		goto retry;
 
 	ext4_es_print_tree(inode);
@@ -2256,7 +2257,9 @@ unsigned int ext4_es_delayed_clu(struct inode *inode, ext4_lblk_t lblk,
  *
  * Used after a newly allocated extent is added to the extents status tree.
  * Requires that the extents in the range have either written or unwritten
- * status.  Must be called while holding i_es_lock.
+ * status.  Must be called while holding i_es_lock. Returns number of new
+ * inserts pending cluster on insert pendings, returns 0 on remove pendings,
+ * return -ENOMEM on failure.
  */
 static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
 			    ext4_lblk_t len,
@@ -2266,6 +2269,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
 	ext4_lblk_t end = lblk + len - 1;
 	ext4_lblk_t first, last;
 	bool f_del = false, l_del = false;
+	int pendings = 0;
 	int ret = 0;
 
 	if (len == 0)
@@ -2293,6 +2297,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
 			ret = __insert_pending(inode, first, prealloc);
 			if (ret < 0)
 				goto out;
+			pendings += ret;
 		} else {
 			last = EXT4_LBLK_CMASK(sbi, end) +
 			       sbi->s_cluster_ratio - 1;
@@ -2304,6 +2309,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
 				ret = __insert_pending(inode, last, prealloc);
 				if (ret < 0)
 					goto out;
+				pendings += ret;
 			} else
 				__remove_pending(inode, last);
 		}
@@ -2316,6 +2322,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
 			ret = __insert_pending(inode, first, prealloc);
 			if (ret < 0)
 				goto out;
+			pendings += ret;
 		} else
 			__remove_pending(inode, first);
 
@@ -2327,9 +2334,10 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
 			ret = __insert_pending(inode, last, prealloc);
 			if (ret < 0)
 				goto out;
+			pendings += ret;
 		} else
 			__remove_pending(inode, last);
 	}
 out:
-	return ret;
+	return (ret < 0) ? ret : pendings;
 }
-- 
2.39.2

Re: [PATCH v3 04/12] ext4: let __revise_pending() return newly inserted pendings

[Jan Kara]

On Tue 13-08-24 20:34:44, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> Let __insert_pending() return 1 after successfully inserting a new
> pending cluster, and also let __revise_pending() to return the number of
> of newly inserted pendings.
> 
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

AFAICS nothing really uses this functionality in this version of the
patchset so we can drop this patch?

								Honza

> ---
>  fs/ext4/extents_status.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 024cd37d53b3..4d24b56cfaf0 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -887,7 +887,7 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>  		es1 = __es_alloc_extent(true);
>  	if ((err1 || err2) && !es2)
>  		es2 = __es_alloc_extent(true);
> -	if ((err1 || err2 || err3) && revise_pending && !pr)
> +	if ((err1 || err2 || err3 < 0) && revise_pending && !pr)
>  		pr = __alloc_pending(true);
>  	write_lock(&EXT4_I(inode)->i_es_lock);
>  
> @@ -915,7 +915,7 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>  
>  	if (revise_pending) {
>  		err3 = __revise_pending(inode, lblk, len, &pr);
> -		if (err3 != 0)
> +		if (err3 < 0)
>  			goto error;
>  		if (pr) {
>  			__free_pending(pr);
> @@ -924,7 +924,7 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>  	}
>  error:
>  	write_unlock(&EXT4_I(inode)->i_es_lock);
> -	if (err1 || err2 || err3)
> +	if (err1 || err2 || err3 < 0)
>  		goto retry;
>  
>  	ext4_es_print_tree(inode);
> @@ -1933,7 +1933,7 @@ static struct pending_reservation *__get_pending(struct inode *inode,
>   * @lblk - logical block in the cluster to be added
>   * @prealloc - preallocated pending entry
>   *
> - * Returns 0 on successful insertion and -ENOMEM on failure.  If the
> + * Returns 1 on successful insertion and -ENOMEM on failure.  If the
>   * pending reservation is already in the set, returns successfully.
>   */
>  static int __insert_pending(struct inode *inode, ext4_lblk_t lblk,
> @@ -1977,6 +1977,7 @@ static int __insert_pending(struct inode *inode, ext4_lblk_t lblk,
>  
>  	rb_link_node(&pr->rb_node, parent, p);
>  	rb_insert_color(&pr->rb_node, &tree->root);
> +	ret = 1;
>  
>  out:
>  	return ret;
> @@ -2098,7 +2099,7 @@ void ext4_es_insert_delayed_extent(struct inode *inode, ext4_lblk_t lblk,
>  		es1 = __es_alloc_extent(true);
>  	if ((err1 || err2) && !es2)
>  		es2 = __es_alloc_extent(true);
> -	if (err1 || err2 || err3) {
> +	if (err1 || err2 || err3 < 0) {
>  		if (lclu_allocated && !pr1)
>  			pr1 = __alloc_pending(true);
>  		if (end_allocated && !pr2)
> @@ -2128,7 +2129,7 @@ void ext4_es_insert_delayed_extent(struct inode *inode, ext4_lblk_t lblk,
>  
>  	if (lclu_allocated) {
>  		err3 = __insert_pending(inode, lblk, &pr1);
> -		if (err3 != 0)
> +		if (err3 < 0)
>  			goto error;
>  		if (pr1) {
>  			__free_pending(pr1);
> @@ -2137,7 +2138,7 @@ void ext4_es_insert_delayed_extent(struct inode *inode, ext4_lblk_t lblk,
>  	}
>  	if (end_allocated) {
>  		err3 = __insert_pending(inode, end, &pr2);
> -		if (err3 != 0)
> +		if (err3 < 0)
>  			goto error;
>  		if (pr2) {
>  			__free_pending(pr2);
> @@ -2146,7 +2147,7 @@ void ext4_es_insert_delayed_extent(struct inode *inode, ext4_lblk_t lblk,
>  	}
>  error:
>  	write_unlock(&EXT4_I(inode)->i_es_lock);
> -	if (err1 || err2 || err3)
> +	if (err1 || err2 || err3 < 0)
>  		goto retry;
>  
>  	ext4_es_print_tree(inode);
> @@ -2256,7 +2257,9 @@ unsigned int ext4_es_delayed_clu(struct inode *inode, ext4_lblk_t lblk,
>   *
>   * Used after a newly allocated extent is added to the extents status tree.
>   * Requires that the extents in the range have either written or unwritten
> - * status.  Must be called while holding i_es_lock.
> + * status.  Must be called while holding i_es_lock. Returns number of new
> + * inserts pending cluster on insert pendings, returns 0 on remove pendings,
> + * return -ENOMEM on failure.
>   */
>  static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
>  			    ext4_lblk_t len,
> @@ -2266,6 +2269,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
>  	ext4_lblk_t end = lblk + len - 1;
>  	ext4_lblk_t first, last;
>  	bool f_del = false, l_del = false;
> +	int pendings = 0;
>  	int ret = 0;
>  
>  	if (len == 0)
> @@ -2293,6 +2297,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
>  			ret = __insert_pending(inode, first, prealloc);
>  			if (ret < 0)
>  				goto out;
> +			pendings += ret;
>  		} else {
>  			last = EXT4_LBLK_CMASK(sbi, end) +
>  			       sbi->s_cluster_ratio - 1;
> @@ -2304,6 +2309,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
>  				ret = __insert_pending(inode, last, prealloc);
>  				if (ret < 0)
>  					goto out;
> +				pendings += ret;
>  			} else
>  				__remove_pending(inode, last);
>  		}
> @@ -2316,6 +2322,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
>  			ret = __insert_pending(inode, first, prealloc);
>  			if (ret < 0)
>  				goto out;
> +			pendings += ret;
>  		} else
>  			__remove_pending(inode, first);
>  
> @@ -2327,9 +2334,10 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
>  			ret = __insert_pending(inode, last, prealloc);
>  			if (ret < 0)
>  				goto out;
> +			pendings += ret;
>  		} else
>  			__remove_pending(inode, last);
>  	}
>  out:
> -	return ret;
> +	return (ret < 0) ? ret : pendings;
>  }
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v3 04/12] ext4: let __revise_pending() return newly inserted pendings

[Jan Kara]

On Wed 04-09-24 12:15:15, Jan Kara wrote:
> On Tue 13-08-24 20:34:44, Zhang Yi wrote:
> > From: Zhang Yi <yi.zhang@huawei.com>
> > 
> > Let __insert_pending() return 1 after successfully inserting a new
> > pending cluster, and also let __revise_pending() to return the number of
> > of newly inserted pendings.
> > 
> > Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> 
> AFAICS nothing really uses this functionality in this version of the
> patchset so we can drop this patch?

Now I've realized patch 6 indeed uses the new return value in one place.
This patch looks good so feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

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

[PATCH v3 05/12] ext4: passing block allocation information to ext4_es_insert_extent()

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

Just pass the block allocation flag to ext4_es_insert_extent() when we
replacing a current extent after an actually block allocation or extent
status conversion, this flag will be used by later changes.

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 fs/ext4/extents.c        | 5 +++--
 fs/ext4/extents_status.c | 6 +++---
 fs/ext4/extents_status.h | 2 +-
 fs/ext4/inode.c          | 8 ++++----
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index e067f2dd0335..671dacd7c873 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3113,7 +3113,7 @@ static void ext4_zeroout_es(struct inode *inode, struct ext4_extent *ex)
 		return;
 
 	ext4_es_insert_extent(inode, ee_block, ee_len, ee_pblock,
-			      EXTENT_STATUS_WRITTEN);
+			      EXTENT_STATUS_WRITTEN, 0);
 }
 
 /* FIXME!! we need to try to merge to left or right after zero-out  */
@@ -4097,7 +4097,8 @@ static ext4_lblk_t ext4_ext_determine_insert_hole(struct inode *inode,
 insert_hole:
 	/* Put just found gap into cache to speed up subsequent requests */
 	ext_debug(inode, " -> %u:%u\n", hole_start, len);
-	ext4_es_insert_extent(inode, hole_start, len, ~0, EXTENT_STATUS_HOLE);
+	ext4_es_insert_extent(inode, hole_start, len, ~0,
+			      EXTENT_STATUS_HOLE, 0);
 
 	/* Update hole_len to reflect hole size after lblk */
 	if (hole_start != lblk)
diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 4d24b56cfaf0..0580bc4bc762 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -848,7 +848,7 @@ static int __es_insert_extent(struct inode *inode, struct extent_status *newes,
  */
 void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 			   ext4_lblk_t len, ext4_fsblk_t pblk,
-			   unsigned int status)
+			   unsigned int status, int flags)
 {
 	struct extent_status newes;
 	ext4_lblk_t end = lblk + len - 1;
@@ -862,8 +862,8 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 	if (EXT4_SB(inode->i_sb)->s_mount_state & EXT4_FC_REPLAY)
 		return;
 
-	es_debug("add [%u/%u) %llu %x to extent status tree of inode %lu\n",
-		 lblk, len, pblk, status, inode->i_ino);
+	es_debug("add [%u/%u) %llu %x %x to extent status tree of inode %lu\n",
+		 lblk, len, pblk, status, flags, inode->i_ino);
 
 	if (!len)
 		return;
diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
index 3c8e2edee5d5..b74d693c1adb 100644
--- a/fs/ext4/extents_status.h
+++ b/fs/ext4/extents_status.h
@@ -129,7 +129,7 @@ extern void ext4_es_init_tree(struct ext4_es_tree *tree);
 
 extern void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 				  ext4_lblk_t len, ext4_fsblk_t pblk,
-				  unsigned int status);
+				  unsigned int status, int flags);
 extern void ext4_es_cache_extent(struct inode *inode, ext4_lblk_t lblk,
 				 ext4_lblk_t len, ext4_fsblk_t pblk,
 				 unsigned int status);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index e9ce1e4e6acb..260a38453604 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -478,7 +478,7 @@ static int ext4_map_query_blocks(handle_t *handle, struct inode *inode,
 	status = map->m_flags & EXT4_MAP_UNWRITTEN ?
 			EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
 	ext4_es_insert_extent(inode, map->m_lblk, map->m_len,
-			      map->m_pblk, status);
+			      map->m_pblk, status, 0);
 	return retval;
 }
 
@@ -559,7 +559,7 @@ static int ext4_map_create_blocks(handle_t *handle, struct inode *inode,
 	status = map->m_flags & EXT4_MAP_UNWRITTEN ?
 			EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
 	ext4_es_insert_extent(inode, map->m_lblk, map->m_len,
-			      map->m_pblk, status);
+			      map->m_pblk, status, flags);
 
 	return retval;
 }
@@ -677,7 +677,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
 		status = map->m_flags & EXT4_MAP_UNWRITTEN ?
 				EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
 		ext4_es_insert_extent(inode, map->m_lblk, map->m_len,
-				      map->m_pblk, status);
+				      map->m_pblk, status, 0);
 	}
 	up_read((&EXT4_I(inode)->i_data_sem));
 
@@ -4065,7 +4065,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
 						    stop_block);
 
 		ext4_es_insert_extent(inode, first_block, hole_len, ~0,
-				      EXTENT_STATUS_HOLE);
+				      EXTENT_STATUS_HOLE, 0);
 		up_write(&EXT4_I(inode)->i_data_sem);
 	}
 	ext4_fc_track_range(handle, inode, first_block, stop_block);
-- 
2.39.2

Re: [PATCH v3 05/12] ext4: passing block allocation information to ext4_es_insert_extent()

[Jan Kara]

On Tue 13-08-24 20:34:45, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> Just pass the block allocation flag to ext4_es_insert_extent() when we
> replacing a current extent after an actually block allocation or extent
> status conversion, this flag will be used by later changes.
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Looks good. Just one suggestion below. With that feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

> @@ -848,7 +848,7 @@ static int __es_insert_extent(struct inode *inode, struct extent_status *newes,
>   */
>  void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>  			   ext4_lblk_t len, ext4_fsblk_t pblk,
> -			   unsigned int status)
> +			   unsigned int status, int flags)

Since you pass flags to ext4_es_insert_extent() only from one place, let's
not pretend these are always full mapping flags and just make this new
argument:

bool delalloc_reserve_used

and from ext4_map_blocks_create() you can pass flags &
EXT4_GET_BLOCKS_DELALLOC_RESERVE.

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

Re: [PATCH v3 05/12] ext4: passing block allocation information to ext4_es_insert_extent()

[Zhang Yi]

On 2024/9/4 18:21, Jan Kara wrote:
> On Tue 13-08-24 20:34:45, Zhang Yi wrote:
>> From: Zhang Yi <yi.zhang@huawei.com>
>>
>> Just pass the block allocation flag to ext4_es_insert_extent() when we
>> replacing a current extent after an actually block allocation or extent
>> status conversion, this flag will be used by later changes.
>>
>> Suggested-by: Jan Kara <jack@suse.cz>
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> 
> Looks good. Just one suggestion below. With that feel free to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>
> 
>> @@ -848,7 +848,7 @@ static int __es_insert_extent(struct inode *inode, struct extent_status *newes,
>>   */
>>  void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>>  			   ext4_lblk_t len, ext4_fsblk_t pblk,
>> -			   unsigned int status)
>> +			   unsigned int status, int flags)
> 
> Since you pass flags to ext4_es_insert_extent() only from one place, let's
> not pretend these are always full mapping flags and just make this new
> argument:
> 
> bool delalloc_reserve_used
> 
> and from ext4_map_blocks_create() you can pass flags &
> EXT4_GET_BLOCKS_DELALLOC_RESERVE.
> 

Sure, it's a better idea than passing full mapping flags, thanks for your
suggestion, but since I've noticed that Ted had already picked this series
into his dev branch, I can send another patch to do this. Thanks a lot for
reviewing this series again!

Yi.

[PATCH v3 06/12] ext4: update delalloc data reserve spcae in ext4_es_insert_extent()

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

Now that we update data reserved space for delalloc after allocating
new blocks in ext4_{ind|ext}_map_blocks(), and if bigalloc feature is
enabled, we also need to query the extents_status tree to calculate the
exact reserved clusters. This is complicated now and it appears that
it's better to do this job in ext4_es_insert_extent(), because
__es_remove_extent() have already count delalloc blocks when removing
delalloc extents and __revise_pending() return new adding pending count,
we could update the reserved blocks easily in ext4_es_insert_extent().

We direct reduce the reserved cluster count when replacing a delalloc
extent. However, thers are two special cases need to concern about the
quota claiming when doing direct block allocation (e.g. from fallocate).

A),
fallocate a range that covers a delalloc extent but start with
non-delayed allocated blocks, e.g. a hole.

  hhhhhhh+ddddddd+ddddddd
  ^^^^^^^^^^^^^^^^^^^^^^^  fallocate this range

Current ext4_map_blocks() can't always trim the extent since it may
release i_data_sem before calling ext4_map_create_blocks() and raced by
another delayed allocation. Hence the EXT4_GET_BLOCKS_DELALLOC_RESERVE
may not set even when we are replacing a delalloc extent, without this
flag set, the quota has already been claimed by ext4_mb_new_blocks(), so
we should release the quota reservations instead of claim them again.

B),
bigalloc feature is enabled, fallocate a range that contains non-delayed
allocated blocks.

  |<         one cluster       >|
  hhhhhhh+hhhhhhh+hhhhhhh+ddddddd
  ^^^^^^^  fallocate this range

This case is similar to above case, the EXT4_GET_BLOCKS_DELALLOC_RESERVE
flag is also not set.

Hence we should release the quota reservations if we replace a delalloc
extent but without EXT4_GET_BLOCKS_DELALLOC_RESERVE set.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 fs/ext4/extents.c        | 37 -------------------------------------
 fs/ext4/extents_status.c | 25 ++++++++++++++++++++++++-
 fs/ext4/indirect.c       |  7 -------
 3 files changed, 24 insertions(+), 45 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 671dacd7c873..db8f9d79477c 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4357,43 +4357,6 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 		goto out;
 	}
 
-	/*
-	 * Reduce the reserved cluster count to reflect successful deferred
-	 * allocation of delayed allocated clusters or direct allocation of
-	 * clusters discovered to be delayed allocated.  Once allocated, a
-	 * cluster is not included in the reserved count.
-	 */
-	if (test_opt(inode->i_sb, DELALLOC) && allocated_clusters) {
-		if (flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE) {
-			/*
-			 * When allocating delayed allocated clusters, simply
-			 * reduce the reserved cluster count and claim quota
-			 */
-			ext4_da_update_reserve_space(inode, allocated_clusters,
-							1);
-		} else {
-			ext4_lblk_t lblk, len;
-			unsigned int n;
-
-			/*
-			 * When allocating non-delayed allocated clusters
-			 * (from fallocate, filemap, DIO, or clusters
-			 * allocated when delalloc has been disabled by
-			 * ext4_nonda_switch), reduce the reserved cluster
-			 * count by the number of allocated clusters that
-			 * have previously been delayed allocated.  Quota
-			 * has been claimed by ext4_mb_new_blocks() above,
-			 * so release the quota reservations made for any
-			 * previously delayed allocated clusters.
-			 */
-			lblk = EXT4_LBLK_CMASK(sbi, map->m_lblk);
-			len = allocated_clusters << sbi->s_cluster_bits;
-			n = ext4_es_delayed_clu(inode, lblk, len);
-			if (n > 0)
-				ext4_da_update_reserve_space(inode, (int) n, 0);
-		}
-	}
-
 	/*
 	 * Cache the extent and update transaction to commit on fdatasync only
 	 * when it is _not_ an unwritten extent.
diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 0580bc4bc762..41adf0d69959 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -853,6 +853,7 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 	struct extent_status newes;
 	ext4_lblk_t end = lblk + len - 1;
 	int err1 = 0, err2 = 0, err3 = 0;
+	int resv_used = 0, pending = 0;
 	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
 	struct extent_status *es1 = NULL;
 	struct extent_status *es2 = NULL;
@@ -891,7 +892,7 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 		pr = __alloc_pending(true);
 	write_lock(&EXT4_I(inode)->i_es_lock);
 
-	err1 = __es_remove_extent(inode, lblk, end, NULL, es1);
+	err1 = __es_remove_extent(inode, lblk, end, &resv_used, es1);
 	if (err1 != 0)
 		goto error;
 	/* Free preallocated extent if it didn't get used. */
@@ -921,9 +922,31 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
 			__free_pending(pr);
 			pr = NULL;
 		}
+		pending = err3;
 	}
 error:
 	write_unlock(&EXT4_I(inode)->i_es_lock);
+	/*
+	 * Reduce the reserved cluster count to reflect successful deferred
+	 * allocation of delayed allocated clusters or direct allocation of
+	 * clusters discovered to be delayed allocated.  Once allocated, a
+	 * cluster is not included in the reserved count.
+	 *
+	 * When direct allocating (from fallocate, filemap, DIO, or clusters
+	 * allocated when delalloc has been disabled by ext4_nonda_switch())
+	 * an extent either 1) contains delayed blocks but start with
+	 * non-delayed allocated blocks (e.g. hole) or 2) contains non-delayed
+	 * allocated blocks which belong to delayed allocated clusters when
+	 * bigalloc feature is enabled, quota has already been claimed by
+	 * ext4_mb_new_blocks(), so release the quota reservations made for
+	 * any previously delayed allocated clusters instead of claim them
+	 * again.
+	 */
+	resv_used += pending;
+	if (resv_used)
+		ext4_da_update_reserve_space(inode, resv_used,
+				flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE);
+
 	if (err1 || err2 || err3 < 0)
 		goto retry;
 
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index d8ca7f64f952..7404f0935c90 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -652,13 +652,6 @@ int ext4_ind_map_blocks(handle_t *handle, struct inode *inode,
 	ext4_update_inode_fsync_trans(handle, inode, 1);
 	count = ar.len;
 
-	/*
-	 * Update reserved blocks/metadata blocks after successful block
-	 * allocation which had been deferred till now.
-	 */
-	if (flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE)
-		ext4_da_update_reserve_space(inode, count, 1);
-
 got_it:
 	map->m_flags |= EXT4_MAP_MAPPED;
 	map->m_pblk = le32_to_cpu(chain[depth-1].key);
-- 
2.39.2

Re: [PATCH v3 06/12] ext4: update delalloc data reserve spcae in ext4_es_insert_extent()

[Jan Kara]

On Tue 13-08-24 20:34:46, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> Now that we update data reserved space for delalloc after allocating
> new blocks in ext4_{ind|ext}_map_blocks(), and if bigalloc feature is
> enabled, we also need to query the extents_status tree to calculate the
> exact reserved clusters. This is complicated now and it appears that
> it's better to do this job in ext4_es_insert_extent(), because
> __es_remove_extent() have already count delalloc blocks when removing
> delalloc extents and __revise_pending() return new adding pending count,
> we could update the reserved blocks easily in ext4_es_insert_extent().
> 
> We direct reduce the reserved cluster count when replacing a delalloc
> extent. However, thers are two special cases need to concern about the
> quota claiming when doing direct block allocation (e.g. from fallocate).
> 
> A),
> fallocate a range that covers a delalloc extent but start with
> non-delayed allocated blocks, e.g. a hole.
> 
>   hhhhhhh+ddddddd+ddddddd
>   ^^^^^^^^^^^^^^^^^^^^^^^  fallocate this range
> 
> Current ext4_map_blocks() can't always trim the extent since it may
> release i_data_sem before calling ext4_map_create_blocks() and raced by
> another delayed allocation. Hence the EXT4_GET_BLOCKS_DELALLOC_RESERVE
> may not set even when we are replacing a delalloc extent, without this
> flag set, the quota has already been claimed by ext4_mb_new_blocks(), so
> we should release the quota reservations instead of claim them again.
> 
> B),
> bigalloc feature is enabled, fallocate a range that contains non-delayed
> allocated blocks.
> 
>   |<         one cluster       >|
>   hhhhhhh+hhhhhhh+hhhhhhh+ddddddd
>   ^^^^^^^  fallocate this range
> 
> This case is similar to above case, the EXT4_GET_BLOCKS_DELALLOC_RESERVE
> flag is also not set.
> 
> Hence we should release the quota reservations if we replace a delalloc
> extent but without EXT4_GET_BLOCKS_DELALLOC_RESERVE set.
> 
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

I really like how simple this ended being! Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/extents.c        | 37 -------------------------------------
>  fs/ext4/extents_status.c | 25 ++++++++++++++++++++++++-
>  fs/ext4/indirect.c       |  7 -------
>  3 files changed, 24 insertions(+), 45 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 671dacd7c873..db8f9d79477c 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4357,43 +4357,6 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
>  		goto out;
>  	}
>  
> -	/*
> -	 * Reduce the reserved cluster count to reflect successful deferred
> -	 * allocation of delayed allocated clusters or direct allocation of
> -	 * clusters discovered to be delayed allocated.  Once allocated, a
> -	 * cluster is not included in the reserved count.
> -	 */
> -	if (test_opt(inode->i_sb, DELALLOC) && allocated_clusters) {
> -		if (flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE) {
> -			/*
> -			 * When allocating delayed allocated clusters, simply
> -			 * reduce the reserved cluster count and claim quota
> -			 */
> -			ext4_da_update_reserve_space(inode, allocated_clusters,
> -							1);
> -		} else {
> -			ext4_lblk_t lblk, len;
> -			unsigned int n;
> -
> -			/*
> -			 * When allocating non-delayed allocated clusters
> -			 * (from fallocate, filemap, DIO, or clusters
> -			 * allocated when delalloc has been disabled by
> -			 * ext4_nonda_switch), reduce the reserved cluster
> -			 * count by the number of allocated clusters that
> -			 * have previously been delayed allocated.  Quota
> -			 * has been claimed by ext4_mb_new_blocks() above,
> -			 * so release the quota reservations made for any
> -			 * previously delayed allocated clusters.
> -			 */
> -			lblk = EXT4_LBLK_CMASK(sbi, map->m_lblk);
> -			len = allocated_clusters << sbi->s_cluster_bits;
> -			n = ext4_es_delayed_clu(inode, lblk, len);
> -			if (n > 0)
> -				ext4_da_update_reserve_space(inode, (int) n, 0);
> -		}
> -	}
> -
>  	/*
>  	 * Cache the extent and update transaction to commit on fdatasync only
>  	 * when it is _not_ an unwritten extent.
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 0580bc4bc762..41adf0d69959 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -853,6 +853,7 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>  	struct extent_status newes;
>  	ext4_lblk_t end = lblk + len - 1;
>  	int err1 = 0, err2 = 0, err3 = 0;
> +	int resv_used = 0, pending = 0;
>  	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
>  	struct extent_status *es1 = NULL;
>  	struct extent_status *es2 = NULL;
> @@ -891,7 +892,7 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>  		pr = __alloc_pending(true);
>  	write_lock(&EXT4_I(inode)->i_es_lock);
>  
> -	err1 = __es_remove_extent(inode, lblk, end, NULL, es1);
> +	err1 = __es_remove_extent(inode, lblk, end, &resv_used, es1);
>  	if (err1 != 0)
>  		goto error;
>  	/* Free preallocated extent if it didn't get used. */
> @@ -921,9 +922,31 @@ void ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk,
>  			__free_pending(pr);
>  			pr = NULL;
>  		}
> +		pending = err3;
>  	}
>  error:
>  	write_unlock(&EXT4_I(inode)->i_es_lock);
> +	/*
> +	 * Reduce the reserved cluster count to reflect successful deferred
> +	 * allocation of delayed allocated clusters or direct allocation of
> +	 * clusters discovered to be delayed allocated.  Once allocated, a
> +	 * cluster is not included in the reserved count.
> +	 *
> +	 * When direct allocating (from fallocate, filemap, DIO, or clusters
> +	 * allocated when delalloc has been disabled by ext4_nonda_switch())
> +	 * an extent either 1) contains delayed blocks but start with
> +	 * non-delayed allocated blocks (e.g. hole) or 2) contains non-delayed
> +	 * allocated blocks which belong to delayed allocated clusters when
> +	 * bigalloc feature is enabled, quota has already been claimed by
> +	 * ext4_mb_new_blocks(), so release the quota reservations made for
> +	 * any previously delayed allocated clusters instead of claim them
> +	 * again.
> +	 */
> +	resv_used += pending;
> +	if (resv_used)
> +		ext4_da_update_reserve_space(inode, resv_used,
> +				flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE);
> +
>  	if (err1 || err2 || err3 < 0)
>  		goto retry;
>  
> diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
> index d8ca7f64f952..7404f0935c90 100644
> --- a/fs/ext4/indirect.c
> +++ b/fs/ext4/indirect.c
> @@ -652,13 +652,6 @@ int ext4_ind_map_blocks(handle_t *handle, struct inode *inode,
>  	ext4_update_inode_fsync_trans(handle, inode, 1);
>  	count = ar.len;
>  
> -	/*
> -	 * Update reserved blocks/metadata blocks after successful block
> -	 * allocation which had been deferred till now.
> -	 */
> -	if (flags & EXT4_GET_BLOCKS_DELALLOC_RESERVE)
> -		ext4_da_update_reserve_space(inode, count, 1);
> -
>  got_it:
>  	map->m_flags |= EXT4_MAP_MAPPED;
>  	map->m_pblk = le32_to_cpu(chain[depth-1].key);
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v3 07/12] ext4: drop ext4_es_delayed_clu()

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

Since we move ext4_da_update_reserve_space() to ext4_es_insert_extent(),
no one uses ext4_es_delayed_clu() and __es_delayed_clu(), just drop
them.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 fs/ext4/extents_status.c | 88 ----------------------------------------
 fs/ext4/extents_status.h |  2 -
 2 files changed, 90 deletions(-)

diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 41adf0d69959..b372b98af366 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -2178,94 +2178,6 @@ void ext4_es_insert_delayed_extent(struct inode *inode, ext4_lblk_t lblk,
 	return;
 }
 
-/*
- * __es_delayed_clu - count number of clusters containing blocks that
- *                    are delayed only
- *
- * @inode - file containing block range
- * @start - logical block defining start of range
- * @end - logical block defining end of range
- *
- * Returns the number of clusters containing only delayed (not delayed
- * and unwritten) blocks in the range specified by @start and @end.  Any
- * cluster or part of a cluster within the range and containing a delayed
- * and not unwritten block within the range is counted as a whole cluster.
- */
-static unsigned int __es_delayed_clu(struct inode *inode, ext4_lblk_t start,
-				     ext4_lblk_t end)
-{
-	struct ext4_es_tree *tree = &EXT4_I(inode)->i_es_tree;
-	struct extent_status *es;
-	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
-	struct rb_node *node;
-	ext4_lblk_t first_lclu, last_lclu;
-	unsigned long long last_counted_lclu;
-	unsigned int n = 0;
-
-	/* guaranteed to be unequal to any ext4_lblk_t value */
-	last_counted_lclu = ~0ULL;
-
-	es = __es_tree_search(&tree->root, start);
-
-	while (es && (es->es_lblk <= end)) {
-		if (ext4_es_is_delonly(es)) {
-			if (es->es_lblk <= start)
-				first_lclu = EXT4_B2C(sbi, start);
-			else
-				first_lclu = EXT4_B2C(sbi, es->es_lblk);
-
-			if (ext4_es_end(es) >= end)
-				last_lclu = EXT4_B2C(sbi, end);
-			else
-				last_lclu = EXT4_B2C(sbi, ext4_es_end(es));
-
-			if (first_lclu == last_counted_lclu)
-				n += last_lclu - first_lclu;
-			else
-				n += last_lclu - first_lclu + 1;
-			last_counted_lclu = last_lclu;
-		}
-		node = rb_next(&es->rb_node);
-		if (!node)
-			break;
-		es = rb_entry(node, struct extent_status, rb_node);
-	}
-
-	return n;
-}
-
-/*
- * ext4_es_delayed_clu - count number of clusters containing blocks that
- *                       are both delayed and unwritten
- *
- * @inode - file containing block range
- * @lblk - logical block defining start of range
- * @len - number of blocks in range
- *
- * Locking for external use of __es_delayed_clu().
- */
-unsigned int ext4_es_delayed_clu(struct inode *inode, ext4_lblk_t lblk,
-				 ext4_lblk_t len)
-{
-	struct ext4_inode_info *ei = EXT4_I(inode);
-	ext4_lblk_t end;
-	unsigned int n;
-
-	if (len == 0)
-		return 0;
-
-	end = lblk + len - 1;
-	WARN_ON(end < lblk);
-
-	read_lock(&ei->i_es_lock);
-
-	n = __es_delayed_clu(inode, lblk, end);
-
-	read_unlock(&ei->i_es_lock);
-
-	return n;
-}
-
 /*
  * __revise_pending - makes, cancels, or leaves unchanged pending cluster
  *                    reservations for a specified block range depending
diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
index b74d693c1adb..47b3b55a852c 100644
--- a/fs/ext4/extents_status.h
+++ b/fs/ext4/extents_status.h
@@ -252,8 +252,6 @@ extern bool ext4_is_pending(struct inode *inode, ext4_lblk_t lblk);
 extern void ext4_es_insert_delayed_extent(struct inode *inode, ext4_lblk_t lblk,
 					  ext4_lblk_t len, bool lclu_allocated,
 					  bool end_allocated);
-extern unsigned int ext4_es_delayed_clu(struct inode *inode, ext4_lblk_t lblk,
-					ext4_lblk_t len);
 extern void ext4_clear_inode_es(struct inode *inode);
 
 #endif /* _EXT4_EXTENTS_STATUS_H */
-- 
2.39.2

Re: [PATCH v3 07/12] ext4: drop ext4_es_delayed_clu()

[Jan Kara]

On Tue 13-08-24 20:34:47, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> Since we move ext4_da_update_reserve_space() to ext4_es_insert_extent(),
> no one uses ext4_es_delayed_clu() and __es_delayed_clu(), just drop
> them.
> 
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Always nice to see code removed :) Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

									Honza

> ---
>  fs/ext4/extents_status.c | 88 ----------------------------------------
>  fs/ext4/extents_status.h |  2 -
>  2 files changed, 90 deletions(-)
> 
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 41adf0d69959..b372b98af366 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -2178,94 +2178,6 @@ void ext4_es_insert_delayed_extent(struct inode *inode, ext4_lblk_t lblk,
>  	return;
>  }
>  
> -/*
> - * __es_delayed_clu - count number of clusters containing blocks that
> - *                    are delayed only
> - *
> - * @inode - file containing block range
> - * @start - logical block defining start of range
> - * @end - logical block defining end of range
> - *
> - * Returns the number of clusters containing only delayed (not delayed
> - * and unwritten) blocks in the range specified by @start and @end.  Any
> - * cluster or part of a cluster within the range and containing a delayed
> - * and not unwritten block within the range is counted as a whole cluster.
> - */
> -static unsigned int __es_delayed_clu(struct inode *inode, ext4_lblk_t start,
> -				     ext4_lblk_t end)
> -{
> -	struct ext4_es_tree *tree = &EXT4_I(inode)->i_es_tree;
> -	struct extent_status *es;
> -	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> -	struct rb_node *node;
> -	ext4_lblk_t first_lclu, last_lclu;
> -	unsigned long long last_counted_lclu;
> -	unsigned int n = 0;
> -
> -	/* guaranteed to be unequal to any ext4_lblk_t value */
> -	last_counted_lclu = ~0ULL;
> -
> -	es = __es_tree_search(&tree->root, start);
> -
> -	while (es && (es->es_lblk <= end)) {
> -		if (ext4_es_is_delonly(es)) {
> -			if (es->es_lblk <= start)
> -				first_lclu = EXT4_B2C(sbi, start);
> -			else
> -				first_lclu = EXT4_B2C(sbi, es->es_lblk);
> -
> -			if (ext4_es_end(es) >= end)
> -				last_lclu = EXT4_B2C(sbi, end);
> -			else
> -				last_lclu = EXT4_B2C(sbi, ext4_es_end(es));
> -
> -			if (first_lclu == last_counted_lclu)
> -				n += last_lclu - first_lclu;
> -			else
> -				n += last_lclu - first_lclu + 1;
> -			last_counted_lclu = last_lclu;
> -		}
> -		node = rb_next(&es->rb_node);
> -		if (!node)
> -			break;
> -		es = rb_entry(node, struct extent_status, rb_node);
> -	}
> -
> -	return n;
> -}
> -
> -/*
> - * ext4_es_delayed_clu - count number of clusters containing blocks that
> - *                       are both delayed and unwritten
> - *
> - * @inode - file containing block range
> - * @lblk - logical block defining start of range
> - * @len - number of blocks in range
> - *
> - * Locking for external use of __es_delayed_clu().
> - */
> -unsigned int ext4_es_delayed_clu(struct inode *inode, ext4_lblk_t lblk,
> -				 ext4_lblk_t len)
> -{
> -	struct ext4_inode_info *ei = EXT4_I(inode);
> -	ext4_lblk_t end;
> -	unsigned int n;
> -
> -	if (len == 0)
> -		return 0;
> -
> -	end = lblk + len - 1;
> -	WARN_ON(end < lblk);
> -
> -	read_lock(&ei->i_es_lock);
> -
> -	n = __es_delayed_clu(inode, lblk, end);
> -
> -	read_unlock(&ei->i_es_lock);
> -
> -	return n;
> -}
> -
>  /*
>   * __revise_pending - makes, cancels, or leaves unchanged pending cluster
>   *                    reservations for a specified block range depending
> diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
> index b74d693c1adb..47b3b55a852c 100644
> --- a/fs/ext4/extents_status.h
> +++ b/fs/ext4/extents_status.h
> @@ -252,8 +252,6 @@ extern bool ext4_is_pending(struct inode *inode, ext4_lblk_t lblk);
>  extern void ext4_es_insert_delayed_extent(struct inode *inode, ext4_lblk_t lblk,
>  					  ext4_lblk_t len, bool lclu_allocated,
>  					  bool end_allocated);
> -extern unsigned int ext4_es_delayed_clu(struct inode *inode, ext4_lblk_t lblk,
> -					ext4_lblk_t len);
>  extern void ext4_clear_inode_es(struct inode *inode);
>  
>  #endif /* _EXT4_EXTENTS_STATUS_H */
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v3 08/12] ext4: use ext4_map_query_blocks() in ext4_map_blocks()

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

The blocks map querying logic in ext4_map_blocks() are the same as
ext4_map_query_blocks(), so switch to directly use it.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/inode.c | 22 +---------------------
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 260a38453604..2fa13e9e78bc 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -658,27 +658,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
 	 * file system block.
 	 */
 	down_read(&EXT4_I(inode)->i_data_sem);
-	if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
-		retval = ext4_ext_map_blocks(handle, inode, map, 0);
-	} else {
-		retval = ext4_ind_map_blocks(handle, inode, map, 0);
-	}
-	if (retval > 0) {
-		unsigned int status;
-
-		if (unlikely(retval != map->m_len)) {
-			ext4_warning(inode->i_sb,
-				     "ES len assertion failed for inode "
-				     "%lu: retval %d != map->m_len %d",
-				     inode->i_ino, retval, map->m_len);
-			WARN_ON(1);
-		}
-
-		status = map->m_flags & EXT4_MAP_UNWRITTEN ?
-				EXTENT_STATUS_UNWRITTEN : EXTENT_STATUS_WRITTEN;
-		ext4_es_insert_extent(inode, map->m_lblk, map->m_len,
-				      map->m_pblk, status, 0);
-	}
+	retval = ext4_map_query_blocks(handle, inode, map);
 	up_read((&EXT4_I(inode)->i_data_sem));
 
 found:
-- 
2.39.2

[PATCH v3 09/12] ext4: drop unused ext4_es_store_status()

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

The helper ext4_es_store_status() is unused now, just drop it.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 fs/ext4/extents_status.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
index 47b3b55a852c..3ca40f018994 100644
--- a/fs/ext4/extents_status.h
+++ b/fs/ext4/extents_status.h
@@ -224,13 +224,6 @@ static inline void ext4_es_store_pblock(struct extent_status *es,
 	es->es_pblk = block;
 }
 
-static inline void ext4_es_store_status(struct extent_status *es,
-					unsigned int status)
-{
-	es->es_pblk = (((ext4_fsblk_t)status << ES_SHIFT) & ES_MASK) |
-		      (es->es_pblk & ~ES_MASK);
-}
-
 static inline void ext4_es_store_pblock_status(struct extent_status *es,
 					       ext4_fsblk_t pb,
 					       unsigned int status)
-- 
2.39.2

Re: [PATCH v3 09/12] ext4: drop unused ext4_es_store_status()

[Jan Kara]

On Tue 13-08-24 20:34:49, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> The helper ext4_es_store_status() is unused now, just drop it.
> 
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Yes. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

									Honza

> ---
>  fs/ext4/extents_status.h | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
> index 47b3b55a852c..3ca40f018994 100644
> --- a/fs/ext4/extents_status.h
> +++ b/fs/ext4/extents_status.h
> @@ -224,13 +224,6 @@ static inline void ext4_es_store_pblock(struct extent_status *es,
>  	es->es_pblk = block;
>  }
>  
> -static inline void ext4_es_store_status(struct extent_status *es,
> -					unsigned int status)
> -{
> -	es->es_pblk = (((ext4_fsblk_t)status << ES_SHIFT) & ES_MASK) |
> -		      (es->es_pblk & ~ES_MASK);
> -}
> -
>  static inline void ext4_es_store_pblock_status(struct extent_status *es,
>  					       ext4_fsblk_t pb,
>  					       unsigned int status)
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v3 10/12] ext4: make extent status types exclusive

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

Since we don't add delayed flag in unwritten extents, all of the four
extent status types EXTENT_STATUS_WRITTEN, EXTENT_STATUS_UNWRITTEN,
EXTENT_STATUS_DELAYED and EXTENT_STATUS_HOLE are exclusive now, add
assertion when storing pblock before inserting extent into status tree
and add comment to the status definition.

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 fs/ext4/extents_status.h | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
index 3ca40f018994..7d7af642f7b2 100644
--- a/fs/ext4/extents_status.h
+++ b/fs/ext4/extents_status.h
@@ -42,6 +42,10 @@ enum {
 #define ES_SHIFT (sizeof(ext4_fsblk_t)*8 - ES_FLAGS)
 #define ES_MASK (~((ext4_fsblk_t)0) << ES_SHIFT)
 
+/*
+ * Besides EXTENT_STATUS_REFERENCED, all these extent type masks
+ * are exclusive, only one type can be set at a time.
+ */
 #define EXTENT_STATUS_WRITTEN	(1 << ES_WRITTEN_B)
 #define EXTENT_STATUS_UNWRITTEN (1 << ES_UNWRITTEN_B)
 #define EXTENT_STATUS_DELAYED	(1 << ES_DELAYED_B)
@@ -51,7 +55,9 @@ enum {
 #define ES_TYPE_MASK	((ext4_fsblk_t)(EXTENT_STATUS_WRITTEN | \
 			  EXTENT_STATUS_UNWRITTEN | \
 			  EXTENT_STATUS_DELAYED | \
-			  EXTENT_STATUS_HOLE) << ES_SHIFT)
+			  EXTENT_STATUS_HOLE))
+
+#define ES_TYPE_VALID(type)	((type) && !((type) & ((type) - 1)))
 
 struct ext4_sb_info;
 struct ext4_extent;
@@ -156,7 +162,7 @@ static inline unsigned int ext4_es_status(struct extent_status *es)
 
 static inline unsigned int ext4_es_type(struct extent_status *es)
 {
-	return (es->es_pblk & ES_TYPE_MASK) >> ES_SHIFT;
+	return (es->es_pblk >> ES_SHIFT) & ES_TYPE_MASK;
 }
 
 static inline int ext4_es_is_written(struct extent_status *es)
@@ -228,6 +234,8 @@ static inline void ext4_es_store_pblock_status(struct extent_status *es,
 					       ext4_fsblk_t pb,
 					       unsigned int status)
 {
+	WARN_ON_ONCE(!ES_TYPE_VALID(status & ES_TYPE_MASK));
+
 	es->es_pblk = (((ext4_fsblk_t)status << ES_SHIFT) & ES_MASK) |
 		      (pb & ~ES_MASK);
 }
-- 
2.39.2

Re: [PATCH v3 10/12] ext4: make extent status types exclusive

[Jan Kara]

On Tue 13-08-24 20:34:50, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> Since we don't add delayed flag in unwritten extents, all of the four
> extent status types EXTENT_STATUS_WRITTEN, EXTENT_STATUS_UNWRITTEN,
> EXTENT_STATUS_DELAYED and EXTENT_STATUS_HOLE are exclusive now, add
> assertion when storing pblock before inserting extent into status tree
> and add comment to the status definition.
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/extents_status.h | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
> index 3ca40f018994..7d7af642f7b2 100644
> --- a/fs/ext4/extents_status.h
> +++ b/fs/ext4/extents_status.h
> @@ -42,6 +42,10 @@ enum {
>  #define ES_SHIFT (sizeof(ext4_fsblk_t)*8 - ES_FLAGS)
>  #define ES_MASK (~((ext4_fsblk_t)0) << ES_SHIFT)
>  
> +/*
> + * Besides EXTENT_STATUS_REFERENCED, all these extent type masks
> + * are exclusive, only one type can be set at a time.
> + */
>  #define EXTENT_STATUS_WRITTEN	(1 << ES_WRITTEN_B)
>  #define EXTENT_STATUS_UNWRITTEN (1 << ES_UNWRITTEN_B)
>  #define EXTENT_STATUS_DELAYED	(1 << ES_DELAYED_B)
> @@ -51,7 +55,9 @@ enum {
>  #define ES_TYPE_MASK	((ext4_fsblk_t)(EXTENT_STATUS_WRITTEN | \
>  			  EXTENT_STATUS_UNWRITTEN | \
>  			  EXTENT_STATUS_DELAYED | \
> -			  EXTENT_STATUS_HOLE) << ES_SHIFT)
> +			  EXTENT_STATUS_HOLE))
> +
> +#define ES_TYPE_VALID(type)	((type) && !((type) & ((type) - 1)))
>  
>  struct ext4_sb_info;
>  struct ext4_extent;
> @@ -156,7 +162,7 @@ static inline unsigned int ext4_es_status(struct extent_status *es)
>  
>  static inline unsigned int ext4_es_type(struct extent_status *es)
>  {
> -	return (es->es_pblk & ES_TYPE_MASK) >> ES_SHIFT;
> +	return (es->es_pblk >> ES_SHIFT) & ES_TYPE_MASK;
>  }
>  
>  static inline int ext4_es_is_written(struct extent_status *es)
> @@ -228,6 +234,8 @@ static inline void ext4_es_store_pblock_status(struct extent_status *es,
>  					       ext4_fsblk_t pb,
>  					       unsigned int status)
>  {
> +	WARN_ON_ONCE(!ES_TYPE_VALID(status & ES_TYPE_MASK));
> +
>  	es->es_pblk = (((ext4_fsblk_t)status << ES_SHIFT) & ES_MASK) |
>  		      (pb & ~ES_MASK);
>  }
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v3 11/12] ext4: drop ext4_es_is_delonly()

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

Since we don't add delayed flag in unwritten extents, so there is no
difference between ext4_es_is_delayed() and ext4_es_is_delonly(),
just drop ext4_es_is_delonly().

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 fs/ext4/extents_status.c | 18 +++++++++---------
 fs/ext4/extents_status.h |  5 -----
 fs/ext4/inode.c          |  4 ++--
 3 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index b372b98af366..68c47ecc01a5 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -558,8 +558,8 @@ static int ext4_es_can_be_merged(struct extent_status *es1,
 	if (ext4_es_is_hole(es1))
 		return 1;
 
-	/* we need to check delayed extent is without unwritten status */
-	if (ext4_es_is_delayed(es1) && !ext4_es_is_unwritten(es1))
+	/* we need to check delayed extent */
+	if (ext4_es_is_delayed(es1))
 		return 1;
 
 	return 0;
@@ -1135,7 +1135,7 @@ static void count_rsvd(struct inode *inode, ext4_lblk_t lblk, long len,
 	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
 	ext4_lblk_t i, end, nclu;
 
-	if (!ext4_es_is_delonly(es))
+	if (!ext4_es_is_delayed(es))
 		return;
 
 	WARN_ON(len <= 0);
@@ -1285,7 +1285,7 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
 		es = rc->left_es;
 		while (es && ext4_es_end(es) >=
 		       EXT4_LBLK_CMASK(sbi, rc->first_do_lblk)) {
-			if (ext4_es_is_delonly(es)) {
+			if (ext4_es_is_delayed(es)) {
 				rc->ndelonly--;
 				left_delonly = true;
 				break;
@@ -1305,7 +1305,7 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
 			}
 			while (es && es->es_lblk <=
 			       EXT4_LBLK_CFILL(sbi, rc->last_do_lblk)) {
-				if (ext4_es_is_delonly(es)) {
+				if (ext4_es_is_delayed(es)) {
 					rc->ndelonly--;
 					right_delonly = true;
 					break;
@@ -2226,7 +2226,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
 	if (EXT4_B2C(sbi, lblk) == EXT4_B2C(sbi, end)) {
 		first = EXT4_LBLK_CMASK(sbi, lblk);
 		if (first != lblk)
-			f_del = __es_scan_range(inode, &ext4_es_is_delonly,
+			f_del = __es_scan_range(inode, &ext4_es_is_delayed,
 						first, lblk - 1);
 		if (f_del) {
 			ret = __insert_pending(inode, first, prealloc);
@@ -2238,7 +2238,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
 			       sbi->s_cluster_ratio - 1;
 			if (last != end)
 				l_del = __es_scan_range(inode,
-							&ext4_es_is_delonly,
+							&ext4_es_is_delayed,
 							end + 1, last);
 			if (l_del) {
 				ret = __insert_pending(inode, last, prealloc);
@@ -2251,7 +2251,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
 	} else {
 		first = EXT4_LBLK_CMASK(sbi, lblk);
 		if (first != lblk)
-			f_del = __es_scan_range(inode, &ext4_es_is_delonly,
+			f_del = __es_scan_range(inode, &ext4_es_is_delayed,
 						first, lblk - 1);
 		if (f_del) {
 			ret = __insert_pending(inode, first, prealloc);
@@ -2263,7 +2263,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
 
 		last = EXT4_LBLK_CMASK(sbi, end) + sbi->s_cluster_ratio - 1;
 		if (last != end)
-			l_del = __es_scan_range(inode, &ext4_es_is_delonly,
+			l_del = __es_scan_range(inode, &ext4_es_is_delayed,
 						end + 1, last);
 		if (l_del) {
 			ret = __insert_pending(inode, last, prealloc);
diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
index 7d7af642f7b2..4424232de298 100644
--- a/fs/ext4/extents_status.h
+++ b/fs/ext4/extents_status.h
@@ -190,11 +190,6 @@ static inline int ext4_es_is_mapped(struct extent_status *es)
 	return (ext4_es_is_written(es) || ext4_es_is_unwritten(es));
 }
 
-static inline int ext4_es_is_delonly(struct extent_status *es)
-{
-	return (ext4_es_is_delayed(es) && !ext4_es_is_unwritten(es));
-}
-
 static inline void ext4_es_set_referenced(struct extent_status *es)
 {
 	es->es_pblk |= ((ext4_fsblk_t)EXTENT_STATUS_REFERENCED) << ES_SHIFT;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 2fa13e9e78bc..bdf466d5a8d4 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1645,7 +1645,7 @@ static int ext4_clu_alloc_state(struct inode *inode, ext4_lblk_t lblk)
 	int ret;
 
 	/* Has delalloc reservation? */
-	if (ext4_es_scan_clu(inode, &ext4_es_is_delonly, lblk))
+	if (ext4_es_scan_clu(inode, &ext4_es_is_delayed, lblk))
 		return 1;
 
 	/* Already been allocated? */
@@ -1766,7 +1766,7 @@ static int ext4_da_map_blocks(struct inode *inode, struct ext4_map_blocks *map)
 		 * Delayed extent could be allocated by fallocate.
 		 * So we need to check it.
 		 */
-		if (ext4_es_is_delonly(&es)) {
+		if (ext4_es_is_delayed(&es)) {
 			map->m_flags |= EXT4_MAP_DELAYED;
 			return 0;
 		}
-- 
2.39.2

Re: [PATCH v3 11/12] ext4: drop ext4_es_is_delonly()

[Jan Kara]

On Tue 13-08-24 20:34:51, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> Since we don't add delayed flag in unwritten extents, so there is no
> difference between ext4_es_is_delayed() and ext4_es_is_delonly(),
> just drop ext4_es_is_delonly().
> 
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/extents_status.c | 18 +++++++++---------
>  fs/ext4/extents_status.h |  5 -----
>  fs/ext4/inode.c          |  4 ++--
>  3 files changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index b372b98af366..68c47ecc01a5 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -558,8 +558,8 @@ static int ext4_es_can_be_merged(struct extent_status *es1,
>  	if (ext4_es_is_hole(es1))
>  		return 1;
>  
> -	/* we need to check delayed extent is without unwritten status */
> -	if (ext4_es_is_delayed(es1) && !ext4_es_is_unwritten(es1))
> +	/* we need to check delayed extent */
> +	if (ext4_es_is_delayed(es1))
>  		return 1;
>  
>  	return 0;
> @@ -1135,7 +1135,7 @@ static void count_rsvd(struct inode *inode, ext4_lblk_t lblk, long len,
>  	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
>  	ext4_lblk_t i, end, nclu;
>  
> -	if (!ext4_es_is_delonly(es))
> +	if (!ext4_es_is_delayed(es))
>  		return;
>  
>  	WARN_ON(len <= 0);
> @@ -1285,7 +1285,7 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
>  		es = rc->left_es;
>  		while (es && ext4_es_end(es) >=
>  		       EXT4_LBLK_CMASK(sbi, rc->first_do_lblk)) {
> -			if (ext4_es_is_delonly(es)) {
> +			if (ext4_es_is_delayed(es)) {
>  				rc->ndelonly--;
>  				left_delonly = true;
>  				break;
> @@ -1305,7 +1305,7 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
>  			}
>  			while (es && es->es_lblk <=
>  			       EXT4_LBLK_CFILL(sbi, rc->last_do_lblk)) {
> -				if (ext4_es_is_delonly(es)) {
> +				if (ext4_es_is_delayed(es)) {
>  					rc->ndelonly--;
>  					right_delonly = true;
>  					break;
> @@ -2226,7 +2226,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
>  	if (EXT4_B2C(sbi, lblk) == EXT4_B2C(sbi, end)) {
>  		first = EXT4_LBLK_CMASK(sbi, lblk);
>  		if (first != lblk)
> -			f_del = __es_scan_range(inode, &ext4_es_is_delonly,
> +			f_del = __es_scan_range(inode, &ext4_es_is_delayed,
>  						first, lblk - 1);
>  		if (f_del) {
>  			ret = __insert_pending(inode, first, prealloc);
> @@ -2238,7 +2238,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
>  			       sbi->s_cluster_ratio - 1;
>  			if (last != end)
>  				l_del = __es_scan_range(inode,
> -							&ext4_es_is_delonly,
> +							&ext4_es_is_delayed,
>  							end + 1, last);
>  			if (l_del) {
>  				ret = __insert_pending(inode, last, prealloc);
> @@ -2251,7 +2251,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
>  	} else {
>  		first = EXT4_LBLK_CMASK(sbi, lblk);
>  		if (first != lblk)
> -			f_del = __es_scan_range(inode, &ext4_es_is_delonly,
> +			f_del = __es_scan_range(inode, &ext4_es_is_delayed,
>  						first, lblk - 1);
>  		if (f_del) {
>  			ret = __insert_pending(inode, first, prealloc);
> @@ -2263,7 +2263,7 @@ static int __revise_pending(struct inode *inode, ext4_lblk_t lblk,
>  
>  		last = EXT4_LBLK_CMASK(sbi, end) + sbi->s_cluster_ratio - 1;
>  		if (last != end)
> -			l_del = __es_scan_range(inode, &ext4_es_is_delonly,
> +			l_del = __es_scan_range(inode, &ext4_es_is_delayed,
>  						end + 1, last);
>  		if (l_del) {
>  			ret = __insert_pending(inode, last, prealloc);
> diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h
> index 7d7af642f7b2..4424232de298 100644
> --- a/fs/ext4/extents_status.h
> +++ b/fs/ext4/extents_status.h
> @@ -190,11 +190,6 @@ static inline int ext4_es_is_mapped(struct extent_status *es)
>  	return (ext4_es_is_written(es) || ext4_es_is_unwritten(es));
>  }
>  
> -static inline int ext4_es_is_delonly(struct extent_status *es)
> -{
> -	return (ext4_es_is_delayed(es) && !ext4_es_is_unwritten(es));
> -}
> -
>  static inline void ext4_es_set_referenced(struct extent_status *es)
>  {
>  	es->es_pblk |= ((ext4_fsblk_t)EXTENT_STATUS_REFERENCED) << ES_SHIFT;
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 2fa13e9e78bc..bdf466d5a8d4 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1645,7 +1645,7 @@ static int ext4_clu_alloc_state(struct inode *inode, ext4_lblk_t lblk)
>  	int ret;
>  
>  	/* Has delalloc reservation? */
> -	if (ext4_es_scan_clu(inode, &ext4_es_is_delonly, lblk))
> +	if (ext4_es_scan_clu(inode, &ext4_es_is_delayed, lblk))
>  		return 1;
>  
>  	/* Already been allocated? */
> @@ -1766,7 +1766,7 @@ static int ext4_da_map_blocks(struct inode *inode, struct ext4_map_blocks *map)
>  		 * Delayed extent could be allocated by fallocate.
>  		 * So we need to check it.
>  		 */
> -		if (ext4_es_is_delonly(&es)) {
> +		if (ext4_es_is_delayed(&es)) {
>  			map->m_flags |= EXT4_MAP_DELAYED;
>  			return 0;
>  		}
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH v3 12/12] ext4: drop all delonly descriptions

[Zhang Yi]

From: Zhang Yi <yi.zhang@huawei.com>

When counting reserved clusters, delayed type is always equal to delonly
type now, hence drop all delonly descriptions in parameters and
comments.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 fs/ext4/extents_status.c | 66 +++++++++++++++++++---------------------
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 68c47ecc01a5..c786691dabd3 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -1067,7 +1067,7 @@ int ext4_es_lookup_extent(struct inode *inode, ext4_lblk_t lblk,
 }
 
 struct rsvd_count {
-	int ndelonly;
+	int ndelayed;
 	bool first_do_lblk_found;
 	ext4_lblk_t first_do_lblk;
 	ext4_lblk_t last_do_lblk;
@@ -1093,10 +1093,10 @@ static void init_rsvd(struct inode *inode, ext4_lblk_t lblk,
 	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
 	struct rb_node *node;
 
-	rc->ndelonly = 0;
+	rc->ndelayed = 0;
 
 	/*
-	 * for bigalloc, note the first delonly block in the range has not
+	 * for bigalloc, note the first delayed block in the range has not
 	 * been found, record the extent containing the block to the left of
 	 * the region to be removed, if any, and note that there's no partial
 	 * cluster to track
@@ -1116,9 +1116,8 @@ static void init_rsvd(struct inode *inode, ext4_lblk_t lblk,
 }
 
 /*
- * count_rsvd - count the clusters containing delayed and not unwritten
- *		(delonly) blocks in a range within an extent and add to
- *	        the running tally in rsvd_count
+ * count_rsvd - count the clusters containing delayed blocks in a range
+ *	        within an extent and add to the running tally in rsvd_count
  *
  * @inode - file containing extent
  * @lblk - first block in range
@@ -1141,7 +1140,7 @@ static void count_rsvd(struct inode *inode, ext4_lblk_t lblk, long len,
 	WARN_ON(len <= 0);
 
 	if (sbi->s_cluster_ratio == 1) {
-		rc->ndelonly += (int) len;
+		rc->ndelayed += (int) len;
 		return;
 	}
 
@@ -1151,7 +1150,7 @@ static void count_rsvd(struct inode *inode, ext4_lblk_t lblk, long len,
 	end = lblk + (ext4_lblk_t) len - 1;
 	end = (end > ext4_es_end(es)) ? ext4_es_end(es) : end;
 
-	/* record the first block of the first delonly extent seen */
+	/* record the first block of the first delayed extent seen */
 	if (!rc->first_do_lblk_found) {
 		rc->first_do_lblk = i;
 		rc->first_do_lblk_found = true;
@@ -1165,7 +1164,7 @@ static void count_rsvd(struct inode *inode, ext4_lblk_t lblk, long len,
 	 * doesn't start with it, count it and stop tracking
 	 */
 	if (rc->partial && (rc->lclu != EXT4_B2C(sbi, i))) {
-		rc->ndelonly++;
+		rc->ndelayed++;
 		rc->partial = false;
 	}
 
@@ -1175,7 +1174,7 @@ static void count_rsvd(struct inode *inode, ext4_lblk_t lblk, long len,
 	 */
 	if (EXT4_LBLK_COFF(sbi, i) != 0) {
 		if (end >= EXT4_LBLK_CFILL(sbi, i)) {
-			rc->ndelonly++;
+			rc->ndelayed++;
 			rc->partial = false;
 			i = EXT4_LBLK_CFILL(sbi, i) + 1;
 		}
@@ -1183,11 +1182,11 @@ static void count_rsvd(struct inode *inode, ext4_lblk_t lblk, long len,
 
 	/*
 	 * if the current cluster starts on a cluster boundary, count the
-	 * number of whole delonly clusters in the extent
+	 * number of whole delayed clusters in the extent
 	 */
 	if ((i + sbi->s_cluster_ratio - 1) <= end) {
 		nclu = (end - i + 1) >> sbi->s_cluster_bits;
-		rc->ndelonly += nclu;
+		rc->ndelayed += nclu;
 		i += nclu << sbi->s_cluster_bits;
 	}
 
@@ -1247,10 +1246,9 @@ static struct pending_reservation *__pr_tree_search(struct rb_root *root,
  * @rc - pointer to reserved count data
  *
  * The number of reservations to be released is equal to the number of
- * clusters containing delayed and not unwritten (delonly) blocks within
- * the range, minus the number of clusters still containing delonly blocks
- * at the ends of the range, and minus the number of pending reservations
- * within the range.
+ * clusters containing delayed blocks within the range, minus the number of
+ * clusters still containing delayed blocks at the ends of the range, and
+ * minus the number of pending reservations within the range.
  */
 static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
 			     struct extent_status *right_es,
@@ -1261,33 +1259,33 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
 	struct ext4_pending_tree *tree = &EXT4_I(inode)->i_pending_tree;
 	struct rb_node *node;
 	ext4_lblk_t first_lclu, last_lclu;
-	bool left_delonly, right_delonly, count_pending;
+	bool left_delayed, right_delayed, count_pending;
 	struct extent_status *es;
 
 	if (sbi->s_cluster_ratio > 1) {
 		/* count any remaining partial cluster */
 		if (rc->partial)
-			rc->ndelonly++;
+			rc->ndelayed++;
 
-		if (rc->ndelonly == 0)
+		if (rc->ndelayed == 0)
 			return 0;
 
 		first_lclu = EXT4_B2C(sbi, rc->first_do_lblk);
 		last_lclu = EXT4_B2C(sbi, rc->last_do_lblk);
 
 		/*
-		 * decrease the delonly count by the number of clusters at the
-		 * ends of the range that still contain delonly blocks -
+		 * decrease the delayed count by the number of clusters at the
+		 * ends of the range that still contain delayed blocks -
 		 * these clusters still need to be reserved
 		 */
-		left_delonly = right_delonly = false;
+		left_delayed = right_delayed = false;
 
 		es = rc->left_es;
 		while (es && ext4_es_end(es) >=
 		       EXT4_LBLK_CMASK(sbi, rc->first_do_lblk)) {
 			if (ext4_es_is_delayed(es)) {
-				rc->ndelonly--;
-				left_delonly = true;
+				rc->ndelayed--;
+				left_delayed = true;
 				break;
 			}
 			node = rb_prev(&es->rb_node);
@@ -1295,7 +1293,7 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
 				break;
 			es = rb_entry(node, struct extent_status, rb_node);
 		}
-		if (right_es && (!left_delonly || first_lclu != last_lclu)) {
+		if (right_es && (!left_delayed || first_lclu != last_lclu)) {
 			if (end < ext4_es_end(right_es)) {
 				es = right_es;
 			} else {
@@ -1306,8 +1304,8 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
 			while (es && es->es_lblk <=
 			       EXT4_LBLK_CFILL(sbi, rc->last_do_lblk)) {
 				if (ext4_es_is_delayed(es)) {
-					rc->ndelonly--;
-					right_delonly = true;
+					rc->ndelayed--;
+					right_delayed = true;
 					break;
 				}
 				node = rb_next(&es->rb_node);
@@ -1321,21 +1319,21 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
 		/*
 		 * Determine the block range that should be searched for
 		 * pending reservations, if any.  Clusters on the ends of the
-		 * original removed range containing delonly blocks are
+		 * original removed range containing delayed blocks are
 		 * excluded.  They've already been accounted for and it's not
 		 * possible to determine if an associated pending reservation
 		 * should be released with the information available in the
 		 * extents status tree.
 		 */
 		if (first_lclu == last_lclu) {
-			if (left_delonly | right_delonly)
+			if (left_delayed | right_delayed)
 				count_pending = false;
 			else
 				count_pending = true;
 		} else {
-			if (left_delonly)
+			if (left_delayed)
 				first_lclu++;
-			if (right_delonly)
+			if (right_delayed)
 				last_lclu--;
 			if (first_lclu <= last_lclu)
 				count_pending = true;
@@ -1346,13 +1344,13 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
 		/*
 		 * a pending reservation found between first_lclu and last_lclu
 		 * represents an allocated cluster that contained at least one
-		 * delonly block, so the delonly total must be reduced by one
+		 * delayed block, so the delayed total must be reduced by one
 		 * for each pending reservation found and released
 		 */
 		if (count_pending) {
 			pr = __pr_tree_search(&tree->root, first_lclu);
 			while (pr && pr->lclu <= last_lclu) {
-				rc->ndelonly--;
+				rc->ndelayed--;
 				node = rb_next(&pr->rb_node);
 				rb_erase(&pr->rb_node, &tree->root);
 				__free_pending(pr);
@@ -1363,7 +1361,7 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
 			}
 		}
 	}
-	return rc->ndelonly;
+	return rc->ndelayed;
 }
 
 
-- 
2.39.2

Re: [PATCH v3 12/12] ext4: drop all delonly descriptions

[Jan Kara]

On Tue 13-08-24 20:34:52, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> When counting reserved clusters, delayed type is always equal to delonly
> type now, hence drop all delonly descriptions in parameters and
> comments.
> 
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/extents_status.c | 66 +++++++++++++++++++---------------------
>  1 file changed, 32 insertions(+), 34 deletions(-)
> 
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 68c47ecc01a5..c786691dabd3 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -1067,7 +1067,7 @@ int ext4_es_lookup_extent(struct inode *inode, ext4_lblk_t lblk,
>  }
>  
>  struct rsvd_count {
> -	int ndelonly;
> +	int ndelayed;
>  	bool first_do_lblk_found;
>  	ext4_lblk_t first_do_lblk;
>  	ext4_lblk_t last_do_lblk;
> @@ -1093,10 +1093,10 @@ static void init_rsvd(struct inode *inode, ext4_lblk_t lblk,
>  	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
>  	struct rb_node *node;
>  
> -	rc->ndelonly = 0;
> +	rc->ndelayed = 0;
>  
>  	/*
> -	 * for bigalloc, note the first delonly block in the range has not
> +	 * for bigalloc, note the first delayed block in the range has not
>  	 * been found, record the extent containing the block to the left of
>  	 * the region to be removed, if any, and note that there's no partial
>  	 * cluster to track
> @@ -1116,9 +1116,8 @@ static void init_rsvd(struct inode *inode, ext4_lblk_t lblk,
>  }
>  
>  /*
> - * count_rsvd - count the clusters containing delayed and not unwritten
> - *		(delonly) blocks in a range within an extent and add to
> - *	        the running tally in rsvd_count
> + * count_rsvd - count the clusters containing delayed blocks in a range
> + *	        within an extent and add to the running tally in rsvd_count
>   *
>   * @inode - file containing extent
>   * @lblk - first block in range
> @@ -1141,7 +1140,7 @@ static void count_rsvd(struct inode *inode, ext4_lblk_t lblk, long len,
>  	WARN_ON(len <= 0);
>  
>  	if (sbi->s_cluster_ratio == 1) {
> -		rc->ndelonly += (int) len;
> +		rc->ndelayed += (int) len;
>  		return;
>  	}
>  
> @@ -1151,7 +1150,7 @@ static void count_rsvd(struct inode *inode, ext4_lblk_t lblk, long len,
>  	end = lblk + (ext4_lblk_t) len - 1;
>  	end = (end > ext4_es_end(es)) ? ext4_es_end(es) : end;
>  
> -	/* record the first block of the first delonly extent seen */
> +	/* record the first block of the first delayed extent seen */
>  	if (!rc->first_do_lblk_found) {
>  		rc->first_do_lblk = i;
>  		rc->first_do_lblk_found = true;
> @@ -1165,7 +1164,7 @@ static void count_rsvd(struct inode *inode, ext4_lblk_t lblk, long len,
>  	 * doesn't start with it, count it and stop tracking
>  	 */
>  	if (rc->partial && (rc->lclu != EXT4_B2C(sbi, i))) {
> -		rc->ndelonly++;
> +		rc->ndelayed++;
>  		rc->partial = false;
>  	}
>  
> @@ -1175,7 +1174,7 @@ static void count_rsvd(struct inode *inode, ext4_lblk_t lblk, long len,
>  	 */
>  	if (EXT4_LBLK_COFF(sbi, i) != 0) {
>  		if (end >= EXT4_LBLK_CFILL(sbi, i)) {
> -			rc->ndelonly++;
> +			rc->ndelayed++;
>  			rc->partial = false;
>  			i = EXT4_LBLK_CFILL(sbi, i) + 1;
>  		}
> @@ -1183,11 +1182,11 @@ static void count_rsvd(struct inode *inode, ext4_lblk_t lblk, long len,
>  
>  	/*
>  	 * if the current cluster starts on a cluster boundary, count the
> -	 * number of whole delonly clusters in the extent
> +	 * number of whole delayed clusters in the extent
>  	 */
>  	if ((i + sbi->s_cluster_ratio - 1) <= end) {
>  		nclu = (end - i + 1) >> sbi->s_cluster_bits;
> -		rc->ndelonly += nclu;
> +		rc->ndelayed += nclu;
>  		i += nclu << sbi->s_cluster_bits;
>  	}
>  
> @@ -1247,10 +1246,9 @@ static struct pending_reservation *__pr_tree_search(struct rb_root *root,
>   * @rc - pointer to reserved count data
>   *
>   * The number of reservations to be released is equal to the number of
> - * clusters containing delayed and not unwritten (delonly) blocks within
> - * the range, minus the number of clusters still containing delonly blocks
> - * at the ends of the range, and minus the number of pending reservations
> - * within the range.
> + * clusters containing delayed blocks within the range, minus the number of
> + * clusters still containing delayed blocks at the ends of the range, and
> + * minus the number of pending reservations within the range.
>   */
>  static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
>  			     struct extent_status *right_es,
> @@ -1261,33 +1259,33 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
>  	struct ext4_pending_tree *tree = &EXT4_I(inode)->i_pending_tree;
>  	struct rb_node *node;
>  	ext4_lblk_t first_lclu, last_lclu;
> -	bool left_delonly, right_delonly, count_pending;
> +	bool left_delayed, right_delayed, count_pending;
>  	struct extent_status *es;
>  
>  	if (sbi->s_cluster_ratio > 1) {
>  		/* count any remaining partial cluster */
>  		if (rc->partial)
> -			rc->ndelonly++;
> +			rc->ndelayed++;
>  
> -		if (rc->ndelonly == 0)
> +		if (rc->ndelayed == 0)
>  			return 0;
>  
>  		first_lclu = EXT4_B2C(sbi, rc->first_do_lblk);
>  		last_lclu = EXT4_B2C(sbi, rc->last_do_lblk);
>  
>  		/*
> -		 * decrease the delonly count by the number of clusters at the
> -		 * ends of the range that still contain delonly blocks -
> +		 * decrease the delayed count by the number of clusters at the
> +		 * ends of the range that still contain delayed blocks -
>  		 * these clusters still need to be reserved
>  		 */
> -		left_delonly = right_delonly = false;
> +		left_delayed = right_delayed = false;
>  
>  		es = rc->left_es;
>  		while (es && ext4_es_end(es) >=
>  		       EXT4_LBLK_CMASK(sbi, rc->first_do_lblk)) {
>  			if (ext4_es_is_delayed(es)) {
> -				rc->ndelonly--;
> -				left_delonly = true;
> +				rc->ndelayed--;
> +				left_delayed = true;
>  				break;
>  			}
>  			node = rb_prev(&es->rb_node);
> @@ -1295,7 +1293,7 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
>  				break;
>  			es = rb_entry(node, struct extent_status, rb_node);
>  		}
> -		if (right_es && (!left_delonly || first_lclu != last_lclu)) {
> +		if (right_es && (!left_delayed || first_lclu != last_lclu)) {
>  			if (end < ext4_es_end(right_es)) {
>  				es = right_es;
>  			} else {
> @@ -1306,8 +1304,8 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
>  			while (es && es->es_lblk <=
>  			       EXT4_LBLK_CFILL(sbi, rc->last_do_lblk)) {
>  				if (ext4_es_is_delayed(es)) {
> -					rc->ndelonly--;
> -					right_delonly = true;
> +					rc->ndelayed--;
> +					right_delayed = true;
>  					break;
>  				}
>  				node = rb_next(&es->rb_node);
> @@ -1321,21 +1319,21 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
>  		/*
>  		 * Determine the block range that should be searched for
>  		 * pending reservations, if any.  Clusters on the ends of the
> -		 * original removed range containing delonly blocks are
> +		 * original removed range containing delayed blocks are
>  		 * excluded.  They've already been accounted for and it's not
>  		 * possible to determine if an associated pending reservation
>  		 * should be released with the information available in the
>  		 * extents status tree.
>  		 */
>  		if (first_lclu == last_lclu) {
> -			if (left_delonly | right_delonly)
> +			if (left_delayed | right_delayed)
>  				count_pending = false;
>  			else
>  				count_pending = true;
>  		} else {
> -			if (left_delonly)
> +			if (left_delayed)
>  				first_lclu++;
> -			if (right_delonly)
> +			if (right_delayed)
>  				last_lclu--;
>  			if (first_lclu <= last_lclu)
>  				count_pending = true;
> @@ -1346,13 +1344,13 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
>  		/*
>  		 * a pending reservation found between first_lclu and last_lclu
>  		 * represents an allocated cluster that contained at least one
> -		 * delonly block, so the delonly total must be reduced by one
> +		 * delayed block, so the delayed total must be reduced by one
>  		 * for each pending reservation found and released
>  		 */
>  		if (count_pending) {
>  			pr = __pr_tree_search(&tree->root, first_lclu);
>  			while (pr && pr->lclu <= last_lclu) {
> -				rc->ndelonly--;
> +				rc->ndelayed--;
>  				node = rb_next(&pr->rb_node);
>  				rb_erase(&pr->rb_node, &tree->root);
>  				__free_pending(pr);
> @@ -1363,7 +1361,7 @@ static unsigned int get_rsvd(struct inode *inode, ext4_lblk_t end,
>  			}
>  		}
>  	}
> -	return rc->ndelonly;
> +	return rc->ndelayed;
>  }
>  
>  
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v3 00/12] ext4: simplify the counting and management of delalloc reserved blocks

[Theodore Ts'o]

On Tue, 13 Aug 2024 20:34:40 +0800, Zhang Yi wrote:
> Changes since v2:
>  - In patch 3, update the chang log as Jan suggested.
>  - In patch 5 and 6, when moving reserved blocks count updating to
>    ext4_es_insert_extent(), chang the condition for determining quota
>    claim by passing allocation information instead of counting used
>    reserved blocks as Jan suggested.
>  - Add patch 9, drop an unused helper ext4_es_store_status().
>  - Add patch 10, make extent status type exclusive, add assertion and
>    commtents as Jan suggested.
> 
> [...]

Applied, thanks!

[01/12] ext4: factor out ext4_map_create_blocks() to allocate new blocks
        commit: 130078d020e0214809f2e13cf4fb80c646020e94
[02/12] ext4: optimize the EXT4_GET_BLOCKS_DELALLOC_RESERVE flag set
        commit: 8b8252884f2ff4d28e3ce1a825057b3ad2900c35
[03/12] ext4: don't set EXTENT_STATUS_DELAYED on allocated blocks
        commit: eba8c368c8cb9ea05c08caf3dd1a0d0b87d614dc
[04/12] ext4: let __revise_pending() return newly inserted pendings
        commit: fccd632670408ab3066712aa90cc972b18d1b617
[05/12] ext4: passing block allocation information to ext4_es_insert_extent()
        commit: f3baf33b9cae0e00fe1870abca952d5dfea53dc6
[06/12] ext4: update delalloc data reserve spcae in ext4_es_insert_extent()
        commit: c543e2429640293d9eda8c7841d4b5d5e8682826
[07/12] ext4: drop ext4_es_delayed_clu()
        commit: 6e124d5b4b02229f8aaa206b1952db31d1687523
[08/12] ext4: use ext4_map_query_blocks() in ext4_map_blocks()
        commit: 15996a848564e40a3d030ec7e4603dddb9f425b6
[09/12] ext4: drop unused ext4_es_store_status()
        commit: 3b4ba269ab6673d664d2522a0e76797a3550983f
[10/12] ext4: make extent status types exclusive
        commit: ce09036ea4f0a54e9dcd7ba644bb1db7cf2d95d4
[11/12] ext4: drop ext4_es_is_delonly()
        commit: b224b18497484eef9d2dbb3c803888a3f3a3475e
[12/12] ext4: drop all delonly descriptions
        commit: 2046657e64a11b61d5ed07e0d60befd86303125e

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>