[PATCH v1] libxfs: fix reservation space for removing transaction

[PATCH v1] libxfs: fix reservation space for removing transaction

[Xiaole He]

In libxfs/xfs_trans_resv.c:

/* libxfs/xfs_trans_resv.c begin */
 1 /*
 2  * For removing a directory entry we can modify:
 3  *    the parent directory inode: inode size
 4  *    the removed inode: inode size
 5  *    the directory btree could join: (max depth + v2) * dir block size
 6  *    the directory bmap btree could join or split: (max depth + v2) * blocksize
 7  * And the bmap_finish transaction can free the dir and bmap blocks giving:
 8  *    the agf for the ag in which the blocks live: 2 * sector size
 9  *    the agfl for the ag in which the blocks live: 2 * sector size
10  *    the superblock for the free block count: sector size
11  ...
12  */
13 STATIC uint
14 xfs_calc_remove_reservation(
15      struct xfs_mount        *mp)
16 {
17      return XFS_DQUOT_LOGRES(mp) +
18              xfs_calc_iunlink_add_reservation(mp) +
19              max((xfs_calc_inode_res(mp, 1) +
20                   xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
21                                    XFS_FSB_TO_B(mp, 1))),
22                  (xfs_calc_buf_res(4, mp->m_sb.sb_sectsize) +
23      ...
24 }
/* libxfs/xfs_trans_resv.c end */

Above lines 8-10 indicates there has 5 sector size of space to be
reserved, but the above line 22 only reserve 4 sector size of space,
this patch fix the problem and sorry for not notice this problem at
Commit d3e53ab7cdc7fabb8c94137e335634e0ed4691e8 ("xfs: fix inode
reservation space for removing transaction").

Signed-off-by: Xiaole He <hexiaole@kylinos.cn>
---
 libxfs/xfs_trans_resv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libxfs/xfs_trans_resv.c b/libxfs/xfs_trans_resv.c
index 04c44480..3d106c77 100644
--- a/libxfs/xfs_trans_resv.c
+++ b/libxfs/xfs_trans_resv.c
@@ -517,7 +517,7 @@ xfs_calc_remove_reservation(
 		max((xfs_calc_inode_res(mp, 2) +
 		     xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
 				      XFS_FSB_TO_B(mp, 1))),
-		    (xfs_calc_buf_res(4, mp->m_sb.sb_sectsize) +
+		    (xfs_calc_buf_res(5, mp->m_sb.sb_sectsize) +
 		     xfs_calc_buf_res(xfs_allocfree_block_count(mp, 2),
 				      XFS_FSB_TO_B(mp, 1))));
 }
-- 
2.27.0

Re: [PATCH v1] libxfs: fix reservation space for removing transaction

[Darrick J. Wong]

On Mon, Feb 06, 2023 at 09:09:49PM +0800, Xiaole He wrote:
> In libxfs/xfs_trans_resv.c:
> 
> /* libxfs/xfs_trans_resv.c begin */
>  1 /*
>  2  * For removing a directory entry we can modify:
>  3  *    the parent directory inode: inode size
>  4  *    the removed inode: inode size
>  5  *    the directory btree could join: (max depth + v2) * dir block size
>  6  *    the directory bmap btree could join or split: (max depth + v2) * blocksize
>  7  * And the bmap_finish transaction can free the dir and bmap blocks giving:
>  8  *    the agf for the ag in which the blocks live: 2 * sector size
>  9  *    the agfl for the ag in which the blocks live: 2 * sector size
> 10  *    the superblock for the free block count: sector size
> 11  ...
> 12  */
> 13 STATIC uint
> 14 xfs_calc_remove_reservation(
> 15      struct xfs_mount        *mp)
> 16 {
> 17      return XFS_DQUOT_LOGRES(mp) +
> 18              xfs_calc_iunlink_add_reservation(mp) +
> 19              max((xfs_calc_inode_res(mp, 1) +

1?  It's 2 currently.  Are you using an old version of xfsprogs?

> 20                   xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
> 21                                    XFS_FSB_TO_B(mp, 1))),
> 22                  (xfs_calc_buf_res(4, mp->m_sb.sb_sectsize) +
> 23      ...
> 24 }
> /* libxfs/xfs_trans_resv.c end */
> 
> Above lines 8-10 indicates there has 5 sector size of space to be
> reserved, but the above line 22 only reserve 4 sector size of space,
> this patch fix the problem and sorry for not notice this problem at
> Commit d3e53ab7cdc7fabb8c94137e335634e0ed4691e8 ("xfs: fix inode
> reservation space for removing transaction").
> 
> Signed-off-by: Xiaole He <hexiaole@kylinos.cn>
> ---
>  libxfs/xfs_trans_resv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libxfs/xfs_trans_resv.c b/libxfs/xfs_trans_resv.c
> index 04c44480..3d106c77 100644
> --- a/libxfs/xfs_trans_resv.c
> +++ b/libxfs/xfs_trans_resv.c
> @@ -517,7 +517,7 @@ xfs_calc_remove_reservation(
>  		max((xfs_calc_inode_res(mp, 2) +

Looks fine, but why the discrepancy between here and the commit message?

>  		     xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
>  				      XFS_FSB_TO_B(mp, 1))),
> -		    (xfs_calc_buf_res(4, mp->m_sb.sb_sectsize) +
> +		    (xfs_calc_buf_res(5, mp->m_sb.sb_sectsize) +

Also: please send patches against the kernel, not xfsprogs.

--D

>  		     xfs_calc_buf_res(xfs_allocfree_block_count(mp, 2),
>  				      XFS_FSB_TO_B(mp, 1))));
>  }
> -- 
> 2.27.0
>