On 01/10/13 07:47, Jeff Liu wrote:
Start to make use of the new helper to figure out space log reservations for those
transactions which are pre-calculated at mount time in xfs_trans.c.
Signed-off-by: Jie Liu<jeff.liu@xxxxxxxxxx>
---
fs/xfs/xfs_trans.c | 244 ++++++++++++++++++++++++----------------------------
1 file changed, 113 insertions(+), 131 deletions(-)
Wow! Reading this patch makes me appreciate the work you did here and
gets my eyes in shape for Dave's UBER user sync patch.
A question for you, or anyone. When these reservations are made, the
comments talk about specify number of agf/agfl (usually 2 or 3) that
will be dirty in the command.
There are other comments that seem to imply an agf/agfl is reserved for
all AGs and then use the multiplier of 4. Is a specific number of AGs
can be involved in the operation or does it want something like sb_agcount?
I think there a couple error (may be more):
/*
@@ -148,18 +145,18 @@ xfs_calc_itruncate_reservation(
struct xfs_mount *mp)
{
return XFS_DQUOT_LOGRES(mp) +
- MAX((mp->m_sb.sb_inodesize +
- XFS_FSB_TO_B(mp, XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) + 1) +
- 128 * (2 + XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK))),
- (4 * mp->m_sb.sb_sectsize +
- 4 * mp->m_sb.sb_sectsize +
- mp->m_sb.sb_sectsize +
- XFS_ALLOCFREE_LOG_RES(mp, 4) +
- 128 * (9 + XFS_ALLOCFREE_LOG_COUNT(mp, 4)) +
- 128 * 5 +
- XFS_ALLOCFREE_LOG_RES(mp, 1) +
- 128 * (2 + XFS_IALLOC_BLOCKS(mp) + mp->m_in_maxlevels +
- XFS_ALLOCFREE_LOG_COUNT(mp, 1))));
+ MAX((xfs_calc_buf_res(1, mp->m_sb.sb_inodesize) +
+ xfs_calc_buf_res(XFS_BM_MAXLEVELS(mp, XFS_DATA_FORK) + 1,
+ XFS_FSB_TO_B(mp, 1))),
+ (xfs_calc_buf_res(9, mp->m_sb.sb_sectsize) +
+ xfs_calc_buf_res(XFS_ALLOCFREE_LOG_COUNT(mp, 4),
+ XFS_FSB_TO_B(mp, 1)) +
+ xfs_calc_buf_res(5, 0) +
+ xfs_calc_buf_res(XFS_ALLOCFREE_LOG_COUNT(mp, 1),
+ XFS_FSB_TO_B(mp, 1)) +
+ xfs_calc_buf_res(2 + XFS_IALLOC_BLOCKS(mp) +
+ mp->m_in_maxlevels,
+ XFS_FSB_TO_B(mp, 1))));
^^^^^^^^^^^^^^
I think this should be 0. In the
original code, I see the headers being
reserved but not the data.
> /*
> @@ -298,18 +287,18 @@ xfs_calc_create_reservation(
> struct xfs_mount *mp)
> {
> return XFS_DQUOT_LOGRES(mp) +
> - MAX((mp->m_sb.sb_inodesize +
> - mp->m_sb.sb_inodesize +
> - mp->m_sb.sb_sectsize +
> - XFS_FSB_TO_B(mp, 1) +
> - XFS_DIROP_LOG_RES(mp) +
> - 128 * (3 + XFS_DIROP_LOG_COUNT(mp))),
> - (3 * mp->m_sb.sb_sectsize +
> - XFS_FSB_TO_B(mp, XFS_IALLOC_BLOCKS(mp)) +
> - XFS_FSB_TO_B(mp, mp->m_in_maxlevels) +
> - XFS_ALLOCFREE_LOG_RES(mp, 1) +
> - 128 * (2 + XFS_IALLOC_BLOCKS(mp) + mp->m_in_maxlevels +
> - XFS_ALLOCFREE_LOG_COUNT(mp, 1))));
> + MAX((xfs_calc_buf_res(2, mp->m_sb.sb_inodesize) +
> + xfs_calc_buf_res(1, mp->m_sb.sb_sectsize) +
> + xfs_calc_buf_res(0, XFS_FSB_TO_B(mp, 1)) +
^^^^
0 * (128+XFS_FSB_TO_B(mp, 1))?
from the header counts, it appears you meant
no headers, so it would be just:
XFS_FSB_TO_B(mp, 1) +
> + xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
> + XFS_FSB_TO_B(mp, 1))),
> + (xfs_calc_buf_res(3, mp->m_sb.sb_sectsize) +
> + xfs_calc_buf_res(XFS_IALLOC_BLOCKS(mp),
> + XFS_FSB_TO_B(mp, 1)) +
> + xfs_calc_buf_res(mp->m_in_maxlevels,
> + XFS_FSB_TO_B(mp, 1)) +
> + xfs_calc_buf_res(XFS_ALLOCFREE_LOG_COUNT(mp, 1),
> + XFS_FSB_TO_B(mp, 1))));
> }
> /*
> @@ -337,16 +326,15 @@ xfs_calc_ifree_reservation(
> struct xfs_mount *mp)
> {
> return XFS_DQUOT_LOGRES(mp) +
> - mp->m_sb.sb_inodesize +
> - mp->m_sb.sb_sectsize +
> - mp->m_sb.sb_sectsize +
> - XFS_FSB_TO_B(mp, 1) +
> - MAX((__uint16_t)XFS_FSB_TO_B(mp, 1),
> - XFS_INODE_CLUSTER_SIZE(mp)) +
^^^ end of MAX() 5th header is
is the single item in MAX
> - 128 * 5 +
> - XFS_ALLOCFREE_LOG_RES(mp, 1) +
> - 128 * (2 + XFS_IALLOC_BLOCKS(mp) + mp->m_in_maxlevels +
> - XFS_ALLOCFREE_LOG_COUNT(mp, 1));
> + xfs_calc_buf_res(1, mp->m_sb.sb_inodesize) +
> + xfs_calc_buf_res(2, mp->m_sb.sb_sectsize) +
> + xfs_calc_buf_res(1, XFS_FSB_TO_B(mp, 1)) +
> + MAX(xfs_calc_buf_res(1, XFS_FSB_TO_B(mp, 1)),
> + XFS_INODE_CLUSTER_SIZE(mp) +
^^^^^ MAX should end here ^^
> + xfs_calc_buf_res(2 + XFS_IALLOC_BLOCKS(mp) +
> + mp->m_in_maxlevels, 0) +
> + xfs_calc_buf_res(XFS_ALLOCFREE_LOG_COUNT(mp, 1),
> + XFS_FSB_TO_B(mp, 1)));
> }
>
/*
@@ -337,16 +326,15 @@ xfs_calc_ifree_reservation(
struct xfs_mount *mp)
{
return XFS_DQUOT_LOGRES(mp) +
- mp->m_sb.sb_inodesize +
- mp->m_sb.sb_sectsize +
- mp->m_sb.sb_sectsize +
- XFS_FSB_TO_B(mp, 1) +
- MAX((__uint16_t)XFS_FSB_TO_B(mp, 1),
^^^^
- XFS_INODE_CLUSTER_SIZE(mp)) +
- 128 * 5 +
- XFS_ALLOCFREE_LOG_RES(mp, 1) +
- 128 * (2 + XFS_IALLOC_BLOCKS(mp) + mp->m_in_maxlevels +
- XFS_ALLOCFREE_LOG_COUNT(mp, 1));
+ xfs_calc_buf_res(1, mp->m_sb.sb_inodesize) +
+ xfs_calc_buf_res(2, mp->m_sb.sb_sectsize) +
+ xfs_calc_buf_res(1, XFS_FSB_TO_B(mp, 1)) +
+ MAX(xfs_calc_buf_res(1, XFS_FSB_TO_B(mp, 1)),
^^^ has extra header added it.
+ XFS_INODE_CLUSTER_SIZE(mp) +
+ xfs_calc_buf_res(2 + XFS_IALLOC_BLOCKS(mp) +
+ mp->m_in_maxlevels, 0) +
+ xfs_calc_buf_res(XFS_ALLOCFREE_LOG_COUNT(mp, 1),
+ XFS_FSB_TO_B(mp, 1)));
}
/*
I will have to go through this patch again and also test prints before
and after the patch.
Before the patch:
write 108216 itrnc 219064 renam 305976 link 153144 remov 153144 symlk
158520 creat 157880
mkdir 157880 ifree 57912 ichng 1592 grwdt 44160 swrit 384 wrtid 384
addfk 69560
atriv 174720 attst 22456 attrr 87992 clagi 640 gwall 65024 grezr 4224
gwrfr 5760
After the patch:
write 108216 itrnc 255928 renam 305976 link 153144 remov 153144 symlk
158520 creat 153784
mkdir 153784 ifree 57784 ichng 1592 grwdt 44160 swrit 384 wrtid 384
addfk 69560
atriv 174720 attst 22456 attrr 87992 clagi 640 gwall 65024 grezr 4224
gwrfr 5760
I plan to test on both sides of the routine's MAX() command too.
----
BTW, besides reusing the same superblock reservation and the commit
widths, the rest of the series looks good to me.
--Mark.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
