In percpu reader mode, trylock() for read had a lost wakeup: on failure
to get the lock, we may have caused a writer to fail to get the lock,
because we temporarily elevated the reader count.
We need to check for waiters after decrementing the read count - not
before.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
In no_data_io mode, we expect data checksums to be wrong - don't want to
spew the log with them.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
When searching the link table for the matching inode, we were searching
for a specific - incorrect - snapshot ID as well, causing us to fail to
find the inode.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Running with fewer max btree paths doesn't work anymore when replication
is enabled - as we've added e.g. the freespace and bucket gens btrees,
we naturally end up needing more btree paths.
This is an issue with lockdep, we end up taking more locks than lockdep
will track (the MAX_LOCKD_DEPTH constant). But bcachefs as merged does
not yet support lockdep anyways, so we can leave that for later.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
The journal read path had some informational log statements preperatory
for ZNS support - they're not of interest to users, so we can turn them
off.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
In may_delete_deleted_inode(), there's a corner case when a snapshot was
taken while we had an unlinked inode: we don't want to delete the inode
in the internal (shared) snapshot node, since it might have been
reattached in a descendent snapshot.
Instead we propagate the key to any snapshot leaves it doesn't exist in,
so that it can be deleted there if necessary, and then clear the
unlinked flag in the internal node.
But we forgot to commit after clearing the unlinked flag, causing us to
go into an infinite loop.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
This fixes a bug discovered by generic/388 where sb->s_fs_info was NULL
while the superblock was still active - the error path was entirely
fubar, and was trying to do something unclear and unecessary.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
During mount, bcachefs mount option processing may sleep while allocating a string buffer.
Fix this by reference counting in order to take the atomic path.
Signed-off-by: Daniel J Blueman <daniel@quora.org>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
may_delete_deleted_inode() was returning without exiting a btree
iterator, eventually causing propagate_key_to_snaphot_leaves() to go
into an infinite loop hitting btree_trans_too_many_iters().
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
This deletes the complicated and somewhat expensive journal
pre-reservation machinery in favor of just using journal watermarks:
when the journal is more than half full, we run journal reclaim more
aggressively, and when the journal is more than 3/4s full we only allow
journal reclaim to get new journal reservations.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
We've rarely been seeing a nonce offset inconsistency that doesn't show
up in tests: this adds some extra verification code to the data update
path that prints out more relevant info when it occurs.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
We really don't want to be invoking memory reclaim with btree locks
held: even aside from (solvable, but tricky) recursion issues, it can
cause painful to diagnose performance edge cases.
This fixes a recently reported issue in btree_key_can_insert_cached().
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Reported-by: Mateusz Guzik <mjguzik@gmail.com>
Fixes: https://lore.kernel.org/linux-bcachefs/CAGudoHEsb_hGRMeWeXh+UF6po0qQuuq_NKSEo+s1sEb6bDLjpA@mail.gmail.com/T/
As prep work for the next patch to fix a key cache reclaim issue, we
need to start tracking whether we're currently holding write locks - so
that we can release and retake the before calling into memory reclaim.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
The btree key cache maintains lists of items that have been freed, but
can't yet be reclaimed because a bch2_trans_relock() call might find
them - we're waiting for SRCU readers to release.
Previously, we wouldn't count these items against the number we're
attempting to scan for, which would mean we'd evict more live key cache
entries - doing quite a bit of potentially unecessary work.
With recent work to make sure we don't hold SRCU locks for too long, it
should be safe to count all the items on the freelists against number to
scan - even if we can't reclaim them yet, we will be able to soon.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
We can't create stripes if we don't have enough devices - this
manifested as an integer underflow bug later.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
bch2_btree_iter_peek_node() can return a NULL ptr (when the tree is
shorter than the search depth); handle this with an early return.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Fixes: https://lore.kernel.org/linux-bcachefs/5fc3c28b-c232-4ec7-b0ac-4ef220ddf976@moroto.mountain/T/
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Transform zero-length array `entries` into a proper flexible-array
member in `struct journal_seq_blacklist_table`; and fix the following
-Warray-bounds warnings:
fs/bcachefs/journal_seq_blacklist.c:148:26: warning: array subscript idx is outside array bounds of 'struct journal_seq_blacklist_table_entry[0]' [-Warray-bounds=]
fs/bcachefs/journal_seq_blacklist.c:150:30: warning: array subscript idx is outside array bounds of 'struct journal_seq_blacklist_table_entry[0]' [-Warray-bounds=]
fs/bcachefs/journal_seq_blacklist.c:154:27: warning: array subscript idx is outside array bounds of 'struct journal_seq_blacklist_table_entry[0]' [-Warray-bounds=]
fs/bcachefs/journal_seq_blacklist.c:176:27: warning: array subscript i is outside array bounds of 'struct journal_seq_blacklist_table_entry[0]' [-Warray-bounds=]
fs/bcachefs/journal_seq_blacklist.c:177:27: warning: array subscript i is outside array bounds of 'struct journal_seq_blacklist_table_entry[0]' [-Warray-bounds=]
fs/bcachefs/journal_seq_blacklist.c:297:34: warning: array subscript i is outside array bounds of 'struct journal_seq_blacklist_table_entry[0]' [-Warray-bounds=]
fs/bcachefs/journal_seq_blacklist.c:298:34: warning: array subscript i is outside array bounds of 'struct journal_seq_blacklist_table_entry[0]' [-Warray-bounds=]
fs/bcachefs/journal_seq_blacklist.c:300:31: warning: array subscript i is outside array bounds of 'struct journal_seq_blacklist_table_entry[0]' [-Warray-bounds=]
This results in no differences in binary output.
This helps with the ongoing efforts to globally enable -Warray-bounds.
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Transform zero-length array `s` into a proper flexible-array
member in `struct snapshot_table` via the DECLARE_FLEX_ARRAY()
helper; and fix tons of the following -Warray-bounds warnings:
fs/bcachefs/snapshot.h:36:21: warning: array subscript <unknown> is outside array bounds of 'struct snapshot_t[0]' [-Warray-bounds=]
fs/bcachefs/snapshot.h:36:21: warning: array subscript <unknown> is outside array bounds of 'struct snapshot_t[0]' [-Warray-bounds=]
fs/bcachefs/snapshot.c:135:70: warning: array subscript <unknown> is outside array bounds of 'struct snapshot_t[0]' [-Warray-bounds=]
fs/bcachefs/snapshot.h:36:21: warning: array subscript <unknown> is outside array bounds of 'struct snapshot_t[0]' [-Warray-bounds=]
fs/bcachefs/snapshot.h:36:21: warning: array subscript <unknown> is outside array bounds of 'struct snapshot_t[0]' [-Warray-bounds=]
fs/bcachefs/snapshot.h:36:21: warning: array subscript <unknown> is outside array bounds of 'struct snapshot_t[0]' [-Warray-bounds=]
This helps with the ongoing efforts to globally enable -Warray-bounds.
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
The bch2_target_to_text_sb are not used outside the file disk_groups.c,
so the modification is defined as static.
fs/bcachefs/disk_groups.c:583:6: warning: no previous prototype for ‘bch2_target_to_text_sb’.
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7144
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
Here's the second big bcachefs pull request. This brings your tree up to
date with my master branch, which is what existing bcachefs users are
currently running.
All but the last few patches have been in linux-next, those being small
fixes. Test results from my dashboard:
https://evilpiepirate.org/~testdashboard/ci?commit=c7046ed0cf9bb33599aa7e72e7b67bba4be42d64
New features:
- rebalance_work btree (and metadata version 1.3): the rebalance thread
no longer has to scan to find extents that need processing - big
scalability improvement.
- sb_errors superblock section: this adds counters for each fsck error
type, since filesystem creation, along with the date of the most
recent error. It'll get us better bug reports (since users do not
typically report errors that fsck was able to fix), and I might add
telemetry for this in the future.
Fixes include:
- multiple snapshot deletion fixes
- members_v2 fixups
- deleted_inodes btree fixes
- copygc thread no longer spins when a device is full but has no
fragmented buckets (i.e. rebalance needs to move data around instead)
- a fix for a memory reclaim issue with the btree key cache: we're now
careful not to hold the srcu read lock that blocks key cache reclaim
for too long
- an early allocator locking fix, from Brian
- endianness fixes, from Brian
- CONFIG_BCACHEFS_DEBUG_TRANSACTIONS no longer defaults to y, a big
performance improvement on multithreaded workloads
-----BEGIN PGP SIGNATURE-----
iQIzBAABCgAdFiEEKnAFLkS8Qha+jvQrE6szbY3KbnYFAmVH9xYACgkQE6szbY3K
bnahLRAAiNRZL73SQ+MW79o4yPqGwt0Eyy/mvoiGpZf1B8uXp0oZ55j2w3l887Uf
LeM03mInAYCPdyp/d4vxqIr96j9BODmRRl8sEkkGdJDzokLG+22F0ovOe45KWTxL
kBoNdng/O/oeOe/1K7taP3KzBvMx2nOF6oA+xfgyCjECMArAIXek0iocyEUR4Ywd
vGKhLNn1k2c+94wacnDYwjjdcLBxoqxsFXlpu6V0BcaY+DX4J3aBaGmj75KEoCI0
VbBOzxrOO4QzJrzW2+hxZZWgGyvReCkBJvqfORfuPxiSbFobTim10MdfZOAMQA1U
Xr1FTEpK1wMX0/pPVgZRqaOsttC+yc/SsfPNgSxybgHPbDlMLaakDHjvYssbKOYG
urDWSMG5yCsktSLj95SXsvUFKZaZFD72SKBNdgdt/nZjwTHuNQ7IkdrMwIrCQ/PT
Ifn50UrR/Ahd8RAd5tyNCPw6U9VfwnxACSNl2KA7ONKpvHb+gSt1JsJTDyz1+gN9
nFVrw1SHKQ6EIV6XhVon/5DEuRTzqoYGWoN08FHEUq9fBlvnVpmbJErCQMplOjz9
OQnAfpJH4YqkpXyjFAjP1V0An+RUn8QvDgXNqC9TyvCYuOliVFuil4y7/c+7oIQU
NEoz+jVLenqsGOGAbduI4/Q567COojRgwEvbebSIxSImXuhCNj4=
=Lo4N
-----END PGP SIGNATURE-----
Merge tag 'bcachefs-2023-11-5' of https://evilpiepirate.org/git/bcachefs
Pull more bcachefs updates from Kent Overstreet:
"Here's the second big bcachefs pull request. This brings your tree up
to date with my master branch, which is what existing bcachefs users
are currently running.
New features:
- rebalance_work btree (and metadata version 1.3): the rebalance
thread no longer has to scan to find extents that need processing -
big scalability improvement.
- sb_errors superblock section: this adds counters for each fsck
error type, since filesystem creation, along with the date of the
most recent error. It'll get us better bug reports (since users do
not typically report errors that fsck was able to fix), and I might
add telemetry for this in the future.
Fixes include:
- multiple snapshot deletion fixes
- members_v2 fixups
- deleted_inodes btree fixes
- copygc thread no longer spins when a device is full but has no
fragmented buckets (i.e. rebalance needs to move data around
instead)
- a fix for a memory reclaim issue with the btree key cache: we're
now careful not to hold the srcu read lock that blocks key cache
reclaim for too long
- an early allocator locking fix, from Brian
- endianness fixes, from Brian
- CONFIG_BCACHEFS_DEBUG_TRANSACTIONS no longer defaults to y, a big
performance improvement on multithreaded workloads"
* tag 'bcachefs-2023-11-5' of https://evilpiepirate.org/git/bcachefs: (70 commits)
bcachefs: Improve stripe checksum error message
bcachefs: Simplify, fix bch2_backpointer_get_key()
bcachefs: kill thing_it_points_to arg to backpointer_not_found()
bcachefs: bch2_ec_read_extent() now takes btree_trans
bcachefs: bch2_stripe_to_text() now prints ptr gens
bcachefs: Don't iterate over journal entries just for btree roots
bcachefs: Break up bch2_journal_write()
bcachefs: Replace ERANGE with private error codes
bcachefs: bkey_copy() is no longer a macro
bcachefs: x-macro-ify inode flags enum
bcachefs: Convert bch2_fs_open() to darray
bcachefs: Move __bch2_members_v2_get_mut to sb-members.h
bcachefs: bch2_prt_datetime()
bcachefs: CONFIG_BCACHEFS_DEBUG_TRANSACTIONS no longer defaults to y
bcachefs: Add a comment for BTREE_INSERT_NOJOURNAL usage
bcachefs: rebalance_work btree is not a snapshots btree
bcachefs: Add missing printk newlines
bcachefs: Fix recovery when forced to use JSET_NO_FLUSH journal entry
bcachefs: .get_parent() should return an error pointer
bcachefs: Fix bch2_delete_dead_inodes()
...
We now include the name of the device in the error message - and also
increment the number of checksum errors on that device.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
- backpointer_not_found() checks backpointers_no_use_write_buffer, no
need to do it inbackpointer_get_key().
- always use backpointer_get_node() for pointers to nodes:
backpointer_get_key() was sometimes returning the key from the root
node unlocked.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
We're not supposed to have more than one btree_trans at a time in a
given thread - that causes recursive locking deadlocks.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Split up bch2_journal_write() to simplify locking:
- bch2_journal_write_pick_flush(), which needs j->lock
- bch2_journal_write_prep, which operates on the journal buffer to be
written and will need the upcoming buf_lock for synchronization with
the btree write buffer flush path
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
We avoid using standard error codes: private, per-callsite error codes
make debugging easier.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
BCACHEFS_DEBUG_TRANSACTIONS is useful, but it's too expensive to have on
by default - and it hasn't been coming up in bug reports.
Turn it off by default until we figure out a way to make it cheaper.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
BTREE_INSERT_NOJOURNAL is primarily used for a performance optimization
related to inode updates and fsync - document it.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
rebalance_work entries may refer to entries in the extents btree, which
is a snapshots btree, or they may also refer to entries in the reflink
btree, which is not.
Hence rebalance_work keys may use the snapshot field but it's not
required to be nonzero - add a new btree flag to reflect this.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
When we didn't find anything in the journal that we'd like to use, and
we're forced to use whatever we can find - that entry will have been a
JSET_NO_FLUSH entry with a garbage last_seq value, since it's not
normally used.
Initialize it to something sane, for bch2_fs_journal_start().
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Delete the useless check for inum == 0; we'll return -ENOENT without it,
which is what we want.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
- the fsck_err() check for the filesystem being clean was incorrect,
causing us to always fail to delete unlinked inodes
- if a snapshot had been taken, the unlinked inode needs to be
propagated to snapshot leaves so the unlink can happen there - fixed.
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
The bucket_offset field of bch_backpointer is a 40-bit bitfield, but the
bch2_backpointer_swab() helper uses swab32. This leads to inconsistency
when an on-disk fs is accessed from an opposite endian machine.
As it turns out, we already have an internal swab40() helper that is
used from the bch_alloc_v4 swab callback. Lift it into the backpointers
header file and use it consistently in both places.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
A simple test to populate a filesystem on one CPU architecture and
fsck on an arch of the opposite byte order produces errors related
to the fragmentation LRU. This occurs because the 64-bit
fragmentation_lru field is not byte-order swapped when reads detect
that the on-disk/bset key values were written in opposite byte-order
of the current CPU.
Update the bch2_alloc_v4 swab callback to handle fragmentation_lru
as is done for other multi-byte fields. This doesn't affect existing
filesystems when accessed by CPUs of the same endianness because the
->swab() callback is only called when the bset flags indicate an
endianness mismatch between the CPU and on-disk data.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
The bcachefs folio writeback code includes a bio full check as well
as a fixed size check to determine when to split off and submit
writeback I/O. The inclusive check of the latter against the limit
means that writeback can submit slightly prematurely. This is not a
functional problem, but results in unnecessarily split I/Os and
extent merging.
This can be observed with a buffered write sized exactly to the
current maximum value (1MB) and with key_merging_disabled=1. The
latter prevents the merge from the second write such that a
subsequent check of the extent list shows a 1020k extent followed by
a contiguous 4k extent.
The purpose for the fixed size check is also undocumented and
somewhat obscure. Lift this check into a new helper that wraps the
bio check, fix the comparison logic, and add a comment to document
the purpose and how we might improve on this in the future.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Guenter Roeck reports a lockdep splat and DEBUG_OBJECTS_WORK related
warning when bch2_copygc_thread() initializes its rhashtable. The
lockdep splat relates to a warning print caused by the fact that the
rhashtable exists on the stack but is not annotated as so. This is
something that could be addressed by INIT_WORK_ONSTACK(), but
rhashtable doesn't expose that control and probably isnt worth the
churn for just one user. Instead, dynamically allocate the
buckets_in_flight structure and avoid the splat that way.
Reported-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
A recent bug report uncovered a scenario where a filesystem never
runs with freespace_initialized, and therefore the user observes
significantly degraded write performance by virtue of running the
early bucket allocator. The associated bug aside, the primary cause
of the performance drop in this particular instance is that the
early bucket allocator does not update the allocation cursor. This
means that every allocation walks the alloc btree from the first
bucket of the associated device looking for a bucket marked as free
space.
Update the early allocator code to set the alloc cursor to the last
processed position in the tree, similar to how the freelist
allocator behaves. With the alloc_cursor being updated, the retry
logic also needs to be updated to restart from the beginning of the
device when a free bucket is not available between the cursor and
the end of the device. Track the restart position in a first_bucket
variable to make the code a bit more easily readable and consistent
with the freelist allocator.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
bcachefs had a transient bug where freespace_initialized was not
properly being set, which lead to unexpected use of the early bucket
allocator at runtime. This issue has been fixed, but the existence
of it uncovered a coherency issue in the early bucket allocation
code that is somewhat related to how uncached iterators deal with
the key cache.
The problem itself manifests as occasional failure of generic/113
due to corruption, often seen as a duplicate backpointer or multiple
data types per-bucket error. The immediate cause of the error is a
racing bucket allocation along the lines of the following sequence:
- Task 1 selects key A in bch2_bucket_alloc_early() and schedules.
- Task 2 selects the same key A, but proceeds to complete the
allocation and associated I/O, after which it releases the
open_bucket.
- Task 1 resumes with key A, but does not recognize the bucket is
now allocated because the open_bucket has been removed
from the hash when it was released in the previous step.
This generally shouldn't happen because the allocating task updates
the alloc btree key before releasing the bucket. This is not
sufficient in this particular instance, however, because an uncached
iterator for a cached btree doesn't actually lock the key cache slot
when no key exists for a given slot in the cache. Thus the fact that
the allocation side updates the cached key means that multiple
uncached iters can stumble across the same alloc key and duplicate
the bucket allocation as described above.
This is something that probably needs a longer term fix in the
iterator code. As a short term fix, close the race through explicit
use of a cached iterator for likely allocation candidates. We don't
want to scan the btree with a cached iterator because that would
unnecessarily pollute the cache. This mitigates cache pollution by
primarily scanning the tree with an uncached iterator, but closes
the race by creating a key cache entry for any prospective slot
prior to the bucket allocation attempt (also similar to how
_alloc_freelist() works via try_alloc_bucket()). This survives many
iterations of generic/113 on a kernel hacked to always use the early
bucket allocator.
Signed-off-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
