xfs
[Top] [All Lists]

Re: [PATCH] xfs: get new buffer for secondary superblocks from the first

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfs: get new buffer for secondary superblocks from the first new AG
From: Jeff Liu <jeff.liu@xxxxxxxxxx>
Date: Thu, 21 Mar 2013 14:27:34 +0800
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130321054428.GN17758@dastard>
References: <514A88C3.2030204@xxxxxxxxxx> <20130321054428.GN17758@dastard>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121028 Thunderbird/16.0.2
On 03/21/2013 01:44 PM, Dave Chinner wrote:
> On Thu, Mar 21, 2013 at 12:12:51PM +0800, Jeff Liu wrote:
>> In growfs operation, the unused parts of superblocks in the latest old AG are
>> zero filled, hence we can safely read the secondary superblock buffer of it
>> rather than getting a new buffer for it again.
> 
> From the code:
> 
>       oagcount = mp->m_sb.sb_agcount;
> 
> And because the AG indexes are zero numbered, the index of the
> last original AGs is (oagcount - 1). Hence we shoul donly be reading
> from the AG headers if the current AG index is less than oagcount.
> 
>> This is a small refinement for commits 1375cb65.
>>
>> Cc: Dave Chinner <david@xxxxxxxxxxxxx>
>> Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx>
>> ---
>>  fs/xfs/xfs_fsops.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
>> index 94eaeed..7d435dd 100644
>> --- a/fs/xfs/xfs_fsops.c
>> +++ b/fs/xfs/xfs_fsops.c
>> @@ -449,7 +449,7 @@ xfs_growfs_data_private(
>>               * disk as the contents of the new area we are growing into is
>>               * completely unknown.
>>               */
>> -            if (agno < oagcount) {
>> +            if (agno <= oagcount) {
> 
> With this change, when agno == oagcount we are one AG beyond the
> original filesystem size and so if we read that space, we are
> reading uninitialised data.
> 
> AFAICT, the current code is correct...
Oops, I made a stupid mistake.  The index of the
last original AGs is (oagcount - 1) rather than oagcount...

Sorry for the noise!

Thanks,
-Jeff

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