[PATCH v2 00/12] xfs: kill hard-coded number 128 for transaction space log reservation

[Mark Tinguely]

On 01/22/13 00:52, Jeff Liu wrote:
> On 01/19/2013 08:20 AM, Dave Chinner wrote:
>> On Fri, Jan 18, 2013 at 03:40:52PM -0600, Mark Tinguely wrote:
>>> On 01/10/13 07:47, Jeff Liu wrote:
>>>> Hello,
>>>>
>>>> Here is the v2 patch set of killing hard-coded number 128 which is used to indicate
>>>> the extra log space reservation for almost all of those transactions.
>>>>
>>>> In this round, I also tried to convert some transactions to pre-calculate out the
>>>> space log reservation from runtime to mount time so that we can make the code related
>>>> to xfs_trans_reserve() looks a bit neat and reduce a bit performance overhead(basically
>>>> can be ignored. :)) IMHO, which were includes: super block quota flags changes,
>>>> quota off/end of quota off, adjust quota limits, quota allocations, log dummy1,
>>>> log super block counters, log super block units/fields, as well as set attributes.
>>>>
>>>>
>>>> Changes of v2 to v1:
>>>> - use xfs_calc_buf_res() to calulate out the space log reservation per item.
>>>>
>>>> Old patches:
>>>> v1:
>>>> http://www.spinics.net/lists/xfs/msg15499.html
>>>>
>>>> Thanks,
>>>> -Jeff
>>>>
>>>> _______________________________________________
>>>> xfs mailing list
>>>> xfs at oss.sgi.com
>>>> http://oss.sgi.com/mailman/listinfo/xfs
>>>
>>>
>>> Hi Jeff,
>>>
>>> I did a quick read of the series and it looks good.
>>>
>>> Any reason to have separate constants for:
>>>
>>>   XFS_SYNC_ICSBCOUNT_LOG_RES(mp)
>>>   XFS_SYNC_ICSBUNIT_LOG_RES(mp)
>>>   XFS_LOG_DUMMY1_LOG_RES(mp)
>>>
>>> since they are the same value and are all superblock operations.
>>
>> Right - they can all use the same "XFS_SB_LOG_RES(mp)" reservation.
>>
>> FWIW, using the notiation "ICSB" is wrong here. ICSB is short for
>> "in-core superblock" (i.e. in memory) but transactions are used for
>> modifying the on-disk superblock. They are two separate things, so
>> let's make sure we get the terminology right. ;)
> Since those transactions are used for changing the on-disk super block,
> how about naming this transaction to XFS_UPDATE_SB_LOG_RES(mp) and
> introduce a corresponding mp->m_reservations.tr_updatesb?
> Does this sounds more meaningful?
>
> Thanks,
> -Jeff
>

I like the simpler "XFS_SB_LOG_RES(mp)". It follows the existing names.

--Mark.