diff mbox series

[RFC,01/16] ext4: correct the start block of counting reserved clusters

Message ID 20230824092619.1327976-2-yi.zhang@huaweicloud.com
State New
Headers show
Series ext4: more accurate metadata reservaion for delalloc mount option | expand

Commit Message

Zhang Yi Aug. 24, 2023, 9:26 a.m. UTC 
From: Zhang Yi <yi.zhang@huawei.com>

When big allocate feature is enabled, we need to count and update
reserved clusters before removing a delayed only extent_status entry.
{init|count|get}_rsvd() have already done this, but the start block
number of this counting isn's correct in the following case.

  lblk            end
   |               |
   v               v
          -------------------------
          |                       | orig_es
          -------------------------
                   ^              ^
      len1 is 0    |     len2     |

If the start block of the orig_es entry founded is bigger than lblk, we
passed lblk as start block to count_rsvd(), but the length is correct,
finally, the range to be counted is offset. This patch fix this by
passing the start blocks to 'orig_es->lblk + len1'.

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

Comments

Jan Kara Aug. 30, 2023, 1:10 p.m. UTC | #1 
On Thu 24-08-23 17:26:04, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> When big allocate feature is enabled, we need to count and update
> reserved clusters before removing a delayed only extent_status entry.
> {init|count|get}_rsvd() have already done this, but the start block
> number of this counting isn's correct in the following case.
> 
>   lblk            end
>    |               |
>    v               v
>           -------------------------
>           |                       | orig_es
>           -------------------------
>                    ^              ^
>       len1 is 0    |     len2     |
> 
> If the start block of the orig_es entry founded is bigger than lblk, we
> passed lblk as start block to count_rsvd(), but the length is correct,
> finally, the range to be counted is offset. This patch fix this by
> passing the start blocks to 'orig_es->lblk + len1'.
> 
> 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 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index 6f7de14c0fa8..5e625ea4545d 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -1405,8 +1405,8 @@ static int __es_remove_extent(struct inode *inode, ext4_lblk_t lblk,
>  			}
>  		}
>  		if (count_reserved)
> -			count_rsvd(inode, lblk, orig_es.es_len - len1 - len2,
> -				   &orig_es, &rc);
> +			count_rsvd(inode, orig_es.es_lblk + len1,
> +				   orig_es.es_len - len1 - len2, &orig_es, &rc);
>  		goto out_get_reserved;
>  	}
>  
> -- 
> 2.39.2
>
Theodore Ts'o Oct. 6, 2023, 2:33 a.m. UTC | #2 
On Wed, Aug 30, 2023 at 03:10:31PM +0200, Jan Kara wrote:
> On Thu 24-08-23 17:26:04, Zhang Yi wrote:
> > From: Zhang Yi <yi.zhang@huawei.com>
> > 
> > When big allocate feature is enabled, we need to count and update
> > reserved clusters before removing a delayed only extent_status entry.
> > {init|count|get}_rsvd() have already done this, but the start block
> > number of this counting isn's correct in the following case.
> > 
> >   lblk            end
> >    |               |
> >    v               v
> >           -------------------------
> >           |                       | orig_es
> >           -------------------------
> >                    ^              ^
> >       len1 is 0    |     len2     |
> > 
> > If the start block of the orig_es entry founded is bigger than lblk, we
> > passed lblk as start block to count_rsvd(), but the length is correct,
> > finally, the range to be counted is offset. This patch fix this by
> > passing the start blocks to 'orig_es->lblk + len1'.
> > 
> > Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> 
> Looks good. Feel free to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks, I've applied the first two patches in this series, since these
are bug fixes.  The rest of the patch series requires more analysis
and review.

						- Ted
diff mbox series

Patch

diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index 6f7de14c0fa8..5e625ea4545d 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -1405,8 +1405,8 @@  static int __es_remove_extent(struct inode *inode, ext4_lblk_t lblk,
 			}
 		}
 		if (count_reserved)
-			count_rsvd(inode, lblk, orig_es.es_len - len1 - len2,
-				   &orig_es, &rc);
+			count_rsvd(inode, orig_es.es_lblk + len1,
+				   orig_es.es_len - len1 - len2, &orig_es, &rc);
 		goto out_get_reserved;
 	}