xfs
[Top] [All Lists]

Re: [PATCH] xfs: recalculate leaf entry pointer after compacting a dir2

To: Mark Tinguely <tinguely@xxxxxxx>
Subject: Re: [PATCH] xfs: recalculate leaf entry pointer after compacting a dir2 block
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Fri, 11 Jan 2013 20:58:08 -0600
Cc: Eric Sandeen <sandeen@xxxxxxxxxx>, Dave Jones <davej@xxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>, Dave Chinner <dchinner@xxxxxxxxxx>
In-reply-to: <50F08E26.1040306@xxxxxxx>
References: <50EEEF4C.8040801@xxxxxxxxxx> <20130110224709.GK3120@dastard> <50F08E26.1040306@xxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130107 Thunderbird/17.0.2
On 1/11/13 4:11 PM, Mark Tinguely wrote:
> On 01/10/13 16:47, Dave Chinner wrote:
>> On Thu, Jan 10, 2013 at 10:41:48AM -0600, Eric Sandeen wrote:
>>> Dave Jones hit this assert when doing a compile on recent git, with
>>> CONFIG_XFS_DEBUG enabled:
>>>
>>> XFS: Assertion failed: (char *)dup - (char *)hdr == 
>>> be16_to_cpu(*xfs_dir2_data_unused_tag_p(dup)), file: 
>>> fs/xfs/xfs_dir2_data.c, line: 828
>>>
>>> Upon further digging, the tag found by xfs_dir2_data_unused_tag_p(dup)
>>> contained "2" and not the proper offset, and I found that this value was
>>> changed after the memmoves under "Use a stale leaf for our new entry."
>>> in xfs_dir2_block_addname(), i.e.
>>>
>>>                          memmove(&blp[mid + 1],&blp[mid],
>>>                                  (highstale - mid) * sizeof(*blp));
>>>
>>> overwrote it.
>>>
>>> What has happened is that the previous call to xfs_dir2_block_compact()
>>> has rearranged things; it changes btp->count as well as the
>>> blp array.  So after we make that call, we must recalculate the
>>> proper pointer to the leaf entries by making another call to
>>> xfs_dir2_block_leaf_p().
>>
>> Perhaps a bit of ascii art to explain it for other reviewers to save
>> them having to spend a long time trying to understand the dir2 code?
>>
>> Pre-compact:
>>
>>   dup    +   unused data pointer - size 72 bytes at offset 3560
>>     |
>>     |
>>   blp    +   leaf entry [0] - good - at offset 3632
>>     +   leaf entry [1] - good
>>     +   leaf entry [2] - stale
>>     +   leaf entry [3] - good
>>     +   leaf entry [4] - good
>>     +   leaf entry [5] - stale
>>     +   leaf entry [6] - good
>>     .....
>>     +   leaf entry [n] - stale
>>     +   leaf entry [n+1] - good
>>     +   block tail - points to offset 3632
>>
>> Post compact (removes all but the highest stale leaf entry, adds the
>> free space to the last unused data region):
>>
>>   dup    +   unused data pointer - size 112 bytes at offset 3560
>>     |
>>     |
>>   blp    |   points into unused region, not to start of leaf array
>>     |
>>     |
>>     +   leaf entry [0] - good - at offset 3672
>>     +   leaf entry [1] - good
>>     +   leaf entry [2] - good
>>     +   leaf entry [3] - good
>>     .....
>>     +   leaf entry [n] - stale
>>     +   leaf entry [n+1] - good
>>     +   block tail - points to offset 3672
>>
>> So as you can see, blp needs to be updated to point at offset 3672...
> 
> Thanks for the study aid - it did help explain the xfs_dir2_block_compact() 
> code.

Feel free to add that to the commit . . .

Thanks,
-Eric

> Reviewed-by: Mark Tinguely <tinguely@xxxxxxx>
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
> 

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