[PATCH v3 02/13] xfs: make use of xfs_calc_buf_res() in xfs_trans.c

[Mark Tinguely]

On 01/25/13 00:19, Jeff Liu wrote:
> On 01/25/2013 05:39 AM, Mark Tinguely wrote:
>> On 01/24/13 05:10, Jeff Liu wrote:
>>> Refine the existing reservations routines with xfs_calc_buf_res() in xfs_trans.c.
>>>
>>> Signed-off-by: Jie Liu<jeff.liu at oracle.com>
>>> ---
>>>    fs/xfs/xfs_trans.c |  238 ++++++++++++++++++++++++----------------------------
>>>    1 file changed, 111 insertions(+), 127 deletions(-)
>>>
>>>    /*
>>> @@ -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, 0))));
>> 					^^^^
>> 	I see the (2 + XFS_IALLOC_BLOCKS(mp) + mp->m_in_maxlevel)
>>           headers in the original code, but I still don't see data.
> XFS_FSB_TO_B(mp, 0) == 0, so it only calculates the headers out without the data part.
>
> But maybe it's better to replace XFS_FSB_TO_B(mp, 0) with 0 directly.
>
>>
>>    I ran the counts for the args of MAX() command and I get a different
>>    result:
>>
>> ARG1   ARG2
>> 25728 218752   before
>> 25728 255616   after - second arg is too big as I expected.
> Strange, there is no 255616 for ARG2 at my old test logs, what's your block/log/inode/sector combination?
>
> I have tried the following combinations on yesterday:
>
> -b size=512 -n size=4096 xfs_calc_itruncate_reservation_MAX(): left=6144       right=81792
>
> -b size=4096 -n size=4096: xfs_calc_itruncate_reservation_MAX(): left=25728	right=251008
>
> -b size=4096 -n size=4096 -i size=2048 xfs_calc_itruncate_reservation_MAX(): left=27520      right=222336
>
> -b size=4096 -n size=65536 -s size=4096 xfs_calc_itruncate_reservation_MAX(): left=25728	right=251008
>
> -b size=4096 -n size=4096 -s size=512 xfs_calc_itruncate_reservation_MAX(): left=25728	right=218752
>
> -b size=4096 -n size=65536 -i size=2048 -s size=512 xfs_calc_itruncate_reservation_MAX(): left=27520      right=222336
>
> -b size=4096 -n size=4096 -s size=4096 xfs_calc_itruncate_reservation_MAX(): left=25728      right=251008
>
> -b size=4096 -n size=65536 xfs_calc_itruncate_reservation_MAX(): left=25728	right=218752
>
>
> Thanks,
> -Jeff
>>
>>
>> Some of the other counts don't match for me either.
>>
>> --Mark.
>>


Sticking a 0 instead of "XFS_FSB_TO_B(mp, 0)" and I get the same
numbers for xfs_calc_itruncate_reservation() before and after the
patch.

Correction to my yesterday email, all the other MAX arguments in the
functions in the file are correct.

I was using an exiting xfs test partition. It was set up as:

meta-data=/dev/sda2   isize=256    agcount=4, agsize=4194432 blks
          =            sectsz=512   attr=2
data     =            bsize=4096   blocks=16777728, imaxpct=25
          =            sunit=0      swidth=0 blks
naming   =version 2   bsize=4096   ascii-ci=0
log      =internal    bsize=4096   blocks=8192, version=2
          =            sectsz=512   sunit=0 blks, lazy-count=1
realtime =none        extsz=4096   blocks=0, rtextents=0

I will manually review the rest of the calculation functions in this
patch and then run tests on differently formatted filesystems.

Thank-you,

--Mark.