-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
zdb: include cloned blocks in block statistics #15123
Conversation
This gives `zdb -b` support for clone blocks. Previously, it didn't know what clones were, so would count their space allocation multiple times and then report leaked space (or, in debug, would assert trying to claim blocks a second time). This commit fixes those bugs, and reports the number of clones and the space "used" (saved) by them. Signed-off-by: Rob Norris <[email protected]> Sponsored-By: OpenDrives Inc. Sponsored-By: Klara Inc.
00ed631
to
36dcb8e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The approach here makes sense to me. When looking the the two zloop
failures we appear to have two problems though:
-
ztest
appears not to have been extended to exercise the BRT code. That's a shame since I'd have expected it to report this leak right away and we would have caught this. We should add at least one test which callszfs_clone_range()
. -
ztest
itself appears to have failed since the instances ran out of free space. That wasn't caused by this PR but we should determine a reasonable way to avoid that prevent these kind of false positive failures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good for me, tanks @robn
@behlendorf I agree on both of the |
This gives `zdb -b` support for clone blocks. Previously, it didn't know what clones were, so would count their space allocation multiple times and then report leaked space (or, in debug, would assert trying to claim blocks a second time). This commit fixes those bugs, and reports the number of clones and the space "used" (saved) by them. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Kay Pedersen <[email protected]> Signed-off-by: Rob Norris <[email protected]> Sponsored-By: OpenDrives Inc. Sponsored-By: Klara Inc. Closes openzfs#15123
This gives `zdb -b` support for clone blocks. Previously, it didn't know what clones were, so would count their space allocation multiple times and then report leaked space (or, in debug, would assert trying to claim blocks a second time). This commit fixes those bugs, and reports the number of clones and the space "used" (saved) by them. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Kay Pedersen <[email protected]> Signed-off-by: Rob Norris <[email protected]> Sponsored-By: OpenDrives Inc. Sponsored-By: Klara Inc. Closes #15123
This gives `zdb -b` support for clone blocks. Previously, it didn't know what clones were, so would count their space allocation multiple times and then report leaked space (or, in debug, would assert trying to claim blocks a second time). This commit fixes those bugs, and reports the number of clones and the space "used" (saved) by them. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Kay Pedersen <[email protected]> Signed-off-by: Rob Norris <[email protected]> Sponsored-By: OpenDrives Inc. Sponsored-By: Klara Inc. Closes openzfs#15123
This gives `zdb -b` support for clone blocks. Previously, it didn't know what clones were, so would count their space allocation multiple times and then report leaked space (or, in debug, would assert trying to claim blocks a second time). This commit fixes those bugs, and reports the number of clones and the space "used" (saved) by them. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Kay Pedersen <[email protected]> Signed-off-by: Rob Norris <[email protected]> Sponsored-By: OpenDrives Inc. Sponsored-By: Klara Inc. Closes openzfs#15123
Motivation and Context
This gives
zdb -b
support for clone blocks.Previously, it didn't know what clones were, so would count their space allocation multiple times and then report leaked space (or, in debug, would assert trying to claim blocks a second time).
This commit fixes those bugs, and reports the number of clones and the space "used" (saved) by them.
Sponsored-By: OpenDrives Inc.
Sponsored-By: Klara Inc.
Description
If the BRT is active, we set up our own tree to track the refcounts for cloned blocks.
The first time we see a block, we look it up in the real BRT. If its there (ie has clones), we record the refcount in our own tree, then count the block as normal.
The second and later time we see the block, we look it up in our own tree. We count the clone, decrement the refcount, and return (so as not to claim the block a second time).
Later, when finalising the stats, we reduce the total allocation on the pool by the total of all cloned blocks (much like we do for dedup blocks). This stops the size being reported as "leaked".
When the last clone is seen, we remove our memory of the block entirely, to try to keep memory usage down.
This adds a function to the BRT proper,
brt_entry_get_refcount
, to lookup the entry and return its refcount.It's worth noting that this could be easily adjusted to check for the BRT refcount being incorrect, however that's not a thing that can happen at the moment (pretty sure) and not quite why I was here, so I didn't bother.
How Has This Been Tested?
By hand:
I don't see any existing tests of
zdb -b
, so I haven't added anything for this. Some tests usezdb -b
but I'm not expecting this to affect their behaviour.Types of changes
Checklist:
Signed-off-by
.