* Re: Possible small bug in xfsprogs-dev/db/metadump.c [not found] <46b8a8850909271220w372d60c3s18a543ed00825082@mail.gmail.com> @ 2009-09-28 17:21 ` Christoph Hellwig 2009-09-28 17:36 ` Richard Sharpe 0 siblings, 1 reply; 5+ messages in thread From: Christoph Hellwig @ 2009-09-28 17:21 UTC (permalink / raw) To: Richard Sharpe; +Cc: xfs [Cc'ed to the list, where people including the most active person on the userspace side hang out] On Sun, Sep 27, 2009 at 12:20:33PM -0700, Richard Sharpe wrote: > Hi folks, > > There seems to be a small bug in > xfsprogs-dev/db/metadump.c:scanfunc_freesp (although I think the same > problem exists in other functions). > > It has a check to see if the number of records is invalid: > > 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; > } > > However, it seems to me that you should pay attention to bb_level in > the node when using that test, because leaf nodes can appear at > multiple levels in the tree. Before that code there is a if (level == 0) return 1; which should take care of the leaf nodes by exiting early. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Possible small bug in xfsprogs-dev/db/metadump.c 2009-09-28 17:21 ` Possible small bug in xfsprogs-dev/db/metadump.c Christoph Hellwig @ 2009-09-28 17:36 ` Richard Sharpe 2009-09-29 13:00 ` Christoph Hellwig 0 siblings, 1 reply; 5+ messages in thread From: Richard Sharpe @ 2009-09-28 17:36 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Mon, Sep 28, 2009 at 10:21 AM, Christoph Hellwig <hch@infradead.org> wrote: > [Cc'ed to the list, where people including the most active person on the > userspace side hang out] > > On Sun, Sep 27, 2009 at 12:20:33PM -0700, Richard Sharpe wrote: >> Hi folks, >> >> There seems to be a small bug in >> xfsprogs-dev/db/metadump.c:scanfunc_freesp (although I think the same >> problem exists in other functions). >> >> It has a check to see if the number of records is invalid: >> >> 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; >> } >> >> However, it seems to me that you should pay attention to bb_level in >> the node when using that test, because leaf nodes can appear at >> multiple levels in the tree. > > Before that code there is a > > if (level == 0) > return 1; > > which should take care of the leaf nodes by exiting early. 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. 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). -- Regards, Richard Sharpe _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Possible small bug in xfsprogs-dev/db/metadump.c 2009-09-28 17:36 ` Richard Sharpe @ 2009-09-29 13:00 ` Christoph Hellwig 2009-09-29 15:45 ` Richard Sharpe 0 siblings, 1 reply; 5+ messages in thread From: Christoph Hellwig @ 2009-09-29 13:00 UTC (permalink / raw) To: Richard Sharpe; +Cc: Christoph Hellwig, xfs On Mon, Sep 28, 2009 at 10:36:13AM -0700, Richard Sharpe wrote: > > > > ? ? ? ?if (level == 0) > > ? ? ? ? ? ? ? ?return 1; > > > > which should take care of the leaf nodes by exiting early. > > 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. > 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. > > > > > > -- > Regards, > Richard Sharpe > > _______________________________________________ > xfs mailing list > xfs@oss.sgi.com > http://oss.sgi.com/mailman/listinfo/xfs ---end quoted text--- _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Possible small bug in xfsprogs-dev/db/metadump.c 2009-09-29 13:00 ` Christoph Hellwig @ 2009-09-29 15:45 ` Richard Sharpe 2009-09-30 0:16 ` Dave Chinner 0 siblings, 1 reply; 5+ messages in thread From: Richard Sharpe @ 2009-09-29 15:45 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Tue, Sep 29, 2009 at 6:00 AM, Christoph Hellwig <hch@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 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Possible small bug in xfsprogs-dev/db/metadump.c 2009-09-29 15:45 ` Richard Sharpe @ 2009-09-30 0:16 ` Dave Chinner 0 siblings, 0 replies; 5+ messages in thread From: Dave Chinner @ 2009-09-30 0:16 UTC (permalink / raw) To: Richard Sharpe; +Cc: Christoph Hellwig, xfs On Tue, Sep 29, 2009 at 08:45:56AM -0700, Richard Sharpe wrote: > 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. No, that is not possible. By definition, a consistent filesystem image has all leaf nodes at level zero. > 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). This sounds to me like the log has not been replayed on this filesystem. AFAICT, when looking at a raw disk image of an XFS filesystem, the only way to get leaf nodes at non-zero levels is to have a dirty log. i.e. the log contains allocation/free transactions that have resulted in a multi-level rebalance of the tree that have not been replayed and written to disk and hence on-disk image of the tree is unbalanced. When the log is replayed, the on disk image will get updated and the tree will appear balanced with all leaves at level 0. > 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 think he was aware of the structure. ;) It seems to me that you are trying to use the wrong tool to walk free space trees and interpret the number of extents. xfs_metadump is intended to capture the exact layout of the filesystem metadata so that it can be reproduced exactly in a different environment. It was not intended as a method of interpreting the potentially inconsistent metadata that it records. xfs_db does what you are trying to do. It already has commands that walk the per AG free space trees and tells you the number of extents, gives an extent size histogram, etc.... Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-09-30 0:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <46b8a8850909271220w372d60c3s18a543ed00825082@mail.gmail.com>
2009-09-28 17:21 ` Possible small bug in xfsprogs-dev/db/metadump.c Christoph Hellwig
2009-09-28 17:36 ` Richard Sharpe
2009-09-29 13:00 ` Christoph Hellwig
2009-09-29 15:45 ` Richard Sharpe
2009-09-30 0:16 ` Dave Chinner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox