- 22 Oct, 2023 40 commits
-
-
Kent Overstreet authored
bch2_inode_delete_keys() was using BTREE_ITER_NOT_EXTENTS, on the assumption that it would never need to split extents. But that caused a race with extents being split by other threads - specifically, the data move path. Extents iterators have the iterator position pointing to the start of the extent, which avoids the race. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
The entire btree will be lost, but that is better than the entire filesystem not being recoverable. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
We can only do this in userspace, unfortunately - but kernel keyrings have never seemed to worked reliably, this is a useful fallback. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
These errors aren't actual errors, and should never be printed - do this in the common helpers. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
- assert in shutdown path that no nocow locks are held - check for overflow when taking nocow locks Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
This makes mount option handling consistent with other filesystems - options may be handled at different layers, so an option we don't know about might not be intended for us. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Previously, we would check for invalid bkeys at transaction commit time, but only if CONFIG_BCACHEFS_DEBUG=y. This check is important enough to always be on - it appears there's been corruption making it into the journal that would have been caught by it. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Previously, equiv was set in the snapshot deletion path, which is where it's needed - equiv, for snapshot ID equivalence classes, would ideally be a private data structure to the snapshot deletion path. But if a new snapshot is created while snapshot deletion is running, move_key_to_correct_snapshot() moves a key to snapshot id 0 - oops. Fixes: https://github.com/koverstreet/bcachefs/issues/593Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Reported-by: smatch Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Torge Matthies authored
Signed-off-by: Torge Matthies <openglfreak@googlemail.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Brian Foster authored
Initial support for the vfs superblock freeze and unfreeze operations. Superblock freeze occurs in stages, where the vfs attempts to quiesce high level write operations, page faults, fs internal operations, and then finally calls into the filesystem for any last stage steps (i.e. log flushing, etc.) before marking the superblock frozen. The majority of write paths are covered by freeze protection (i.e. sb_start_write() and friends) in higher level common code, with the exception of the fs-internal SB_FREEZE_FS stage (i.e. sb_start_intwrite()). This typically maps to active filesystem transactions in a manner that allows the vfs to implement a barrier of internal fs operations during the freeze sequence. This is not a viable model for bcachefs, however, because it utilizes transactions both to populate the journal as well as to perform journal reclaim. This means that mapping intwrite protection to transaction lifecycle or transaction commit is likely to deadlock freeze, as quiescing the journal requires transactional operations blocked by the final stage of freeze. The flipside of this is that bcachefs does already maintain its own internal sets of write references for similar purposes, currently utilized for transitions from read-write to read-only mode. Since this largely mirrors the high level sequence involved with freeze, we can simply invoke this mechanism in the freeze callback to fully quiesce the filesystem in the final stage. This means that while the SB_FREEZE_FS stage is essentially a no-op, the ->freeze_fs() callback that immediately follows begins by performing effectively the same step by quiescing all internal write references. One caveat to this approach is that without integration of internal freeze protection, write operations gated on internal write refs will fail with an internal -EROFS error rather than block on acquiring freeze protection. IOW, this is roughly equivalent to only having support for sb_start_intwrite_trylock(), and not the blocking variant. Many of these paths already use non-blocking internal write refs and so would map into an sb_start_intwrite_trylock() anyways. The only instance of this I've been able to uncover that doesn't explicitly rely on a higher level non-blocking write ref is the bch2_rbio_narrow_crcs() path, which updates crcs in certain read cases, and Kent has pointed out isn't critical if it happens to fail due to read-only status. Given that, implement basic freeze support as described above and leave tighter integration with internal freeze protection as a possible future enhancement. There are multiple potential ideas worth exploring here. For example, we could implement a multi-stage freeze callback that might allow bcachefs to quiesce its internal write references without deadlocks, we could integrate intwrite protection with bcachefs' internal write references somehow or another, or perhaps consider implementing blocking support for internal write refs to be used specifically for freeze, etc. In the meantime, this enables functional freeze support and the associated test coverage that comes with it. Signed-off-by: Brian Foster <bfoster@redhat.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
- fix a few uninitialized return values - return a proper error code in lookup_lostfound() Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
- it's no longer possible for trans to be NULL - also, move "wait for read to complete" to the slowpath, __bch2_btree_node_get(). Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
kvfree_rcu() was renamed - not removed. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
strndup_user() returns an error pointer, not NULL. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
bch2_journal_write() expects process context, it takes journal_lock as needed. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
crypto_alloc_sync_skcipher() returns an ERR_PTR, not NULL. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
When bucket sector counts were changed from u16s to u32s, a few things were missed. This fixes an overflow check, and a truncation that prevented the overflow check from firing. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
copy_to_user() returns the number of bytes successfully copied - not an errcode. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Brian Foster authored
bcachefs freeze testing via fstests generic/390 occasionally reproduces the following BUG from bch2_fs_read_only(): BUG_ON(atomic_long_read(&c->btree_key_cache.nr_dirty)); This indicates that one or more dirty key cache keys still exist after the attempt to flush and quiesce the fs. The sequence that leads to this problem actually occurs on unfreeze (ro->rw), and looks something like the following: - Task A begins a transaction commit and acquires journal_res for the current seq. This transaction intends to perform key cache insertion. - Task B begins a bch2_journal_flush() via bch2_sync_fs(). This ends up in journal_entry_want_write(), which closes the current journal entry and drops the reference to the pin list created on entry open. The pin put pops the front of the journal via fast reclaim since the reference count has dropped to 0. - Task A attempts to set the journal pin for the associated cached key, but bch2_journal_pin_set() skips the pin insert because the seq of the transaction reservation is behind the front of the pin list fifo. The end result is that the pin associated with the cached key is not added, which prevents a subsequent reclaim from processing the key and thus leaves it dangling at freeze time. The fundamental cause of this problem is that the front of the journal is allowed to pop before a transaction with outstanding reservation on the associated journal seq is able to add a pin. The count for the pin list associated with the seq drops to zero and is prematurely reclaimed as a result. The logical fix for this problem lies in how the journal buffer is managed in similar scenarios where the entry might have been closed before a transaction with outstanding reservations happens to be committed. When a journal entry is opened, the current sequence number is bumped, the associated pin list is initialized with a reference count of 1, and the journal buffer reference count is bumped (via journal_state_inc()). When a journal reservation is acquired, the reservation also acquires a reference on the associated buffer. If the journal entry is closed in the meantime, it drops both the pin and buffer references held by the open entry, but the buffer still has references held by outstanding reservation. After the associated transaction commits, the reservation release drops the associated buffer references and the buffer is written out once the reference count has dropped to zero. The fundamental problem here is that the lifecycle of the pin list reference held by an open journal entry is too short to cover the processing of transactions with outstanding reservations. The simplest way to address this is to expand the pin list reference to the lifecycle of the buffer vs. the shorter lifecycle of the open journal entry. This ensures the pin list for a seq with outstanding reservation cannot be popped and reclaimed before all outstanding reservations have been released, even if the associated journal entry has been closed for further reservations. Move the pin put from journal entry close to where final processing of the journal buffer occurs. Create a duplicate helper to cover the case where the caller doesn't already hold the journal lock. This allows generic/390 to pass reliably. Signed-off-by: Brian Foster <bfoster@redhat.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Brian Foster authored
bcachefs freeze testing has uncovered some raciness between journal entry open/close and pin list reference count management. The details of the problem are described in a separate patch. In preparation for the associated fix, refactor the journal buffer put path a bit to allow it to eventually handle dropping the pin list reference currently held by an open journal entry. Retain the journal write dispatch helper since the closure code is inlined and we don't want to increase the amount of inline code in the transaction commit path, but rename the function to reflect the purpose of final processing of the journal buffer. Signed-off-by: Brian Foster <bfoster@redhat.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Brian Foster authored
We have a couple journal pin put helpers to handle cases where the journal lock is already held or not. Refactor the helpers to lock and reclaim from the highest level and open code the reclaim from the one caller of the internal variant. The latter call will be moved into the journal buf release helper in a later patch. Signed-off-by: Brian Foster <bfoster@redhat.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Dan Carpenter authored
This code accidentally left out the "ret = " assignment so the errors from for_each_btree_key2() are not checked. Fixes: 53534482a250 ("bcachefs: for_each_btree_key2()") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Dan Carpenter authored
The copy_to_user() function returns the number of bytes that it wasn't able to copy but we want to return -EFAULT to the user. Fixes: e0750d947352 ("bcachefs: Initial commit") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Dan Carpenter authored
The "ret = bkey_err(k);" assignment was accidentally left out so the call to bch2_btree_iter_peek_slot() is not checked for errors. Fixes: 53306e096d91 ("bcachefs: Always check for transaction restarts") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Dan Carpenter authored
The clean up code at the end of the function uses "acl" so it needs to be initialized to NULL. Fixes: 53306e096d91 ("bcachefs: Always check for transaction restarts") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Nick Desaulniers authored
Fixes the following observed error reported by Nathan on IRC. fs/bcachefs/io_misc.c:467:6: error: explicitly assigning value of variable of type 'int' to itself [-Werror,-Wself-assign] 467 | ret = ret; | ~~~ ^ ~~~ Reported-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Jiapeng Chong authored
./fs/bcachefs/btree_update.h: journal.h is included more than once. Reported-by: Abaci Robot <abaci@linux.alibaba.com> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=6573Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Dan Carpenter authored
There is a typo here where it uses ";" instead of "?:". The result is that bch2_fs_fs_io_direct_init() is called unconditionally and the errors from it are not checked. Fixes: 0060c68159fc ("bcachefs: Split up fs-io.[ch]") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> Reviewed-by: Brian Foster <bfoster@redhat.com>
-
Dan Carpenter authored
On 32 bit systems, "sizeof(*arg) + replica_entries_bytes" can have an integer overflow leading to memory corruption. Use size_add() to prevent this. Fixes: b44dd3797034 ("bcachefs: Redo filesystem usage ioctls") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Dan Carpenter authored
The copy_to_user() function returns the number of bytes remaining but we want to return -EFAULT to the user. Fixes: e0750d947352 ("bcachefs: Initial commit") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
bucket_lock() previously open coded a spinlock, because we need to cram a spinlock into a single byte. But it turns out not all archs support xchg() on a single byte; since we need struct bucket to be small, this means we have to play fun games with casts and ifdefs for endianness. This fixes building on 32 bit arm, and likely other architectures. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> Cc: linux-bcachefs@vger.kernel.org Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Per previous commit, bare unreachable() considered harmful, convert to BUG() Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Josh Poimboeuf authored
In general it's a good idea to avoid using bare unreachable() because it introduces undefined behavior in compiled code. In this case it even confuses GCC into emitting an empty unused bch2_dev_buckets_reserved.part.0() function. Use BUG() instead, which is nice and defined. While in theory it should never trigger, if something were to go awry and the BCH_WATERMARK_NR case were to actually hit, the failure mode is much more robust. Fixes the following warnings: vmlinux.o: warning: objtool: bch2_bucket_alloc_trans() falls through to next function bch2_reset_alloc_cursors() vmlinux.o: warning: objtool: bch2_dev_buckets_reserved.part.0() is missing an ELF size annotation Reported-by: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Christophe JAILLET authored
Remove a redundant call to bch2_free_super(). This is harmless because bch2_free_super() has a memset() at its end. So a second call would only lead to from kfree(NULL). Remove the redundant call and only rely on the error handling path. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-