zdb: fix BRT dump #16335
Merged
zdb: fix BRT dump #16335
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BRT refcounts are stored as eight uint8_ts rather than a single uint64_t. This means that za_first_integer is only the first byte, so max 256. This fixes it by doing a lookup for the whole value. Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
|
I'd prefer we actually fix BRT (at this point probably with another feature flag for backward compatibility) to store one integer of 8 bytes rather than 8 integers of one byte. It is trivial to make code compatible on read, repeating read if we receive respective error, while for write we'd use feature flag to decide which way to write it. Aside of being just right it would fix endianness compatibility, which I think is broken now. I've noticed this few month ago, but haven't got to it yet. |
|
I mean, I agree, but unless someone is working on it (I'm not) it'd be good to at least get this merged so it does the right thing. |
tonyhutter
approved these changes
Jul 17, 2024
|
Just looking for one more approval and we can merge it. |
amotin
approved these changes
Jul 18, 2024
lundman
pushed a commit
to openzfsonwindows/openzfs
that referenced
this pull request
Sep 4, 2024
BRT refcounts are stored as eight uint8_ts rather than a single uint64_t. This means that za_first_integer is only the first byte, so max 256. This fixes it by doing a lookup for the whole value. Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Signed-off-by: Rob Norris <rob.norris@klarasystems.com> Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Alexander Motin <mav@FreeBSD.org>
robn
added a commit
to robn/zfs
that referenced
this pull request
Nov 5, 2024
BRT refcounts are stored as eight uint8_ts rather than a single uint64_t. This means that za_first_integer is only the first byte, so max 256. This fixes it by doing a lookup for the whole value. Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Signed-off-by: Rob Norris <rob.norris@klarasystems.com> Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Alexander Motin <mav@FreeBSD.org> (cherry picked from commit aea42e1)
ptr1337
pushed a commit
to CachyOS/zfs
that referenced
this pull request
Nov 14, 2024
BRT refcounts are stored as eight uint8_ts rather than a single uint64_t. This means that za_first_integer is only the first byte, so max 256. This fixes it by doing a lookup for the whole value. Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Signed-off-by: Rob Norris <rob.norris@klarasystems.com> Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[Sponsors: Klara, Inc., Wasabi Technology, Inc.]
Motivation and Context
I was using
zdb -TTTto watch refcount changes as I did some file copies. To my horror, they wrapped from 255 to 0. After I calmed down and start reading the code, it turns out that actuallyzdbwas just holding it slightly wrong. Phew.Description
Slightly weirdly, BRT refcounts are stored as eight
uint8_ts rather than a singleuint64_t. This means thatza_first_integeris only the first byte, so max 256. This fixes it by doing a lookup for the whole value.How Has This Been Tested?
Hand tested on Linux and FreeBSD.
Types of changes
Checklist:
Signed-off-by.