xfs
[Top] [All Lists]

Re: [PATCH v2 00/12] xfs: kill hard-coded number 128 for transaction spa

To: Mark Tinguely <tinguely@xxxxxxx>
Subject: Re: [PATCH v2 00/12] xfs: kill hard-coded number 128 for transaction space log reservation
From: Jeff Liu <jeff.liu@xxxxxxxxxx>
Date: Sat, 19 Jan 2013 11:43:16 +0800
Cc: Dave Chinner <david@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130119002017.GS2498@dastard>
References: <50EEC680.9040903@xxxxxxxxxx> <50F9C164.2050806@xxxxxxx> <20130119002017.GS2498@dastard>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121028 Thunderbird/16.0.2
Hi Mark and Dave,

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@xxxxxxxxxxx
>>> 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.
I am intending to make the code looks a bit clear by introducing those separate 
definitions. :)
> 
> 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. ;)
Yes, how about if we can add a new XFS_SB_LOG_RES(mp) and call it in those 
transactions directly?

Thanks,
-Jeff

<Prev in Thread] Current Thread [Next in Thread>