| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Introduce checks for FREE_SPACE_INFO item, which include:
- Key alignment check
The objectid is the logical bytenr of the chunk/bg, and offset is the
length of the chunk/bg, thus they should all be aligned to the fs
block size.
- Item size check
The FREE_SPACE_INFO should a fix size.
- Flags check
The flags member should have no other flags than
BTRFS_FREE_SPACE_USING_BITMAPS.
For future expansion, introduce a new macro
BTRFS_FREE_SPACE_FLAGS_MASK for such checks.
And since we're here, the BTRFS_FREE_SPACE_USING_BITMAPS should not
use unsigned long long, as the flags is only 32 bits wide.
So fix that to use unsigned long.
- Extent count check
That member shows how many free space bitmap/extent items there are
inside the chunk/bg.
We know the chunk size (from key->offset), thus there should be at
most (key->offset >> sectorsize_bits) blocks inside the chunk.
Use that value as the upper limit and if that counter is larger than
that, there is a high chance it's a bitflip in high bits.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Limit the number of zones reclaimed in flush_space()'s RECLAIM_ZONES
state.
This prevents possibly long running reclaim sweeps to block other tasks in
the system, while the system is under pressure anyways, causing the
tasks to hang.
An example of this can be seen here, triggered by fstests generic/551:
generic/551 [ 27.042349] run fstests generic/551 at 2026-02-27 11:05:30
BTRFS: device fsid 78c16e29-20d9-4c8e-bc04-7ba431be38ff devid 1 transid 8 /dev/vdb (254:16) scanned by mount (806)
BTRFS info (device vdb): first mount of filesystem 78c16e29-20d9-4c8e-bc04-7ba431be38ff
BTRFS info (device vdb): using crc32c checksum algorithm
BTRFS info (device vdb): host-managed zoned block device /dev/vdb, 64 zones of 268435456 bytes
BTRFS info (device vdb): zoned mode enabled with zone size 268435456
BTRFS info (device vdb): checking UUID tree
BTRFS info (device vdb): enabling free space tree
INFO: task kworker/u38:1:90 blocked for more than 120 seconds.
Not tainted 7.0.0-rc1+ #345
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:kworker/u38:1 state:D stack:0 pid:90 tgid:90 ppid:2 task_flags:0x4208060 flags:0x00080000
Workqueue: events_unbound btrfs_async_reclaim_data_space
Call Trace:
<TASK>
__schedule+0x34f/0xe70
schedule+0x41/0x140
schedule_timeout+0xa3/0x110
? mark_held_locks+0x40/0x70
? lockdep_hardirqs_on_prepare+0xd8/0x1c0
? trace_hardirqs_on+0x18/0x100
? lockdep_hardirqs_on+0x84/0x130
? _raw_spin_unlock_irq+0x33/0x50
wait_for_completion+0xa4/0x150
? __flush_work+0x24c/0x550
__flush_work+0x339/0x550
? __pfx_wq_barrier_func+0x10/0x10
? wait_for_completion+0x39/0x150
flush_space+0x243/0x660
? find_held_lock+0x2b/0x80
? kvm_sched_clock_read+0x11/0x20
? local_clock_noinstr+0x17/0x110
? local_clock+0x15/0x30
? lock_release+0x1b7/0x4b0
do_async_reclaim_data_space+0xe8/0x160
btrfs_async_reclaim_data_space+0x19/0x30
process_one_work+0x20a/0x5f0
? lock_is_held_type+0xcd/0x130
worker_thread+0x1e2/0x3c0
? __pfx_worker_thread+0x10/0x10
kthread+0x103/0x150
? __pfx_kthread+0x10/0x10
ret_from_fork+0x20d/0x320
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
Showing all locks held in the system:
1 lock held by khungtaskd/67:
#0: ffffffff824d58e0 (rcu_read_lock){....}-{1:3}, at: debug_show_all_locks+0x3d/0x194
2 locks held by kworker/u38:1/90:
#0: ffff8881000aa158 ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work+0x3c4/0x5f0
#1: ffffc90000c17e58 ((work_completion)(&fs_info->async_data_reclaim_work)){+.+.}-{0:0}, at: process_one_work+0x1c0/0x5f0
5 locks held by kworker/u39:1/191:
#0: ffff8881000aa158 ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work+0x3c4/0x5f0
#1: ffffc90000dfbe58 ((work_completion)(&fs_info->reclaim_bgs_work)){+.+.}-{0:0}, at: process_one_work+0x1c0/0x5f0
#2: ffff888101da0420 (sb_writers#9){.+.+}-{0:0}, at: process_one_work+0x20a/0x5f0
#3: ffff88811040a648 (&fs_info->reclaim_bgs_lock){+.+.}-{4:4}, at: btrfs_reclaim_bgs_work+0x1de/0x770
#4: ffff888110408a18 (&fs_info->cleaner_mutex){+.+.}-{4:4}, at: btrfs_relocate_block_group+0x95a/0x20f0
1 lock held by aio-dio-write-v/980:
#0: ffff888110093008 (&sb->s_type->i_mutex_key#15){++++}-{4:4}, at: btrfs_inode_lock+0x51/0xb0
=============================================
To prevent these long running reclaims from blocking the system, only
reclaim 5 block_groups in the RECLAIM_ZONES state of flush_space(). Also
as these reclaims are now constrained, it opens up the use for a
synchronous call to brtfs_reclaim_block_groups(), eliminating the need
to place the reclaim task on a workqueue and then flushing the workqueue
again.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Create a function btrfs_reclaim_block_groups() that gets called from the
block-group reclaim worker.
This allows creating synchronous block_group reclaim later on.
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The main work of reclaiming a single block-group in
btrfs_reclaim_bgs_work() is done inside the loop iterating over all the
block_groups in the fs_info->reclaim_bgs list.
Factor out reclaim of a single block group from the loop to improve
readability.
No functional change intended.
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Currently we call cow_file_range_inline() in different situations, from
regular cow_file_range() to compress_file_range().
This is because inline extent creation has different conditions based on
whether it's a compressed one or not.
But on the other hand, inline extent creation shouldn't be so
distributed, we can just have a dedicated branch in
btrfs_run_delalloc_range().
It will become more obvious for compressed inline cases, it makes no
sense to go through all the complex async extent mechanism just to
inline a single block.
So here we introduce a dedicated run_delalloc_inline() helper, and
remove all inline related handling from cow_file_range() and
compress_file_range().
There is a special update to inode_need_compress(), that a new
@check_inline parameter is introduced.
This is to allow inline specific checks to be done inside
run_delalloc_inline(), which allows single block compression, but
other call sites should always reject single block compression.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Previously we have to call mapping_set_error() inside the
for_each_folio_all() loop, because we do not have a better way to grab
an inode, other than through folio->mapping.
But nowadays every btrfs_bio has its inode member populated, thus we can
easily grab the inode and its i_mapping easily, without the help from a
folio.
Now we can move that mapping_set_error() out of the loop, and use
bbio->inode to grab the i_mapping.
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The check is not necessary because:
- There is already assert_bbio_alignment() at btrfs_submit_bbio()
- There is also btrfs_subpage_assert() for all btrfs_folio_*() helpers
- The original commit mentions the check may go away in the future
Commit 17a5adccf3fd01 ("btrfs: do away with non-whole_page extent
I/O") introduced the check first, and in the commit message:
I've replaced the whole_page computations with warnings, just to be
sure that we're not issuing partial page reads or writes. The
warnings should probably just go away some time.
- No similar check in all other endio functions
No matter if it's data read, compressed read or write.
- There is no such report for very long
I do not even remember if there is any such report.
Thus the need to do such check in end_bbio_data_write() is very weak,
and we can just get rid of it.
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Add a btrfs_search_slot_restart tracepoint that fires at each restart
site in btrfs_search_slot(), recording the root, tree level, and
reason for the restart. This enables tracking search slot restarts
which contribute to COW amplification under memory pressure.
The four restart reasons are:
- write_lock: insufficient write lock level, need to restart with
higher lock
- setup_nodes: node setup returned -EAGAIN
- slot_zero: insertion at slot 0 requires higher write lock level
- read_block: read_block_for_search returned -EAGAIN (block not
cached or lock contention)
COW counts are already tracked by the existing trace_btrfs_cow_block()
tracepoint. The per-restart-site tracepoint avoids counter overhead
in the critical path when tracepoints are disabled, and provides
richer per-event information that bpftrace scripts can aggregate into
counts, histograms, and per-root breakdowns.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Leo Martins <loemra.dev@gmail.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Inhibit writeback on COW'd extent buffers for the lifetime of the
transaction handle, preventing background writeback from setting
BTRFS_HEADER_FLAG_WRITTEN and causing unnecessary re-COW.
COW amplification occurs when background writeback flushes an extent
buffer that a transaction handle is still actively modifying. When
lock_extent_buffer_for_io() transitions a buffer from dirty to
writeback, it sets BTRFS_HEADER_FLAG_WRITTEN, marking the block as
having been persisted to disk at its current bytenr. Once WRITTEN is
set, should_cow_block() must either COW the block again or overwrite
it in place, both of which are unnecessary overhead when the buffer
is still being modified by the same handle that allocated it. By
inhibiting background writeback on actively-used buffers, WRITTEN is
never set while a transaction handle holds a reference to the buffer,
avoiding this overhead entirely.
Add an atomic_t writeback_inhibitors counter to struct extent_buffer,
which fits in an existing 6-byte hole without increasing struct size.
When a buffer is COW'd in btrfs_force_cow_block(), call
btrfs_inhibit_eb_writeback() to store the eb in the transaction
handle's writeback_inhibited_ebs xarray (keyed by eb->start), take a
reference, and increment writeback_inhibitors. The function handles
dedup (same eb inhibited twice by the same handle) and replacement
(different eb at the same logical address). Allocation failure is
graceful: the buffer simply falls back to the pre-existing behavior
where it may be written back and re-COW'd.
Also inhibit writeback in should_cow_block() when COW is skipped,
so that every transaction handle that reuses an already-COW'd buffer
also inhibits its writeback. Without this, if handle A COWs a block
and inhibits it, and handle B later reuses the same block without
inhibiting, handle A's uninhibit on end_transaction leaves the buffer
unprotected while handle B is still using it. This ensures all handles
that access a COW'd buffer contribute to the inhibitor count, and the
buffer remains protected until the last handle releases it.
In lock_extent_buffer_for_io(), when writeback_inhibitors is non-zero
and the writeback mode is WB_SYNC_NONE, skip the buffer. WB_SYNC_NONE
is used by the VM flusher threads for background and periodic
writeback, which are the only paths that cause COW amplification by
opportunistically writing out dirty extent buffers mid-transaction.
Skipping these is safe because the buffers remain dirty in the page
cache and will be written out at transaction commit time.
WB_SYNC_ALL must always proceed regardless of writeback_inhibitors.
This is required for correctness in the fsync path: btrfs_sync_log()
writes log tree blocks via filemap_fdatawrite_range() (WB_SYNC_ALL)
while the transaction handle that inhibited those same blocks is still
active. Without the WB_SYNC_ALL bypass, those inhibited log tree
blocks would be silently skipped, resulting in an incomplete log on
disk and corruption on replay. btrfs_write_and_wait_transaction()
also uses WB_SYNC_ALL via filemap_fdatawrite_range(); for that path,
inhibitors are already cleared beforehand, but the bypass ensures
correctness regardless.
Uninhibit in __btrfs_end_transaction() before atomic_dec(num_writers)
to prevent a race where the committer proceeds while buffers are still
inhibited. Also uninhibit in btrfs_commit_transaction() before writing
and in cleanup_transaction() for the error path.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Sun YangKai <sunk67188@gmail.com>
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Leo Martins <loemra.dev@gmail.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We have two locations using open-coded 512K size, as the async chunk
size.
For compression we have not only the max size a compressed extent can
represent (128K), but also how large an async chunk can be (512K).
Although we have a macro for the maximum compressed extent size, we do
not have any macro for the async chunk size.
Add such a macro and replace the two open-coded SZ_512K.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We're under the IS_ERR() branch so we know that 'ret', which got assigned
the value of PTR_ERR(di) is always negative, so there's no point in
checking if it's negative.
Reviewed-by: Boris Burkov <boris@bur.io>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
There's no point in checking if the uuid root exists in
btrfs_uuid_tree_add(), since we already do it in btrfs_uuid_tree_lookup().
We can just remove the check from btrfs_uuid_tree_add() and make
btrfs_uuid_tree_lookup() return -EINVAL instead of -ENOENT in case the
uuid tree does not exists.
Reviewed-by: Boris Burkov <boris@bur.io>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We never return -EEXIST from btrfs_uuid_tree_add(), if the item already
exists we extend it, so it's pointless to check for such return value.
Furthermore, in create_pending_snapshot(), the logic is completely broken.
The goal was to not error out and abort the transaction in case of -EEXIST
but we left 'ret' with the -EEXIST value, so we end up setting
pending->error to -EEXIST and return that error up the call chain up to
btrfs_commit_transaction(), which will abort the transaction.
Reviewed-by: Boris Burkov <boris@bur.io>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Commit 347b7042fb26 ("Merge patch series "fs: generic file IO error
reporting"") has introduced a common framework for reporting errors to
fsnotify in a standard way.
One of the functions being introduced is fserror_report_shutdown() that,
when combined with the experimental support for shutdown in btrfs, it
means that user-space can also easily detect whenever a btrfs filesystem
has been marked as shutdown.
Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Commit 2932ba8d9c99 ("slab: Introduce kmalloc_obj() and family")
introduced, among many others, the kzalloc_objs() helper, which has some
benefits over kcalloc(). Namely, internal introspection of the allocated
type now becomes possible, allowing for future alignment-aware choices
to be made by the allocator and future hardening work that can be type
sensitive. Dropping 'sizeof' comes also as a nice side-effect.
Moreover, this also allows us to be in line with the recent tree-wide
migration to the kmalloc_obj() and family of helpers. See
commit 69050f8d6d07 ("treewide: Replace kmalloc with kmalloc_obj for
non-scalar types").
Reviewed-by: Kees Cook <kees@kernel.org>
Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Currently we zero out all the remaining bytes of the last folio of
the compressed bio, then round the bio size to fs block boundary.
But that is done in two different functions, zero_last_folio() to zero
the remaining bytes of the last folio, and round_up_last_block() to
round up the bio to fs block boundary.
There are some minor problems:
- zero_last_folio() is zeroing ranges we won't submit
This is mostly affecting block size < page size cases, where we can
have a large folio (e.g. 64K), but the fs block size is only 4K.
In that case, we may only want to submit the first 4K of the folio,
the remaining range won't matter, but we still zero them all.
This causes unnecessary CPU usage just to zero out some bytes we won't
utilized.
- compressed_bio_last_folio() is called twice in two different functions
Which in theory we only need to call it once.
Enhance the situation by:
- Only zero out bytes up to the fs block boundary
Thus this will reduce some overhead for bs < ps cases.
- Move the folio_zero_range() call into round_up_last_block()
So that we can reuse the same folio returned by
compressed_bio_last_folio().
Reviewed-by: Anand Jain <asj@kernel.org>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The member compressed_bio::compressed_len can be replaced by the bio
size, as we always submit the full compressed data without any partial
read/write.
Furthermore we already have enough ASSERT()s making sure the bio size
matches the ordered extent or the extent map.
This saves 8 bytes from compressed_bio:
Before:
struct compressed_bio {
u64 start; /* 0 8 */
unsigned int len; /* 8 4 */
unsigned int compressed_len; /* 12 4 */
u8 compress_type; /* 16 1 */
bool writeback; /* 17 1 */
/* XXX 6 bytes hole, try to pack */
struct btrfs_bio * orig_bbio; /* 24 8 */
struct btrfs_bio bbio __attribute__((__aligned__(8))); /* 32 304 */
/* XXX last struct has 1 bit hole */
/* size: 336, cachelines: 6, members: 7 */
/* sum members: 330, holes: 1, sum holes: 6 */
/* member types with bit holes: 1, total: 1 */
/* forced alignments: 1 */
/* last cacheline: 16 bytes */
} __attribute__((__aligned__(8)));
After:
struct compressed_bio {
u64 start; /* 0 8 */
unsigned int len; /* 8 4 */
u8 compress_type; /* 12 1 */
bool writeback; /* 13 1 */
/* XXX 2 bytes hole, try to pack */
struct btrfs_bio * orig_bbio; /* 16 8 */
struct btrfs_bio bbio __attribute__((__aligned__(8))); /* 24 304 */
/* XXX last struct has 1 bit hole */
/* size: 328, cachelines: 6, members: 6 */
/* sum members: 326, holes: 1, sum holes: 2 */
/* member types with bit holes: 1, total: 1 */
/* forced alignments: 1 */
/* last cacheline: 8 bytes */
} __attribute__((__aligned__(8)));
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We have several call sites doing the same work to calculate the size of
a bio:
struct bio_vec *bvec;
u32 bio_size = 0;
int i;
bio_for_each_bvec_all(bvec, bio, i)
bio_size += bvec->bv_len;
We can use a common helper instead of open-coding it everywhere.
This also allows us to constify the @bio_size variables used in all the
call sites.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The nowait flag is always false in this context, making the conditional
check unnecessary. Simplify the code by directly assigning -ENOTBLK.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Alexey Velichayshiy <a.velichayshiy@ispras.ru>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Fix the unlikely added to btrfs_insert_one_raid_extent() by commit
a929904cf73b65 ("btrfs: add unlikely annotations to branches leading to
transaction abort"): the exclamation point is in the wrong place, so we
are telling the compiler that allocation failure is actually expected.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
All internal functions should be given a btrfs_inode for consistency and
not a VFS inode. So pass a btrfs_inode instead of a VFS inode.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
It's useless to print the result of the condition, it's always 0 if the
assertion is triggered, so it doesn't provide any useful information.
Examples:
assertion failed: cb->bbio.bio.bi_iter.bi_size == disk_num_bytes :: 0, in inode.c:9991
assertion failed: folio_test_writeback(folio) :: 0, in subpage.c:476
So stop printing that, it's always ":: 0" for any assertion triggered
(except for conditions that are just an identifier).
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
There are several functions that take pointer arguments but don't need to
modify the objects they point to, so add the const qualifiers.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
There's no need to COW the root node of the subvolume we are snapshotting
because we then call btrfs_copy_root(), which creates a copy of the root
node and sets its generation to the current transaction. So remove this
redundant COW right before calling btrfs_copy_root(), saving one extent
allocation, memory allocation, copying things, etc, and making the code
less confusing. Also rename the extent buffer variable from "old" to
"root_eb" since that name no longer makes any sense after removing the
unnecessary COW operation.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
When a snapshot is being created, the atomic counter snapshot_force_cow
is incremented to force incoming writes to fallback to COW. This is a
critical mechanism to protect the consistency of the snapshot being taken.
Currently, can_nocow_file_extent() checks this counter only after
performing several checks, most notably the expensive cross-reference
check via btrfs_cross_ref_exist(). btrfs_cross_ref_exist() releases the
path and performs a search in the extent tree or backref cache, which
involves btree traversals and locking overhead.
Moves the snapshot_force_cow check to the very beginning of
can_nocow_file_extent().
This reordering is safe and beneficial because:
1. args->writeback_path is invariant for the duration of the call (set
by caller run_delalloc_nocow).
2. is_freespace_inode is a static property of the inode.
3. The state of snapshot_force_cow is driven by the btrfs_mksnapshot()
process. Checking it earlier does not change the outcome of the NOCOW
decision, but effectively prunes the expensive code path when a
fallback to COW is inevitable.
By failing fast when a snapshot is pending, we avoid the unnecessary
overhead of btrfs_cross_ref_exist() and other extent item checks in the
scenario where NOCOW is already known to be impossible.
Signed-off-by: Chen Guan Jie <jk.chen1095@gmail.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
[BUG]
The following sequence will set the file with nocompress flag:
# mkfs.btrfs -f $dev
# mount $dev $mnt -o max_inline=4,compress
# xfs_io -f -c "pwrite 0 2k" -c sync $mnt/foobar
The inode will have NOCOMPRESS flag, even if the content itself (all 0xcd)
can still be compressed very well:
item 4 key (257 INODE_ITEM 0) itemoff 15879 itemsize 160
generation 9 transid 10 size 2097152 nbytes 1052672
block group 0 mode 100600 links 1 uid 0 gid 0 rdev 0
sequence 257 flags 0x8(NOCOMPRESS)
Please note that, this behavior is there even before commit 59615e2c1f63
("btrfs: reject single block sized compression early").
[CAUSE]
At compress_file_range(), after btrfs_compress_folios() call, we try
making an inlined extent by calling cow_file_range_inline().
But cow_file_range_inline() calls can_cow_file_range_inline() which has
more accurate checks on if the range can be inlined.
One of the user configurable conditions is the "max_inline=" mount
option. If that value is set low (like the example, 4 bytes, which
cannot store any header), or the compressed content is just slightly
larger than 2K (the default value, meaning a 50% compression ratio),
cow_file_range_inline() will return 1 immediately.
And since we're here only to try inline the compressed data, the range
is no larger than a single fs block.
Thus compression is never going to make it a win, we fall back to
marking the inode incompressible unavoidably.
[FIX]
Just add an extra check after inline attempt, so that if the inline
attempt failed, do not set the nocompress flag.
As there is no way to remove that flag, and the default 50% compression
ratio is way too strict for the whole inode.
CC: stable@vger.kernel.org # 6.12+
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Both functions btrfs_finish_ordered_extent() and
btrfs_mark_ordered_io_finished() are accepting an optional folio
parameter.
That @folio is passed into can_finish_ordered_extent(), which later will
test and clear the ordered flag for the involved range.
However I do not think there is any other call site that can clear
ordered flags of an page cache folio and can affect
can_finish_ordered_extent().
There are limited *_clear_ordered() callers out of
can_finish_ordered_extent() function:
- btrfs_migrate_folio()
This is completely unrelated, it's just migrating the ordered flag to
the new folio.
- btrfs_cleanup_ordered_extents()
We manually clean the ordered flags of all involved folios, then call
btrfs_mark_ordered_io_finished() without a @folio parameter.
So it doesn't need and didn't pass a @folio parameter in the first
place.
- btrfs_writepage_fixup_worker()
This function is going to be removed soon, and we should not hit that
function anymore.
- btrfs_invalidate_folio()
This is the real call site we need to bother with.
If we already have a bio running, btrfs_finish_ordered_extent() in
end_bbio_data_write() will be executed first, as
btrfs_invalidate_folio() will wait for the writeback to finish.
Thus if there is a running bio, it will not see the range has
ordered flags, and just skip to the next range.
If there is no bio running, meaning the ordered extent is created but
the folio is not yet submitted.
In that case btrfs_invalidate_folio() will manually clear the folio
ordered range, but then manually finish the ordered extent with
btrfs_dec_test_ordered_pending() without bothering the folio ordered
flags.
Meaning if the OE range with folio ordered flags will be finished
manually without the need to call can_finish_ordered_extent().
This means all can_finish_ordered_extent() call sites should get a range
that has folio ordered flag set, thus the old "return false" branch
should never be triggered.
Now we can:
- Remove the @folio parameter from involved functions
* btrfs_mark_ordered_io_finished()
* btrfs_finish_ordered_extent()
For call sites passing a @folio into those functions, let them
manually clear the ordered flag of involved folios.
- Move btrfs_finish_ordered_extent() out of the loop in
end_bbio_data_write()
We only need to call btrfs_finish_ordered_extent() once per bbio,
not per folio.
- Add an ASSERT() to make sure all folio ranges have ordered flags
It's only for end_bbio_data_write().
And we already have enough safe nets to catch over-accounting of ordered
extents.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We already have btrfs_ordered_extent::inode, thus there is no need to
pass a btrfs_inode parameter to btrfs_remove_ordered_extent().
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
There is a lengthy comment introduced in commit b3ff8f1d380e ("btrfs:
Don't submit any btree write bio if the fs has errors") and commit
c9583ada8cc4 ("btrfs: avoid double clean up when submit_one_bio()
failed"), explaining two things:
- Why we don't want to submit metadata write if the fs has errors
- Why we re-set @ret to 0 if it's positive
However it's no longer uptodate by the following reasons:
- We have better checks nowadays
Commit 2618849f31e7 ("btrfs: ensure no dirty metadata is written back
for an fs with errors") has introduced better checks, that if the
fs is in an error state, metadata writes will not result in any bio
but instead complete immediately.
That covers all metadata writes better.
- Mentioned incorrect function name
The commit c9583ada8cc4 ("btrfs: avoid double clean up when
submit_one_bio() failed") introduced this ret > 0 handling, but at that
time the function name submit_extent_page() was already incorrect.
It was submit_eb_page() that could return >0 at that time,
and submit_extent_page() could only return 0 or <0 for errors, never >0.
Later commit b35397d1d325 ("btrfs: convert submit_extent_page() to use
a folio") changed "submit_extent_page()" to "submit_extent_folio()" in
the comment, but it doesn't make any difference since the function name
is wrong from day 1.
Finally commit 5e121ae687b8 ("btrfs: use buffer xarray for extent
buffer writeback operations") completely reworked how metadata
writeback works, and removed submit_eb_page(), leaving only the wrong
function name in the comment.
Furthermore the function submit_extent_folio() still exists in the
latest code base, but is never utilized for metadata writeback, causing
more confusion.
Just remove the lengthy comment, and replace the "if (ret > 0)" check
with an ASSERT(), since only btrfs_check_meta_write_pointer() can modify
@ret and it returns 0 or <0 for errors.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
There's no need to pass the maximum between the block group's start offset
and BTRFS_SUPER_INFO_OFFSET (64K) since we can't have any block groups
allocated in the first megabyte, as that's reserved space. Furthermore,
even if we could, the correct thing to do was to pass the block group's
start offset anyway - and that's precisely what we do for block groups
hat happen to contain a superblock mirror (the range for the super block
is never marked as free and it's marked as dirty in the
fs_info->excluded_extents io tree).
So simplify this and get rid of that maximum expression.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Remove duplicate inclusion of delayed-inode.h in disk-io.c to clean up
redundant code.
Signed-off-by: Chen Ni <nichen@iscas.ac.cn>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We have several functions with parameters defined as booleans but then we
have callers passing integers, 0 or 1, instead of false and true. While
this isn't a bug since 0 and 1 are converted to false and true, it is odd
and less readable. Change the callers to pass true and false literals
instead.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The 'out' label is pointless as there are no cleanups to perform there,
we can replace every goto with a direct return.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The function btrfs_csum_file_blocks() is a little confusing, unlike
btrfs_csum_one_bio(), it is not calculating the checksum of some file
blocks.
Instead it's just inserting the already calculated checksums into a given
root (can be a csum root or a log tree).
So rename it to btrfs_insert_data_csums() to reflect its behavior better.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The structure btrfs_ordered_extent has a lot of list heads for different
purposes, passing a random list_head pointer is never a good idea as if
the wrong list is passed in, the type casting along with the fs will be
screwed up.
Instead pass the btrfs_ordered_extent pointer, and grab the csum_list
inside add_pending_csums() to make it a little safer.
Since we're here, also update the comments to follow the current style.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
That list head records all pending checksums for that ordered
extent. And unlike other lists, we just use the name "list", which can
be very confusing for readers.
Rename it to "csum_list" which follows the remaining lists, showing the
purpose of the list.
And since we're here, remove a comment inside
btrfs_finish_ordered_zoned() where we have
"ASSERT(!list_empty(&ordered->csum_list))" to make sure the OE has
pending csums.
That comment is only here to make sure we do not call list_first_entry()
before checking BTRFS_ORDERED_PREALLOC.
But since we already have that bit checked and even have a dedicated
ASSERT(), there is no need for that comment anymore.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
None of the callers of `cache_save_setup` care about the return type as
the space cache is purely and optimization.
Also the free space cache is a deprecated feature that is being phased
out.
Change the return type of `cache_save_setup` to void to reflect this.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We join a transaction with the goal of catching the current transaction
and then commit it to get rid of pinned extents and reclaim free space,
but a join can create a new transaction if there isn't any running, and if
right before we did the join the current transaction happened to be
committed by someone else (like the transaction kthread for example),
we end up starting and committing a new transaction, causing rotation of
the super block backup roots besides extra and useless IO.
So instead of doing a transaction join followed by a commit, use the
helper btrfs_commit_current_transaction() which ensures no transaction is
created if there isn't any running.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We have several places that call extent_buffer_uptodate() after reading a
tree block with read_tree_block(), but that is redundant since we already
call extent_buffer_uptodate() in the call chain of read_tree_block():
read_tree_block()
btrfs_read_extent_buffer()
read_extent_buffer_pages()
returns -EIO if extent_buffer_uptodate() returns false
So remove those redundant checks.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Instead of open coding testing the uptodate bit on the extent buffer's
flags, use the existing helper extent_buffer_uptodate() (which is even
shorter to type). Also change the helper's return value from int to bool,
since we always use it in a boolean context.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
On zoned block devices, DATA block groups can accumulate large amounts
of zone_unusable space (space between the write pointer and zone end).
When zone_unusable reaches high levels (e.g., 98% of total space), new
allocations fail with ENOSPC even though space could be reclaimed by
relocating data and resetting zones.
The existing flush states don't handle this scenario effectively - they
either try to free cached space (which doesn't exist for zone_unusable)
or reset empty zones (which doesn't help when zones contain valid data
mixed with zone_unusable space).
Add a new RECLAIM_ZONES flush state that triggers the block group
reclaim machinery. This state:
- Calls btrfs_reclaim_sweep() to identify reclaimable block groups
- Calls btrfs_reclaim_bgs() to queue reclaim work
- Waits for reclaim_bgs_work to complete via flush_work()
- Commits the transaction to finalize changes
The reclaim work (btrfs_reclaim_bgs_work) safely relocates valid data
from fragmented block groups to other locations before resetting zones,
converting zone_unusable space back into usable space.
Insert RECLAIM_ZONES before RESET_ZONES in data_flush_states so that
we attempt to reclaim partially-used block groups before falling back
to resetting completely empty ones.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
On zoned block devices, block groups accumulate zone_unusable space
(space between the write pointer and zone end that cannot be allocated
until the zone is reset). When a block group becomes mostly
zone_unusable but still contains some valid data and it gets added to the
unused_bgs list it can never be deleted because it's not actually empty.
The deletion code (btrfs_delete_unused_bgs) skips these block groups
due to the btrfs_is_block_group_used() check, leaving them on the
unused_bgs list indefinitely. This causes two problems:
1. The block groups are never reclaimed, permanently wasting space
2. Eventually leads to ENOSPC even though reclaimable space exists
Fix by detecting block groups where zone_unusable exceeds 50% of the
block group size. Move these to the reclaim_bgs list instead of
skipping them. This triggers btrfs_reclaim_bgs_work() which:
1. Marks the block group read-only
2. Relocates the remaining valid data via btrfs_relocate_chunk()
3. Removes the emptied block group
4. Resets the zones, converting zone_unusable back to usable space
The 50% threshold ensures we only reclaim block groups where most space
is unusable, making relocation worthwhile. Block groups with less
zone_unusable are left on unused_bgs to potentially become fully empty
through normal deletion.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
On zoned filesystems metadata space accounting can become overly optimistic
due to delayed refs reservations growing without a hard upper bound.
The delayed_refs_rsv block reservation is allowed to speculatively grow and
is only backed by actual metadata space when refilled. On zoned devices this
can result in delayed_refs_rsv reserving a large portion of metadata space
that is already effectively unusable due to zone write pointer constraints.
As a result, space_info->may_use can grow far beyond the usable metadata
capacity, causing the allocator to believe space is available when it is not.
This leads to premature ENOSPC failures and "cannot satisfy tickets" reports
even though commits would be able to make progress by flushing delayed refs.
Analysis of "-o enospc_debug" dumps using a Python debug script
confirmed that delayed_refs_rsv was responsible for the majority of
metadata overcommit on zoned devices. By correlating space_info counters
(total, used, may_use, zone_unusable) across transactions, the analysis
showed that may_use continued to grow even after usable metadata space
was exhausted, with delayed refs refills accounting for the excess
reservations.
Here's the output of the analysis:
======================================================================
Space Type: METADATA
======================================================================
Raw Values:
Total: 256.00 MB (268435456 bytes)
Used: 128.00 KB (131072 bytes)
Pinned: 16.00 KB (16384 bytes)
Reserved: 144.00 KB (147456 bytes)
May Use: 255.48 MB (267894784 bytes)
Zone Unusable: 192.00 KB (196608 bytes)
Calculated Metrics:
Actually Usable: 255.81 MB (total - zone_unusable)
Committed: 255.77 MB (used + pinned + reserved + may_use)
Consumed: 320.00 KB (used + zone_unusable)
Percentages:
Zone Unusable: 0.07% of total
May Use: 99.80% of total
Fix this by adding a zoned-specific cap in btrfs_delayed_refs_rsv_refill():
Before reserving additional metadata bytes, limit the delayed refs
reservation based on the usable metadata space (total bytes minus
zone_unusable). If the reservation would exceed this cap, return -EAGAIN
to trigger the existing flush/commit logic instead of overcommitting
metadata space.
This preserves the existing reservation and flushing semantics while
preventing metadata overcommit on zoned devices. The change is limited to
metadata space and does not affect non-zoned filesystems.
This patch addresses premature metadata ENOSPC conditions on zoned devices
and ensures delayed refs are throttled before exhausting usable metadata.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We are calling extent_buffer_uptodate() twice, and the result will not
change before the second call. So remove the second call.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
[BUG]
Since commit 59615e2c1f63 ("btrfs: reject single block sized compression
early"), the following script will result the inode to have NOCOMPRESS
flag, meanwhile old kernels don't:
# mkfs.btrfs -f $dev
# mount $dev $mnt -o max_inline=2k,compress=zstd
# truncate -s 8k $mnt/foobar
# xfs_io -f -c "pwrite 0 2k" $mnt/foobar
# sync
Before that commit, the inode will not have NOCOMPRESS flag:
item 4 key (257 INODE_ITEM 0) itemoff 15879 itemsize 160
generation 9 transid 9 size 8192 nbytes 4096
block group 0 mode 100644 links 1 uid 0 gid 0 rdev 0
sequence 3 flags 0x0(none)
But after that commit, the inode will have NOCOMPRESS flag:
item 4 key (257 INODE_ITEM 0) itemoff 15879 itemsize 160
generation 9 transid 10 size 8192 nbytes 4096
block group 0 mode 100644 links 1 uid 0 gid 0 rdev 0
sequence 3 flags 0x8(NOCOMPRESS)
This will make a lot of files no longer to be compressed.
[CAUSE]
The old compressed inline check looks like this:
if (total_compressed <= blocksize &&
(start > 0 || end + 1 < inode->disk_i_size))
goto cleanup_and_bail_uncompressed;
That inline part check is equal to "!(start == 0 && end + 1 >=
inode->disk_i_size)", but the new check no longer has that disk_i_size
check.
Thus it means any single block sized write at file offset 0 will pass
the inline check, which is wrong.
Furthermore, since we have merged the old check into
inode_need_compress(), there is no disk_i_size based inline check
anymore, we will always try compressing that single block at file offset
0, then later find out it's not a net win and go to the
mark_incompressible tag.
This results the inode to have NOCOMPRESS flag.
[FIX]
Add back the missing disk_i_size based check into inode_need_compress().
Now the same script will no longer cause NOCOMPRESS flag.
Fixes: 59615e2c1f63 ("btrfs: reject single block sized compression early")
Reported-by: Chris Mason <clm@meta.com>
Link: https://lore.kernel.org/linux-btrfs/20260208183840.975975-1-clm@meta.com/
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
In case we have multiple devices, there is no point in setting the written
flag in the super block on every iteration over the device list. Just do
it once before the loop.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
There's no need to pass max_mirrors to write_all_supers() since from the
given transaction handle we can infer if we are in a transaction commit
or fsync context, so we can determine how many mirrors we need to use.
So remove the max_mirror argument from write_all_supers() and stop
adjusting it in the callees write_dev_supers() and wait_dev_supers(),
simplifying them besides the parameter list for write_all_supers().
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Mark all the unexpected error checks as unlikely, to make it more clear
they are unexpected and to allow the compiler to potentially generate
better code.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We are in a transaction context and have an handle, so instead of using
the less preferred btrfs_handle_fs_error(), abort the transaction and
log an error to give some context information.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We are holding a transaction In every context we call write_all_supers(),
so pass the transaction handle instead of fs_info to it. This will allow
us to abort the transaction in write_all_supers() instead of calling
btrfs_handle_fs_error() in a later patch.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
