Now that tempfiles are using pool checkin functions to control their
size, we don't need a size limit in realign().
We keep the limit in make_copy because it's a sanity check against
letting a multi-terabyte copy operation slip through.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Get rid of the thread-local TempFiles and use Pool instead. This
eliminates a potential FD leak when the loadavg governor repeatedly
creates and destroys threads.
With the old per-thread TempFiles, we were guaranteed to have exclusive
ownership of the TempFile object within the current thread. Pool is
somewhat stricter: it only guarantees ownership while the checked-out
Handle exists. Adjust the users of TempFile objects to ensure they hold
the Handle object until they are finished using the TempFile.
It appears that maintaining large, heavily-reflinked, long-lived temporary
files costs more than truncating after every use: btrfs has to write
multiple references to the temporary file's extents, then some commits
later, remove references as the temporary file is deleted or truncated.
Using the temporary file in a dedupe operation flushes the data to disk,
so nothing is saved by pretending that there is writeback pipelining and
trying to avoid flushes in truncate. Pool provides usage tracking and
a checkin callback, so use it to truncate the temporary file immediately
after every use.
Redesign TempFile so that every instance creates exactly one Fd which
persists over the lifetime of the TempFile object. Provide a reset()
method which resets the file back to the initial state and call it from
the Pool checkin callback. This makes TempFile's lifetime equivalent to
its Fd's lifetime, which simplifies interactions with FdCache and Roots.
This change means we can now blacklist temporary files without having
an effective memory leak, so do that. We also have a reason to ever
remove something from the blacklist, so add a method for that too.
In order to move to extent-centric addressing, we need to be able to
reliably open temporary files by root and inode number. Previously we
would place TempFile fd's into the cache with insert_root_ino, but the
cache would be cleared periodically, and it would not be possible to
reopen temporary files after that happened. Now that the TempFile's
lifetime is the same as the TempFile Fd's lifetime, we can have TempFile
manage a separate FileId -> Fd map in Roots which is unaffected by the
periodic cache clearing. BeesRoots::open_root_ino_nocache will check
this map before attempting to open the file via btrfs root+ino lookup,
and return it through the cache as if Roots had opened the file via btrfs.
Hold a reference to BeesRoots in BeesTempFile because the usual way
to get such a reference now throws an exception in BeesTempFile's
destructor.
These changes make method BeesTempFile::create() and all methods named
insert_root_ino unnecessary, so delete them.
We construct and destroy TempFiles much less often now, so make their
constructor and destructor more informative.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Pool is a place to store shared_ptrs to generated objects (T) that are
too expensive to create and destroy between individual uses, such as
temporary files. Objects in a Pool have no distinct identity
(contrast with Cache or NamedPtr).
Users of the Pool invoke the Pool function call overload and "check out"
a shared_ptr<T> for a T object from the Pool. When the last referencing
shared_otr<T> is destroyed, the T object is "checked in" to the Pool.
Each call of the Pool function overload checks out a shared_ptr<T> to a T
object that is not currently referenced by any other public shared_ptr<T>.
If there are no existing T objects in the Pool, a new T is constructed
by calling the generator function.
The clear() method destroys all checked in T objects owned by the Pool
at the time the method is called. T objects that are checked out are
not affected by clear(), and they will be stored in the Pool when they
are checked in.
If the checkout function is provided, it is called on a shared_ptr<T>
during checkout, before returning to the caller.
If the checkin function is provided, it is called on a shared_ptr<T>
before returning it to the Pool. The checkin function must not throw
exceptions.
The Pool may be destroyed while T objects are checked out of the Pool.
In that case, when the T objects are checked in, the T object is
immediately destroyed without calling the checkin function.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
A prealloc extent reference can be deduped immediately and asynchronously.
There is no need to slow down extent scanning to do it.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
I was never able to prove a connection between fsync() and deadlock bugs.
There were too many deadlock bugs to be able to isolate a bug that is
triggered specifically by fsync.
Update the comment (which has been unchanged since kernel 4.14). We still
may want to do fsync() on temporary files someday, but there's a full
internal API rewrite between here and there.
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>
Number of items should be low enough that we don't have too many stale
items, but high enough to amortize system call overhead to a reasonable
ratio.
Number of bytes should be constant: one worst-case metadata page (the
btrfs limit is 64K, though 16K is much more common) so that we always
have enough space for one worst-case item; otherwise, we get EOVERFLOW
if we set the number of items too low and there's a big item in the tree,
and we can't make further progress.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
There are lots of ways the search can fail, but it's hard to pick one
without knowing the parameters.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
It's a pain to read, edit, and format large blocks of text in C++ code,
so rip the usage message out of bees.cc and put it in a plain text file.
Use a minimal translator to convert it into a C string.
While we're here, remove the multiple roots feature from the command
line synopsis, as we don't really support it any more. Also clarify
that "id 5" is "subvol id 5", and describe in one sentence what
workaround-btrfs-send does.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Silence the unused variable warning. The compiler is correct, but we
may implement line-level debug at some point in the future, so we
want to keep the member and parameters.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Remove unused function getenv_or_die. All of our environment variable
parameters are optional or have default values.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Get rid of unused template instantiation.
Drop the unused realtime signals from the ntoa table. If in the future
we really need to solve clang's issue with them, we'll address it then.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
A long time ago, when bees used dedicated threads to scan each subvol, the
calculation of the "dedup_unique_bytes" statistic was still wrong.
This stat can only be calculated when dedupe runs on extent data items
instead of extent reference items. Remove the stat variable until
that happens.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
There was a 4th tree mod log crash that showed up in testing. It can
be reproduced or eliminated by applying or reverting d2311e698578
("btrfs: relocation: Delay reloc tree deletion after merge_reloc_roots")
to a 5.4.x kernel before 5.4.54.
Unfortunately, the test can only run if several other patches that
fixed other bugs in d2311e698578 are applied or removed at the same time.
Commit d2311e698578 introduces a bug which destroys filesystems under test
long before tree mod log failures can be reproduced in testing. One of
those patches also fixes tree mod log issue #4. I do not know which one,
but since kernels after 5.1 cannot run without all of those patches, I do
not think it matters.
Tree mod issue #4 is the reason why the tree mod workaround is still
required on all kernels before 5.4. The issue still exists on older
LTS kernels, e.g. 4.9.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Rewrite the text related to 'btrfs send' to clarify that the send
workaround is no longer necessary to avoid kernel crashes, but still
useful because send and dedupe still do not work at the same time.
Replace "many backref code changes" with a specific commit reference,
and improve the grammar of some issue descriptions.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Apparently there's Github Flavored Markdown, and there's the markup
language that github uses, and they are distinct things.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Present known kernel bugs in table form with issue descriptions,
fixed and broken kernel versions, and references to fixes.
Update kernel version recommendations to include information on kernel
versions up to 5.8.14.
Reduce emphasis on data corruption bugs which are 1) two or more
years old now, and 2) much less bad than the bugs in kernel 5.1.
Add deprecation warning for kernels before 4.15.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Prefer to use cmark-gfm with extension 'table' so we can use tables in
locally-generated HTML files. If cmark-gfm is not installed then
fall back to some other Markdown implemeentation, but the tables will
be broken on every other implementation I have tried so far.
Also make the HTML output depend on the Makefile, since there may be
document translation options specified there (like '-e table' or an
entirely different Markdown implementation).
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
uncaught_exception() had only the one valid use case, and it can be
reimplemented by literally calling current_exception() instead.
current_exception() has several valid use cases, so it is not likely
to be deprecated any time soon.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
We cannot use BeesContext::roots() until after
BeesContext::set_root_path() has been called.
Save up the parameter settings until then.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
"Storm of softlockups" starts with a simple BUG_ON, but after the
BUG_ON, all cores that are waiting on spinlocks get stuck.
The _first_ kernel call trace is required to identify the bug.
At least two such bugs have been identified.
Add some notes about the conflict between LOGICAL_INO and balance,
and the recently added bees workaround.
Update the gotchas page for balances to point to the kernel bugs page.
Remove "bees and the full balance will both work correctly" as that
statement is not true.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This avoids some kernel bugs. One of them is fixed in 5.3.4 and later:
efad8a853a "Btrfs: fix use-after-free when using the tree modification log"
There are apparently others in current kernels, so for now just put bees
on pause until the balance is done.
At some point we may want to provide an option to disable this
workaround; however, running bees and balance at the same time makes
neither particularly fast, so maybe we'll just leave it this way.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Saying just "This feature" at some log levels could be puzzling. Let's
remove this message, the feature works without problems for a year.
Signed-off-by: Kai Krakow <kai@kaishome.de>
In version 2.30 glibc added it's own gettid() function. This resulted in
"error: call of overloaded ‘gettid()’ is ambiguous" because gettid()
now exists in both namespace crucible and std.
For now, use explicit references to namespace crucible. This continues
to work with new and old libc without having to test specific library
versions.
At some point, glibc gettid() will be deployed widely enough that we can
remove the crucible version entirely.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Localize the hash function in bees to a single spot to make it easier
to change later (or at runtime).
Remove some code that was using a property of CRC as an optimization.
The optimization doesn't work for other hash functions, and running the
CRC function takes more CPU time than the optimization saved.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
CityHash64 appears to be the fastest available block hashing algorithm
that is good enough for dedupe. It takes much less CPU than the CRC64
function, and avoids hash-collision problems with file formats that use
CRC64 as an integrity check on 4K block boundaries.
Extracted from git://github.com/google/cityhash with the "CRC" hash
functions (which require Intel/AMD CPU support) removed. We don't
need those, and they introduce a new (if only theoretical) build-time
dependency.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
We got away with this because GCC 4.8 (and apparently every GCC prior
to 9) didn't notice or care, and because there is nothing referenced
inside the lambda function body that isn't accessible from any other
kind of function body (i.e. the capture wasn't needed at all).
GCC 9 now enforces what the C++ standard said all along: there is
no need to allow capture-default in this case, so it is not.
Fix by removing the offending capture-default.
Fixes: https://github.com/Zygo/bees/issues/112
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>
We are getting a lot of exceptions when an inline extent is too large for the
TREE_SEARCH_V2 buffer. This disrupts ExtentWalker's extent boundary
search when there is an inline extent at the beginning of a file:
# fiemap foo
Log 0x0..0x1000 Phy 0x0..0x1000 Flags FIEMAP_EXTENT_NOT_ALIGNED|FIEMAP_EXTENT_DATA_INLINE
Log 0x1000..0x2000 Phy 0x7307f9000..0x7307fa000 Flags 0
Log 0x2000..0x3000 Phy 0x731078000..0x731079000 Flags 0
Log 0x3000..0x5000 Phy 0x73127d000..0x73127f000 Flags FIEMAP_EXTENT_ENCODED
Log 0x5000..0x6000 Phy 0x73137a000..0x73137b000 Flags 0
Log 0x6000..0x7000 Phy 0x731683000..0x731684000 Flags 0
Log 0x7000..0x8000 Phy 0x73224f000..0x732250000 Flags 0
Log 0x8000..0x9000 Phy 0x7323c9000..0x7323ca000 Flags 0
Log 0x9000..0xb000 Phy 0x732425000..0x732427000 Flags FIEMAP_EXTENT_ENCODED
Log 0xb000..0xc000 Phy 0x732598000..0x732599000 Flags 0
Log 0xc000..0xd000 Phy 0x7325d5000..0x7325d6000 Flags FIEMAP_EXTENT_LAST
# fiewalk foo
exception type std::system_error: BTRFS_IOC_TREE_SEARCH_V2: /tmp/foo at fs.cc:844: Value too large for defined data type
Normally crawlers simply skip over inline extents, but ExtentWalker will
seek backward from the first non-inline extent to confirm that it has
an accurate starting block for the target extent. This fails when it
encounters the first inline extent.
strace reveals that buffer size is too small for the first extent,
as seen here:
ioctl(3, BTRFS_IOC_TREE_SEARCH_V2, {key={tree_id=258, min_objectid=78897856, max_objectid=UINT64_MAX, min_offset=0, max_offset=UINT64_MAX, min_transid=0, max_transid=UINT64_MAX, min_type=BTRFS_EXTENT_DATA_KEY, max_type=BTRFS_EXTENT_DATA_KEY, nr_items=16}, buf_size=1360} => {buf_size=1418}) = -1 EOVERFLOW (Value too large for defined data type)
Fix this by increasing the buffer size until it can handle the largest
possible object on the largest possible btrfs metadata page (65536 bytes).
BtrfsExtentWalker already has optimizations to minimize the allocation
cost, so we don't need any changes there.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Some build environments (ARM? AARCH64?) do not have the fields
si_lower and si_upper in siginfo.
bees doesn't need them, so don't try to access them.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Update the version ranges on the dependencies.
FIXME/TODO: start dropping early versions that don't work with current
code?
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
flushoncommit or not-flushoncommit isn't really a bees matter--it's
a sysadmin's tradeoff between reliability and performance. bees does
not affect that tradeoff because all dedupe src extents are flushed, so
bees introduces no *new* data loss risks in the noflushoncommit
case--i.e. any data that you could lose while running bees, you'd also
lose when not running bees.
Note that the converse is not true: bees might trigger flushing on
data that would not normally have been flushed with noflushoncommit,
and improve data integrity after a crash as a side-effect of dedupe
operations. The risks of noflushoncommit might be reduced by running
bees. I don't have evidence based on experimental data to support that
conclusion, so I'll just leave this possibility as a rumor in a commit
log message.
lvmcache can be moved from the "bad" list to the "good" list now.
bcache remains in the "bad" list due to some non-data-losing failures
that only seem to happen with bcache.
Add a note about CPUs with strange endianness or page sizes, as nobody
seems to have tried those.
Remove "at great cost" from the btrfs send workaround. The cost is
the cost, there is no need to editorialize.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The existence of information about known data corruption bugs should be
visible from the top-level page.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
* comprehensive list of kernels with bees-triggered corruption bug fixes
* deadlock between dedupe and rename is now fixed (in some places)
* compressed data corruption is now fixed (in more places)
* btrfs send fix for one bug is now merged in 5.2-rc1, another bug remains
* retired the bcache/lvmcache bug (can't reproduce those bugs any more,
although I *can* reproduce an interesting non-destructive bcache bug)
* new minor bug entries for two harmless kernel warnings
* new entry for storm-of-soft-lockups
Fixes: https://github.com/Zygo/bees/issues/107
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This is especially useful when dynamic load management allocates more
worker threads than active tasks, so the extra threads are effectively
invisible.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This may help users understand some of the things that happen inside
bees...or it may just be horribly long and confusing.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Enable much simpler Task management: each time a Task needs to be done
at least once in the future, simply invoke the run() method on the Task.
The Task will ensure that it only runs once, only appears in a queue
once, and will run again if a run request is made while the Task is
already running.
Make the queue policy a member of the Task rather than a method. This
enables Tasks to reschedule themselves, possibly on the appropriate queue
if we have more than one of those some day.
This happens to make Tasks more similar to Linux kernel workers.
This similarity is coincidental, but not undesirable.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
