Remove a number of #if 0's.
Remove the redundant thread yield after implementing the same or better
in LockSet.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Mutex locks are released and acquired unfairly, causing arbitrary delays
in acquiring locks. This prevents threads from releasing subvol FD's
which in turn blocks subvol deletes.
Fix by implementing a priority queue in LockSet to ensure fairness.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
When an extent ref is modified, all of the refs in the same metadata
page get the same transid in the TREE_SEARCH_V2 header. All of the
extents are rescanned by later subvol scans. This causes up to 80%
overhead due to redundant reads of the same extents.
A proper fix for this requires extent-based scanning instead of
extent-ref-based scanning. Until that happens, filter out new references
to old extents.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
BEESNOTE puts a message on the status message stack. BEESINFO logs a
message with rate limiting. The message that was flooding the logs
was coming from BEESINFO not BEESNOTE.
Fix earlier commit which removed the wrong message.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
After a few hundred subvol threads start running, the inode cache starts
to thrash, and the log gets spammed with messages of the form:
"open_root_nocache <subvolid>: <path>"
Ideally there would be some way to schedule work to minimize inode
thrashing. Until that gets done, just silence the messages for now.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
After a few hundred subvol threads start running, the inode cache starts
to thrash, and the log gets spammed with messages of the form:
"open_root_nocache <subvolid>: <path>"
Ideally there would be some way to schedule work to minimize inode
thrashing. Until that gets done, just silence the messages for now.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
With many threads it is inconvenient to reassemble the elided parts of
the dedup src/dst and scan filenames output. Simply output them
unconditionally, and balance the line lengths.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Tune the concurrency model to work a little better with large numbers
of subvols. This is much less than the full rewrite Bees desparately
needs, but it provides a marginal improvement until the new code is ready.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
With many threads it is inconvenient to reassemble the elided parts of
the dedup src/dst and scan filenames output. Simply output them
unconditionally, and balance the line lengths.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This code has been #if 0 for a long time, and it seems unlikely it
will ever be useful in the future.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
If we lose a race and open the wrong file, we will not retry with the
next path if the file we opened had incompatible flags. We need to keep
trying paths until we open the correct file or run out of paths.
Fix by moving the inode flag check after the checks for file identity.
Output attributes in hex to be consistent with other attribute error
messages.
There is no need to report root and file paths separately in the error
message for incompatible flags because we have confirmed the identity of
the file before the incompatible flag error is detected. Other messages
in this loop still output root path and file_path separately because
the identity of 'rv' is unknown at the time these messages are emitted.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
If we lose a race and open the wrong file, we will not retry with the
next path if the file we opened had incompatible flags. We need to keep
trying paths until we open the correct file or run out of paths.
Fix by moving the inode flag check after the checks for file identity.
Output attributes in hex to be consistent with other attribute error
messages.
There is no need to report root and file paths separately in the error
message for incompatible flags because we have confirmed the identity of
the file before the incompatible flag error is detected. Other messages
in this loop still output root path and file_path separately because
the identity of 'rv' is unknown at the time these messages are emitted.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Keep track of the locking thread so we can see why we are deadlocked
in gdb.
Use a handle type for locks based on shared_ptr. Change the handle type
name to flush out any non-auto local variables.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
(cherry picked from commit aa0b22d445)
This helps identify causes of the "same physical address in dedup"
exception.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
(cherry picked from commit cc7b4f22b5)
BLOCK_SIZE_MIN_EXTENT_DEFRAG, BLOCK_SIZE_MIN_EXTENT_SPLIT, and others
are no longer used. Remove them.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
(cherry picked from commit a3d7032eda)
Add time spent in file create and copy operations to the stats.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
(cherry picked from commit f01c20f972)
A BEESTRACE closure could throw an exception. Trap those so we don't
end up in terminate().
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
(cherry picked from commit 59660cfc00)
Reads can block indefinitely due to bugs, low io priority, or poor
storage performance. Record the block origin data in the thread state
so we can see which reads are problematic.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
(cherry picked from commit f56f736d28)
Use () instead of [] when the respective end of the byte range touches
the beginning or end of the file. Also omit the '0' at beginning of
file.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
(cherry picked from commit 3023b7f57a)
Use a different character to make it easier to search for bytenr ranges
in the logs.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
(cherry picked from commit d43199e3d6)
This will allow the default size limit for cache objects to be changed
with impunity.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
(cherry picked from commit 9daa51edaa)
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 e509210428)
BLOCK_SIZE_MIN_EXTENT_DEFRAG, BLOCK_SIZE_MIN_EXTENT_SPLIT, and others
are no longer used. Remove them.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Reads can block indefinitely due to bugs, low io priority, or poor
storage performance. Record the block origin data in the thread state
so we can see which reads are problematic.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Use () instead of [] when the respective end of the byte range touches
the beginning or end of the file. Also omit the '0' at beginning of
file.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
All testing so far incidates more crawlers go faster up to a limit
much larger than btrfs's performance limitations on subvols, even on
spinning rust. Remove the artificial constraint.
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>
We were holding weak refs until the next time the resource ID was used.
This is a bad thing if resource IDs are sparse (e.g. pointers or hashes)
because we'll never see an ID twice.
To fix, determine whether we released the last instance of a resource,
and if so, free its weak ref immediately.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Keep track of the locking thread so we can see why we are deadlocked
in gdb.
Use a handle type for locks based on shared_ptr. Change the handle type
name to flush out any non-auto local variables.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Holding file FDs open for long periods of time delays inode destruction.
For very large files this can lead to excessive delays while bees dedups
data that will cease to be reachable.
Use the same workaround for file FDs (in the root_ino cache) that
is used for subvols (in the root cache): forcibly close all cached
FDs at regular intervals. The FD cache will reacquire FDs from files
that still have existing paths, and will abandon FDs from files that
no longer have existing paths. The non-existing-path case is not new
(bees has always been able to discover deleted inodes) so it is already
handled by existing code.
Fixes: https://github.com/Zygo/bees/issues/18
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Interesting things happen when blindly swapping the release-build CCFLAGS
with the debug-build commented-out CCFLAGS. None of these things that
happen are good.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The bugs in other parts of the code have been identified and fixed,
so the overprotective locks around shared_ptr can be removed.
Keep the other improvements to the Resource class.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This might improve performance on systems with more than 3 CPU cores...or
it might bring such a machine to its knees.
TODO: find out which of those two things happens.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Dedup was spending a lot of time waiting for the ioctl mutex while it
was held by non-dedup ioctls; however, when dedup finally locked the
mutex, its average run time was comparatively short and the variance
was low.
With the various workarounds and kernel fixes in place, FILE_EXTENT_SAME
and bees play well enough together that we can allow multiple threads
to do dedup at the same time. The extent bytenr lockset should be
sufficient to prevent undesirable results (i.e. dup data not removed,
or deadlocks on old kernels).
Remove the ioctl lock on dedup.
LOGICAL_INO and SEARCH_V2 (as used by BeesCrawl) remain under the ioctl
mutex because they can still have abitrarily large run times.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This prevents two threads from attempting to dispose of the same physical
extent at the same time. This is a more precise exclusion than the
general lock on all tmpfiles.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
BeesRoots::crawl_state_erase may invoke BeesCrawl::~BeesCrawl, which
will do a join on its crawl thread, which might be trying to lock
BeesRoots::m_mutex, which is locked by crawl_state_erase at the time.
Fix this by creating an extra reference to the BeesCrawl object, then
releasing the lock on BeesRoots::m_mutex, then deleting the reference.
The BeesCrawl object may still call methods on BeesRoots, but the only
such method is BeesRoots::crawl_state_set_dirty, and that method has
no dependency on the erased BeesCrawl shared_ptr.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This is yet another multi-threaded Bees experiment.
This time we are dividing the work by subvol: one thread is created to
process each subvol in the filesystem. There is no change in behavior
on filesystems containing only one subvol.
In order to avoid or mitigate the impact of kernel bugs and performance
issues, the btrfs ioctls FILE_EXTENT_SAME, SEARCH_V2, and LOGICAL_INO are
serialized. Only one thread may execute any of these ioctls at any time.
All three ioctls share a single lock.
In order to simplify the implementation, only one thread is permitted to
create a temporary file during one call to scan_one_extent. This prevents
multiple threads from racing to replace the same physical extent with
separate physical copies.
The single "crawl" thread is replaced by one "crawl_<root_number>"
for each subvol.
The crawl size is reduced from 4096 items to 1024. This reduces the
memory requirement per subvol and keeps the data in memory fresher.
It also increases the number of log messages, so turn some of them off.
TODO: Currently there is no configurable limit on the total number
of threads. The number of CPUs is used as an upper bound on the number
of active threads; however, we still have one thread per subvol even if
all most of the threads do is wait for locks.
TODO: Some of the single-threaded code is left behind until I make up
my mind about whether this experiment is successful.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
check_overflow() will invalidate iterators if it decides there are too
many cache entries.
If items are deleted from the cache, search for the inserted item again
to ensure the iterator is valid.
Increase size of timestamp to size_t.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Some whitespace fixes. Remove some duplicate code. Don't lock
two BeesStats objects in the - operator method.
Get the locking for T& at(const K&) right to avoid locking a mutex
recursively. Make the non-const version of the function private.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Before:
unique_lock<mutex> lock(some_mutex);
// run lock.~unique_lock() because return
// return reference to unprotected heap
return foo[bar];
After:
unique_lock<mutex> lock(some_mutex);
// make copy of object on heap protected by mutex lock
auto tmp_copy = foo[bar];
// run lock.~unique_lock() because return
// pass locally allocated object to copy constructor
return tmp_copy;
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Before:
unique_lock<mutex> lock(some_mutex);
// run lock.~unique_lock() because return
// return reference to unprotected heap
return foo[bar];
After:
unique_lock<mutex> lock(some_mutex);
// make copy of object on heap protected by mutex lock
auto tmp_copy = foo[bar];
// run lock.~unique_lock() because return
// pass locally allocated object to copy constructor
return tmp_copy;
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
If we release the lock first (and C++ destructor order says we do), then
the return value will be constructed from data living in an unprotected
container object. That data might be destroyed before we get to the
copy constructor for the return value.
Make a temporary copy of the return value that won't be destroyed by any
other thread, then unlock the mutex, then return the copy object.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Get rid of the ResourceHolder class.
Fix GCC static template member instantiation issues.
Replace assert() with exceptions.
shared_ptr can't seem to do reference counting in a multi-threaded
environment. The code looks correct (for both ResourceHandle and
std::shared_ptr); however, continual segfaults don't lie.
Carpet-bomb with mutex locks to reduce the likelihood of losing shared_ptr
races.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Found by valgrind. It was mostly harmless because the range of
usable values is limited by m_burst (which was initialized) and 0.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
"s_name" was a thread_local variable, not static, and did not require a
mutex to protect access. A deadlock is possible if a thread triggers an
exception with a handler that attempts to log a message (as the top-level
exception handler in bees does).
Remove multiple unnecessary mutex locks. Rename the thread_local variables
to make their scope clearer.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Add MADV_DONTDUMP to the list of advice flags.
There are now three flags which may or may not be supported by the
target kernel. Try each one and log its success or failure separately.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
The hash table statistics calculation in BeesHashTable::prefetch_loop
and the data-driven operation of the extent scanner always pulls the
hash table into RAM as fast as the disk will push the data. We never
use the prefetch rate limit, so remove it.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
This fixes a bug where bees tries to process itself as a btrfs filesystem.
This is a species of bug that I only notice *after* pushing to a public
git repo.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Extend the LockSet class so that the total number of locked (active)
items can be limited. When the limit is reached, no new items can be
locked until some existing locked items are unlocked.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Every git commit was causing bees.cc and bees-hash.cc to be rebuilt,
which was expensive and unnecessary.
Signed-off-by: Zygo Blaxell <bees@furryterror.org>
Zygo Blaxell