Fix the missing symbols that popped up when adding chunk tree to
lib/fs.cc. Also define the missing symbols instead of merely trying to
avoid them.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This allows plugging in an ostream at run time so that we can audit all
the search calls we are doing.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Enable use of the ioctl to probe whether two fds refer to the same btrfs,
without throwing an exception.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Commit 72c3bf8438830b65cae7bdaff126053e562280e5 ("fs: handle ENOENT
within lib") was meant to prevent exceptions when a subvol is deleted.
If the search ioctl fails, the kernel won't set nr_items in the
ioctl output, which means `nr_items` still has the input value. When
ENOENT is detected, `this->nr_items` is set to 0, then later `*this =
ioctl_ptr->key` overwrites `this->nr_items` with the original requested
number of items.
This replaced the ENOENT exception with an exception triggered by
interpreting garbage in the memory buffer. The number of exceptions
was reduced because the memory buffers are frequently reused, but upper
layers would then reject the data or ignore it because it didn't match
the key range.
Fix by setting `ioctl_ptr->key.nr_items`, which then overwrites
`this->nr_items`, so the loop that extracts items from the ioctl data
gets the right number of items (i.e. zero).
Fixes: 72c3bf8438830b65cae7bdaff126053e562280e5 ("fs: handle ENOENT within lib")
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This prevents the storms of exceptions that occur when a subvol is
deleted. We simply treat the entire tree as if it was empty.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The kernel has not required a 16 MiB limit on dedupe requests since
v4.18-rc1 b67287682688 ("Btrfs: dedupe_file_range ioctl: remove 16MiB
restriction").
Kernels before v4.18 would truncate the request and return the size
actually deduped in `bytes_deduped`. Kernel v4.18 and later will loop
in the kernel until the entire request is satisfied (although still
in 16 MiB chunks, so larger extents will be split).
Modify the loop in userspace to measure the size the kernel actually
deduped, instead of assuming the kernel will only accept 16 MiB.
On current kernels this will always loop exactly once.
Since we now rely on `bytes_deduped`, make sure it has a sane value.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Apparently reinterpret_cast<uint64_t> sign-extends 32-bit pointers.
This is OK when running on a 32-bit kernel that will truncate the pointer
to 32 bits, but when running on a 64-bit kernel, the extra bits are
interpreted as part of the (now very invalid) address.
Use <uintptr_t> instead, which is unsigned, integer, and the same word
size as the arch's pointer type. Ordinary numeric conversion can take
it from there, filling the rest of the word with zeros.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Some malloc implementations will try to mmap() and munmap() large buffers
every time they are used, causing a severe loss of performance.
Nothing ever overrode the virtual methods, and there was no virtual
destructor, so they cause compiler warnings at build time when used with
a template that tries to delete pointers to them.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Another instance of the pattern where we derived a crucible class
from a btrfs struct. Make it an automatic variable instead.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
We only use BtrfsExtentInfo when it's exactly equivalent to the
base, so drop the derived class.
While we're here, fix BtrfsExtentSame::add so it uses a btrfs-compatible
uint64_t instead of an off_t.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
We already had a function that was _similar_, so add decoding for compress
type NONE, give it a less specific name, and declare it in fs.h.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
In commit 14ce81c08 "fs: get rid of silly base class that causes build
failures now" I neglected to set the dest_count field in the ioctl
arg structure, so bees master hasn't been deduping anything for about
three weeks.
I'd put a THROW_CHECK in here to catch this kind of bug in the future,
but it would be placed at exactly the point where this fix is.
Fixes: 14ce81c08
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Yet another build failure of the form:
error: flexible array member fiemap... not at end of struct crucible::Fiemap...
bees doesn't use fiemap any more, so the fixes here are minimal changes
to make it build, not shining examples of C++ class design.
Signer-off-by: Zygo Blaxell <bees@furryterror.org>
This fixes another build failure of the form:
error: flexible array member btrfs_... not at end of struct crucible::Btrfs...
Fixes: https://github.com/Zygo/bees/issues/236
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The base class thing was an ugly way to get around the lack of C99
compound literals in C++, and also to make the bare ioctls usable with
the derived classes.
Today, both clang and gcc have C99 compound literals, so there's no need
to do crazy things with memset. We never used the derived classes for
ioctls, and for this specific ioctl it would have been a very, very bad
idea, so there's no need to support that either. We do need to jump
through hoops for ostream& operator<<() but we had to do those anyway
as there are other members in the derived type.
So we can simply drop the base class, and build the args object on the
stack in `do_ioctl`. This also removes the need to verify initialization.
There's no bug here since the `info` member of the base class was
never used in place by the derived class, but new compilers reject the
flexible array member in the base class because the derived class makes
`info` be not at the end of the struct any more:
error: flexible array member btrfs_ioctl_same_args::info not at end of struct crucible::BtrfsExtentSame
Fixes: https://github.com/Zygo/bees/issues/232
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Sprinkle in some asserts to make sure compilers aren't getting creative.
This may introduce a new compiler dependency, as I suspect older versions
of GCC don't support this syntax.
It definitely needs a new compiler flag to suppress a warning when some
fields are not explicitly initialized. If we've omitted a field, it's
because it's a field we don't know (or care) about, and we want that
thing initialized to zero.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
When we are searching the btrfs metadata trees, we usually want only
one type of item. If the last item in a search result is not of the
desired type, we can restart the search at the next possible key with
that item type, potentially skipping over some uninteresting items we
would otherwise have to fetch, process, and discard.
Also remove a bug in the previous next_min code that would skip over
items if the offset overflowed and the next objectid in the tree had a
lower item type number than the previous objectid. This doesn't seem
to be a bug that has ever happened, as it would require a file to roll
over in the offset field.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Switch various methods in fs to use ByteVector to cut down on the number
of slow allocations and copies.
Automatically determine the correct size for TREE_SEARCH_V2 buffers
based on the number of items requested, and grow the buffer as needed.
This eliminates the need to cache some objects that were heavy to create.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The assignment operator will use member-wise assignment, which
assumes the object's this pointer is aligned. That doesn't
happen when the object in question is part of a btrfs search
result, and aarch64 faults over it.
Use memcpy instead, which has no alignment constraints.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The weird things distros do to the path where uuid.h gets installed
have broken bees builds for the last time.
We were only using uuid to support a legacy feature that was removed
over four years ago.
Hypothetical users who are upgrading directly from bees v0.1 should
probably restart all the crawlers anyway--there were bugs. Also, if any
such users exist, I respect their tremendous patience with the horrible
performance all these years--bees got about 30x faster since v0.1.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The Linux kernel's btrfs headers are better than the libbtrfs-dev headers:
- the libbtrfs-dev headers have C++ language compatibility issues
- upstream version in Linux kernel is more accurate and up to date
- macros in libbtrfs-dev's ctree.h hide information that would
enable bees to perform runtime buffer length checking
- enum types whose presence cannot be detected with #ifdef
When accessing members of metadata items from the filesystem, we want
to verify that the member we are accessing is within the boundaries of
the item that was retrieved; otherwise, a memory access violation may
occur or garbage may be returned to the caller. A simple C++ template,
given a pointer to a structure member and a buffer, can determine that
the buffer contains enough bytes to safely access a struct member.
This was implemented back in 2016, but left unused due to ctree.h issues.
Some btrfs metadata structures have variable length despite using a
fixed-size in-memory structure. The members that appear earliest in
the structure contain information about which following members of the
structure are used. The item stored in the filesystem is truncated after
the last used member, and all following members must not be accessed.
'btrfs_stack_*' accessor macros obscure the memory boundaries of the
members they access, which makes it impossible for a C++ template to
verify the memory access. If the template checks the length of the
entire structure, it will find an access violation for variable-length
metadata items because the item is rarely large enough for the entire
structure.
Get rid of all the libbtrfs-dev accessor macros and reimplement them
with the necessary buffer length checks.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Use uint8_t when we mean uint8_t, i.e. vector<uint8_t> instead of
vector<char>.
Add a template parameter instead of vector so we can swap in a
non-copying data type.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Define a local copy of the header that has fields for the csum type
and length, so we can build in places that haven't caught up to kernel
5.5 headers yet.
The reason why the csum type and length are not unconditionally filled
in eludes me. csum_length is necessarily non-zero, and the cost of
the conditional is worse than the cost of the copy, so the whole flags
dance is a WTF...but it's part of the kernel API now, so it's too late
to NAK it.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Perf blames this operator for >1% of instructions with -O2, and
70% of instructions without -O2.
Let the compiler inline the function.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The requested size may not match the final size of the container,
so consistently use the container's size after prepare(), not the
requested size.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
It is not possible to emulate extent-same by clone in a safe way.
EXTENT_SAME has been supported in btrfs since kernel 3.13, which
is much too old to contemplate running bees on.
Remove this dangerous and unused function.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Make sure the result set is empty before running the ioctl in case
something tries to consume the result without checking the error status.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
If we are not zero-filling containers then the overhead of allocating them
on each use is negligible. The effect that the thread_local containers
were having on RAM usage was very non-negligible.
Use dynamic containers (members or stack objects) for better control
of object lifetimes and much lower peak RAM usage. They're a tiny bit
faster, too.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Automatically fall back to LOGICAL_INO if LOGICAL_INO_V2 fails and no
_V2 flags are used.
Add methods to set the flags argument with build portability to older
headers.
Use thread_local storage for the somewhat large buffers used by
LOGICAL_INO_V2 (and other users of BtrfsDataContainer like INO_PATHS).
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The memset is just doing an assignment from one dereferenced pointer to
another, so do an assignment to keep GCC 8 happy.
Fixes: https://github.com/Zygo/bees/issues/64
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
perf blames the SEARCH_V2 ioctl wrapper for a lot of time spent in malloc.
Use a thread_local buffer for ioctl results, and reuse it between runs.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
(cherry picked from commit e509210428951e645d33916694a17aed1950991d)
This gets rid of some more big memsets. It may replace them
with a lot of tiny mallocs, though. If this turns out to be
a bad idea then at least we can easily revert the change.
We really do need some large buffers for BtrfsIoctlSearchKey in some
cases, but we don't need to zero them out first. Don't do that so we
save some CPU.
Reduce the default buffer size to 4K because most BISK users don't get
need much more than 1K. Set the buffer size explicitly to the product of
the number of items and the desired item size in the places that really
need a lot of items.
I accidentally did a pre-push verification on a 32-bit build host.
There were a surprisingly small number of problems, so fix them.
Bees now builds on a 32-bit host. Let's not update README just yet,
though: the 32-bit ioctl support fails immediately after startup on a
64-bit kernel.
