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

[Eric Sandeen]

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 at sgi.com>
> 
> _______________________________________________
> xfs mailing list
> xfs at oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>