Possible small bug in xfsprogs-dev/db/metadump.c

[Richard Sharpe]

On Tue, Sep 29, 2009 at 6:00 AM, Christoph Hellwig <hch at infradead.org> wrote:
> On Mon, Sep 28, 2009 at 10:36:13AM -0700, Richard Sharpe wrote:
>> Well, yes there is, but that is the problem I encountered. It is level
>> as passed in when starting at the top of the tree, which is obtained
>> from the levels value in the AGF, and is decremented by one on each
>> recursion:
>>
>>         if (!(*func)(iocur_top->data, agno, agbno, level - 1, btype, arg))
>>
>> However, what should really be looked at is the value bb_level in the
>> header in each free-space Btree node.
>
> I think bother are equally valid.  The one passed in fro mthe top should
> always be right while the ondisk one might be corrupted.

Well, yes, but then anything could be corrupted on disk, including the
superblock and the pointers in interior nodes in the free space Btree.

>> After I made that change to my changes, I started being able to
>> properly count all leaf nodes and free extents, and the numbers came
>> out where I expected them to be (instead of not seeing many leaf nodes
>> and vastly undercounting free extents).
>
> Still not quite understanding your problem here.  Do you have a tool
> of your own that doesn't start from the btree root but only walks
> subtrees?  In that case you need to look at the on-disk level unless
> you can find out easily what level you start at.  But I don't think we
> need this for the existing tools that always walk from the root.

No, I was walking down from the root of the tree.

However, now I understand what is going on.

Assume a free space tree with levels = 3 (from the AGF). However, not
all leaf nodes will be at depth 3 in the tree, some will be at depth 2
in the tree.

However, in scanfunc_freesp we see:

        if (level == 0)
                return 1;

        numrecs = be16_to_cpu(block->bb_numrecs);
        if (numrecs > mp->m_alloc_mxr[1]) {
                if (show_warnings)
                        print_warning("invalid numrecs (%u) in %s block %u/%u",
                                numrecs, typtab[btype].name, agno, agbno);
                return 1;
        }

If we hit level == 0 then we exit, however, for a leaf node that is at
depth 2 (level 1, but bb_level == 0) we will see numrecs >
mp->m_alloc_mxr[1] and will also skip such records (ie, will not
recurse into them).

However, if the user does "metadump -w" they will see warnings that
are bogus and suggests that the author was not really aware of the
real structure of the tree.

I do think it is a minor issue since it will not lead to dropping any
records as I first feared.

Since I wanted to count the number of free extents in the free-space
trees I needed to actually see all leaf nodes, and thus it became a
problem for me.

-- 
Regards,
Richard Sharpe