- 22 Oct, 2023 40 commits
-
-
Nathan Chancellor authored
When building bcachefs with -Wincompatible-function-pointer-types-strict, a clang warning designed to catch issues with mismatched function pointer types, which will be fatal at runtime due to kernel Control Flow Integrity (kCFI), there are several instances along the lines of: fs/bcachefs/bkey_methods.c:118:2: error: incompatible function pointer types initializing 'int (*)(const struct bch_fs *, struct bkey_s_c, enum bkey_invalid_flags, struct printbuf *)' with an expression of type 'int (const struct bch_fs *, struct bkey_s_c, unsigned int, struct printbuf *)' [-Werror,-Wincompatible-function-pointer-types-strict] 118 | BCH_BKEY_TYPES() | ^~~~~~~~~~~~~~~~ fs/bcachefs/bcachefs_format.h:342:2: note: expanded from macro 'BCH_BKEY_TYPES' 342 | x(deleted, 0) \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~ fs/bcachefs/bkey_methods.c:117:41: note: expanded from macro 'x' 117 | #define x(name, nr) [KEY_TYPE_##name] = bch2_bkey_ops_##name, | ^~~~~~~~~~~~~~~~~~~~ <scratch space>:206:1: note: expanded from here 206 | bch2_bkey_ops_deleted | ^~~~~~~~~~~~~~~~~~~~~ fs/bcachefs/bkey_methods.c:34:17: note: expanded from macro 'bch2_bkey_ops_deleted' 34 | .key_invalid = deleted_key_invalid, \ | ^~~~~~~~~~~~~~~~~~~ The flags parameter should be of type 'enum bkey_invalid_flags', not 'unsigned int'. Adjust the type everywhere so that there is no more warning. Signed-off-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Nathan Chancellor authored
When building bcachefs for 32-bit ARM, there is a compiler warning in bch2_bucket_gens_invalid() due to use of an incorrect format specifier: fs/bcachefs/alloc_background.c:530:10: error: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Werror,-Wformat] 529 | prt_printf(err, "bad val size (%lu != %zu)", | ~~~ | %zu 530 | bkey_val_bytes(k.k), sizeof(struct bch_bucket_gens)); | ^~~~~~~~~~~~~~~~~~~ fs/bcachefs/util.h:223:54: note: expanded from macro 'prt_printf' 223 | #define prt_printf(_out, ...) bch2_prt_printf(_out, __VA_ARGS__) | ^~~~~~~~~~~ On 64-bit architectures, size_t is 'unsigned long', so there is no warning when using %lu but on 32-bit architectures, size_t is 'unsigned int'. Use '%zu', the format specifier for 'size_t', to eliminate the warning. Fixes: 4be0d766a7e9 ("bcachefs: bucket_gens btree") Signed-off-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Nathan Chancellor authored
When building bcachefs for 32-bit ARM, there is a compiler warning in bch2_alloc_v4_invalid() due to use of an incorrect format specifier: fs/bcachefs/alloc_background.c:246:30: error: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Werror,-Wformat] 245 | prt_printf(err, "bad val size (%u > %lu)", | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | %u 246 | alloc_v4_u64s(a.v), bkey_val_u64s(k.k)); | ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ fs/bcachefs/bkey.h:58:27: note: expanded from macro 'bkey_val_u64s' 58 | #define bkey_val_u64s(_k) ((_k)->u64s - BKEY_U64s) | ^ fs/bcachefs/util.h:223:54: note: expanded from macro 'prt_printf' 223 | #define prt_printf(_out, ...) bch2_prt_printf(_out, __VA_ARGS__) | ^~~~~~~~~~~ This expression is of type 'size_t'. On 64-bit architectures, size_t is 'unsigned long', so there is no warning when using %lu but on 32-bit architectures, size_t is 'unsigned int'. Use '%zu', the format specifier for 'size_t' to eliminate the warning. Fixes: 11be8e8db283 ("bcachefs: New on disk format: Backpointers") Signed-off-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Nathan Chancellor authored
When building bcachefs for 32-bit ARM, there is a compiler warning in bch2_btree_key_cache_to_text() due to use of an incorrect format specifier: fs/bcachefs/btree_key_cache.c:1060:36: error: format specifies type 'size_t' (aka 'unsigned int') but the argument has type 'long' [-Werror,-Wformat] 1060 | prt_printf(out, "nr_freed:\t%zu", atomic_long_read(&c->nr_freed)); | ~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | %ld fs/bcachefs/util.h:223:54: note: expanded from macro 'prt_printf' 223 | #define prt_printf(_out, ...) bch2_prt_printf(_out, __VA_ARGS__) | ^~~~~~~~~~~ 1 error generated. On 64-bit architectures, size_t is 'unsigned long', so there is no warning when using %zu but on 32-bit architectures, size_t is 'unsigned int'. Use '%lu' to match the other format specifiers used in this function for printing values returned from atomic_long_read(). Fixes: 6d799930ce0f ("bcachefs: btree key cache pcpu freedlist") Signed-off-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Nathan Chancellor authored
When building bcachefs for 32-bit ARM, there is a compiler warning in bch2_set_bucket_needs_journal_commit() due to a debug print using the wrong specifier: fs/bcachefs/buckets_waiting_for_journal.c:137:30: error: format specifies type 'size_t' (aka 'unsigned int') but the argument has type 'unsigned long' [-Werror,-Wformat] 136 | pr_debug("took %zu rehashes, table at %zu/%zu elements", | ~~~ | %lu 137 | nr_rehashes, nr_elements, 1UL << b->t->bits); | ^~~~~~~~~~~~~~~~~ include/linux/printk.h:579:26: note: expanded from macro 'pr_debug' 579 | dynamic_pr_debug(fmt, ##__VA_ARGS__) | ~~~ ^~~~~~~~~~~ include/linux/dynamic_debug.h:270:22: note: expanded from macro 'dynamic_pr_debug' 270 | pr_fmt(fmt), ##__VA_ARGS__) | ~~~ ^~~~~~~~~~~ include/linux/dynamic_debug.h:250:59: note: expanded from macro '_dynamic_func_call' 250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__) | ^~~~~~~~~~~ include/linux/dynamic_debug.h:248:65: note: expanded from macro '_dynamic_func_call_cls' 248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__) | ^~~~~~~~~~~ include/linux/dynamic_debug.h:224:15: note: expanded from macro '__dynamic_func_call_cls' 224 | func(&id, ##__VA_ARGS__); \ | ^~~~~~~~~~~ 1 error generated. On 64-bit architectures, size_t is 'unsigned long', so there is no warning when using %zu but on 32-bit architectures, size_t is 'unsigned int'. Use the correct specifier to resolve the warning. Fixes: 7a82e75ddaef ("bcachefs: New data structure for buckets waiting on journal commit") Signed-off-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Colin Ian King authored
There are several spelling mistakes in error messages. Fix these. Signed-off-by: Colin Ian King <colin.i.king@gmail.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Colin Ian King authored
The pointer q is being assigned a value but it is never read. The assignment and pointer are redundant and can be removed. Cleans up clang scan build warning: fs/bcachefs/quota.c:813:2: warning: Value stored to 'q' is never read [deadcode.DeadStores] Signed-off-by: Colin Ian King <colin.i.king@gmail.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Colin Ian King authored
Variable offset_into_extent is being assigned to zero and a few statements later it is being re-assigned again to the save value. The second assignment is redundant and can be removed. Cleans up clang-scan build warning: fs/bcachefs/io.c:2722:3: warning: Value stored to 'offset_into_extent' is never read [deadcode.DeadStores] Signed-off-by: Colin Ian King <colin.i.king@gmail.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Colin Ian King authored
The variables start_offset and end_offset are being initialized with values that are never read, they being re-assigned later on. The initializations are redundant and can be removed. Cleans up clang-scan build warnings: fs/bcachefs/fs-io.c:243:11: warning: Value stored to 'start_offset' during its initialization is never read [deadcode.DeadStores] fs/bcachefs/fs-io.c:244:11: warning: Value stored to 'end_offset' during its initialization is never read [deadcode.DeadStores] Signed-off-by: Colin Ian King <colin.i.king@gmail.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Colin Ian King authored
The pointer dst is being initialized with a value that is never read, it is being re-assigned later on when it is used in a while-loop The initialization is redundant and can be removed. Cleans up clang-scan build warning: fs/bcachefs/disk_groups.c:186:30: warning: Value stored to 'dst' during its initialization is never read [deadcode.DeadStores] Signed-off-by: Colin Ian King <colin.i.king@gmail.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Colin Ian King authored
The pointer d is being initialized with a value that is never read, it is being re-assigned later on when it is used in a for-loop. The initialization is redundant and can be removed. Cleans up clang-scan build warning: fs/bcachefs/buckets.c:1303:25: warning: Value stored to 'd' during its initialization is never read [deadcode.DeadStores] Signed-off-by: Colin Ian King <colin.i.king@gmail.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Add a tracepoint to print the reason a read wasn't promoted. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Now that we have the logged operations btree, we can make finsert/fcollapse atomic w.r.t. unclean shutdown as well. This adds bch_logged_op_finsert to represent the state of an finsert or fcollapse, which is a bit more complicated than truncate since we need to track our position in the "shift extents" operation. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Previously, we guaranteed atomicity of truncate after unclean shutdown with the BCH_INODE_I_SIZE_DIRTY flag - which required a full scan of the inodes btree. Recently the deleted inodes btree was added so that we no longer have to scan for deleted inodes, but truncate was unfinished and that change left it broken. This patch uses the new logged operations btree to fix truncate atomicity; we now log an operation that can be replayed at the start of a truncate. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Add a new btree for long running logged operations - i.e. for logging operations that we can't do within a single btree transaction, so that they can be resumed if we crash. Keys in the logged operations btree will represent operations in progress, with the state of the operation stored in the value. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
This pulls the non vfs specific parts of truncate and finsert/fcollapse out of fs-io.c, and moves them to io_misc.c. This is prep work for logging these operations, to make them atomic in the event of a crash. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
More reorganization, this splits up io.c into - io_read.c - io_misc.c - fallocate, fpunch, truncate - io_write.c Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Factor out a slowpath into a separate function. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
In the bch2_fs_alloc() error path we call bch2_fs_free() without setting BCH_FS_STOPPING - this is fine. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
bch2_quota_read(), when scanning for inodes, may attempt to look up inodes that have been deleted in the main subvolume - this is not an error. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
blk_mode_t was recently introduced; we should be using it now, instead of fmode_t. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
When we handle a transaction restart in a nested context, we need to return -BCH_ERR_transaction_restart_nested because we invalidated the outer context's iterators and locks. bch2_propagate_key_to_snapshot_leaves() wasn't doing this, this patch fixes it to use trans_was_restarted(). Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
This changes mark_btree_node_locked() to take an enum btree_node_locked_type, not a six_lock_type, since BTREE_NODE_UNLOCKED is -1 which may cause problems converting back and forth to six_lock_type if short enums are in use. With this change, we never store BTREE_NODE_UNLOCKED in a six_lock_type enum. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
It's no longer legal to use a zero size array as a flexible array member - this causes UBSAN to complain. This patch switches our zero size arrays to normal flexible array members when possible, and inserts casts in other places (e.g. where we use the zero size array as a marker partway through an array). Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
We can now print out acls from bch2_xattr_to_text(), when the xattr contains an acl. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Brian Foster authored
Commit c2d5ff36065a4 ("bcachefs: Start journal reclaim thread earlier") tweaked reclaim thread management to start a bit earlier in the mount sequence by moving the start call from __bch2_fs_read_write() to bch2_fs_journal_start(). This has the side effect of never starting the reclaim thread on a ro->rw transition, which can be observed by monitoring reclaim behavior via the journal_reclaim tracepoints. I.e. once an fs has remounted ro->rw, we only ever rely on direct reclaim from that point forward. Since bch2_journal_reclaim_start() properly handles the case where the reclaim thread has already been created, restore the start call in the read-write helper. This allows the reclaim thread to start early when appropriate and also exit/restart on remounts or freeze cycles. In the latter case it may be possible to simply allow the task to freeze rather than destroy it, but for now just fix the immediate bug. Signed-off-by: Brian Foster <bfoster@redhat.com> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
We weren't correctly checking snapshot skiplist nodes - we were checking if they were in the same tree, not if they were an actual ancestor. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Since we set bucket data type to BCH_DATA_stripe based on the data pointer, not just the stripe pointer, it doesn't make sense to check for no stripe in the .key_invalid method - this is a situation that shouldn't happen, but our other fsck/repair code handles it. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Print more information out about moving contexts - fold in the output of the redundant bch2_data_jobs_to_text(), and also include information relevant to whether move_data() should be blocked. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
When doing updates early in recovery, before we can go RW, we still want to check that keys are valid at commit time - this moves key invalid checking to before the "btree updates to journal" path. 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
If fsck finds a key that needs work done, the primary example being an unlinked inode that needs to be deleted, and the key is in an internal snapshot node, we have a bit of a conundrum. The conundrum is that internal snapshot nodes are shared, and we in general do updates in internal snapshot nodes because there may be overwrites in some snapshots and not others, and this may affect other keys referenced by this key (i.e. extents). For example, we might be seeing an unlinked inode in an internal snapshot node, but then in one child snapshot the inode might have been reattached and might not be unlinked. Deleting the inode in the internal snapshot node would be wrong, because then we'll delete all the extents that the child snapshot references. But if an unlinked inode does not have any overwrites in child snapshots, we're fine: the inode is overwrritten in all child snapshots, so we can do the deletion at the point of comonality in the snapshot tree, i.e. the node where we found it. This patch adds a new helper, bch2_propagate_key_to_snapshot_leaves(), to handle the case where we need a to update a key that does have overwrites in child snapshots: we copy the key to leaf snapshot nodes, and then rewind fsck and process the needed updates there. With this, fsck can now always correctly handle unlinked inodes found in internal snapshot nodes. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
After deleteing snapshots, we may be left with a snapshot tree where some nodes only have one child, and we have a linear chain. Interior snapshot nodes are never used directly (i.e. they never have subvolumes that point to them), they are only referered to by child snapshot nodes - hence, they are redundant. The existing code talks about redundant snapshot nodes as forming and equivalence class; i.e. nodes for which snapshot_t->equiv is equal. In a given equivalence class, we only ever need a single key at a given position - i.e. multiple versions with different snapshot fields are redundant. The existing snapshot cleanup code deletes these redundant keys, but not redundant nodes. It turns out this is buggy, because we assume that after snapshot deletion finishes we should only have a single key per equivalence class, but the btree update path doesn't preserve this - overwriting keys in old snapshots doesn't check for the equivalence class being equal, and thus we can end up with duplicate keys in the same equivalence class and fsck complaining about snapshot deletion not having run correctly. The equivalence class notion has been leaking out of the core snapshots code and into too much other code, i.e. fsck, so this patch takes a different approach: snapshot deletion now moves keys to the node in an equivalence class being kept (the leafiest node) and then deletes the redundant nodes in the equivalance class. Some work has to be done to correctly delete interior snapshot nodes; snapshot node depth and skiplist fields for descendent nodes have to be fixed. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
The is_ancestor bitmap is at optimization for bch2_snapshot_is_ancestor; once we get sufficiently close to the ancestor ID we're searching for we test a bitmap. But initialization of the is_ancestor bitmap was broken; we do it by using bch2_snapshot_parent(), but we call that on nodes that haven't been initialized yet with bch2_mark_snapshot(). Fix this by adding a separate loop in bch2_snapshots_read() for initializing the is_ancestor bitmap, and also add some new debug asserts for checking this sort of breakage in the future. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-
Kent Overstreet authored
In the bch2_mount() error path, we were calling deactivate_locked_super(), which calls ->kill_sb(), which in our case was calling bch2_fs_free() without __bch2_fs_stop(). This changes bch2_mount() to just call bch2_fs_stop() directly. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
-