linux-block.git
4 weeks agobtrfs: fix invalid data space release when truncating block in NOCOW mode
Filipe Manana [Fri, 9 May 2025 16:08:50 +0000 (17:08 +0100)]
btrfs: fix invalid data space release when truncating block in NOCOW mode

If when truncating a block we fail to reserve data space and then we
proceed anyway because we can do a NOCOW write, if we later get an error
when trying to get the folio from the inode's mapping, we end up releasing
data space that we haven't reserved, screwing up the bytes_may_use counter
from the data space_info, eventually resulting in an underflow when all
other reservations done by other tasks are released, if any, or right away
if there are no other reservations at the moment.

This is because when we get an error when trying to grab the block's folio
we call btrfs_delalloc_release_space(), which releases metadata (which we
have reserved) and data (which we haven't reserved).

Fix this by calling btrfs_delalloc_release_space() only if we did reserve
data space, that is, if we aren't falling back to NOCOW, meaning the local
variable @only_release_metadata has a false value, otherwise release only
metadata by calling btrfs_delalloc_release_metadata().

Fixes: 6d4572a9d71d ("btrfs: allow btrfs_truncate_block() to fallback to nocow for data space reservation")
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: update Kconfig option descriptions
David Sterba [Tue, 6 May 2025 11:04:26 +0000 (13:04 +0200)]
btrfs: update Kconfig option descriptions

Expand what the options do and if they are OK to be enabled.

Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: update list of features built under experimental config
David Sterba [Tue, 6 May 2025 11:04:25 +0000 (13:04 +0200)]
btrfs: update list of features built under experimental config

The list is out of date, the extent shrinker got fixed in 6.13. Add new
entries: the COW fixup warning in 6.15, rund robin policies in 6.14.

Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: send: remove btrfs_debug() calls
David Sterba [Wed, 7 May 2025 16:28:06 +0000 (18:28 +0200)]
btrfs: send: remove btrfs_debug() calls

There are debugging prints for each emitted send command and other
related actions. This does not seem right as the number of commands can
be high and dumping that to the system log will likely hit some rate
limiting.  This should be done by trace points that are more lightweight
and can keep up with high frequency.

Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: use boolean for delalloc argument to btrfs_free_reserved_extent()
Filipe Manana [Wed, 7 May 2025 15:33:45 +0000 (16:33 +0100)]
btrfs: use boolean for delalloc argument to btrfs_free_reserved_extent()

We are using an integer for the 'delalloc' argument but all we need is a
boolean, so switch the type to 'bool' and rename the parameter to
'is_delalloc' to better match the fact that it's a boolean.

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>
4 weeks agobtrfs: use boolean for delalloc argument to btrfs_free_reserved_bytes()
Filipe Manana [Wed, 7 May 2025 15:28:51 +0000 (16:28 +0100)]
btrfs: use boolean for delalloc argument to btrfs_free_reserved_bytes()

We are using an integer for the 'delalloc' argument but all we need is a
boolean, so switch the type to 'bool' and rename the parameter to
'is_delalloc' to better match the fact that it's a boolean.

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>
4 weeks agobtrfs: fold error checks when allocating ordered extent and update comments
Filipe Manana [Wed, 7 May 2025 15:05:06 +0000 (16:05 +0100)]
btrfs: fold error checks when allocating ordered extent and update comments

Instead of having an error check and return on each branch of the if
statement, move the error check to happen after that if branch, reducing
source code and object code sizes.

Before this change:

   $ size fs/btrfs/btrfs.ko
      text    data     bss     dec     hex filename
   1840174  163742   16136 2020052  1ed2d4 fs/btrfs/btrfs.ko

After this change:

   $ size fs/btrfs/btrfs.ko
      text    data     bss     dec     hex filename
   1840138  163742   16136 2020016  1ed2b0 fs/btrfs/btrfs.ko

While at it and moving the comments, update the comments to be more clear
about how qgroup reserved space is released and the intricacies of how
it's managed for COW writes.

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>
4 weeks agobtrfs: check we grabbed inode reference when allocating an ordered extent
Filipe Manana [Tue, 6 May 2025 14:56:45 +0000 (15:56 +0100)]
btrfs: check we grabbed inode reference when allocating an ordered extent

When allocating an ordered extent we call igrab() to get a reference on
the inode and attach it to the ordered extent. For an ordered extent we
always must have an inode reference since we during its life cycle we
need to access the inode for several things like for example:

* Inserting the ordered extent right after allocating it, when calling
  insert_ordered_extent() - we need to lock the inode's ordered_tree_lock;

* In the bio submission path we need to add checksums to the ordered
  extent and we end up at btrfs_add_ordered_sum(), where again we need
  to grab the inode from the ordered extent to lock the inode's
  ordered_tree_lock;

* When finishing an ordered extent, at btrfs_finish_ordered_extent(), we
  need again to access its inode in order to lock the inode's
  ordered_tree_lock;

* Etc etc etc.

Everywhere we deal with an ordered extent we always expect its inode to
be not NULL, the only exception being btrfs_put_ordered_extent() where
we check if it's NULL before calling btrfs_add_delayed_iput(), even though
we have already assumed it's not NULL when calling the tracepoint
trace_btrfs_ordered_extent_put() since the tracepoint dereferences the
inode to extract its number and root without ever checking it's NULL.

The igrab() call can return NULL if the inode is about to be freed or is
being freed (its state has I_FREEING or I_WILL_FREE set), and that's why
there's such check at btrfs_put_ordered_extent(). The igrab() and NULL
check were introduced in commit 5fd02043553b ("Btrfs: finish ordered
extents in their own thread") but even back then we always needed and
assumed igrab() returned a non-NULL pointer, since for example when
removing an ordered extent, at btrfs_remove_ordered_extent(), we assumed
the inode pointer was not NULL in order to access the inode's ordered
extent tree.

In fact whenever we allocate an ordered extent we are holding an inode
reference and the inode is not being freed or going to be freed (which
happens in the final iput), and since we depend on the inode for the
life cycle of the ordered extent, just make ordered extent allocation
to fail in case igrab() returns NULL and trigger a warning, to make it
clear it's not expected. This allows to remove the confusing NULL inode
check at btrfs_put_ordered_extent().

Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: fix qgroup reservation leak on failure to allocate ordered extent
Filipe Manana [Wed, 7 May 2025 12:05:36 +0000 (13:05 +0100)]
btrfs: fix qgroup reservation leak on failure to allocate ordered extent

If we fail to allocate an ordered extent for a COW write we end up leaking
a qgroup data reservation since we called btrfs_qgroup_release_data() but
we didn't call btrfs_qgroup_free_refroot() (which would happen when
running the respective data delayed ref created by ordered extent
completion or when finishing the ordered extent in case an error happened).

So make sure we call btrfs_qgroup_free_refroot() if we fail to allocate an
ordered extent for a COW write.

Fixes: 7dbeaad0af7d ("btrfs: change timing for qgroup reserved space for ordered extents to fix reserved space leak")
CC: stable@vger.kernel.org # 6.1+
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>
4 weeks agobtrfs: scrub: reduce memory usage of struct scrub_sector_verification
Qu Wenruo [Wed, 7 May 2025 04:25:42 +0000 (13:55 +0930)]
btrfs: scrub: reduce memory usage of struct scrub_sector_verification

That structure records needed info for block verification (either data
checksum pointer, or expected tree block generation).

But there is also a boolean to tell if this block belongs to a metadata
or not, as the data checksum pointer and expected tree block generation
is already a union, we need a dedicated bit to tell if this block is a
metadata or not.

However such layout means we're wasting 63 bits for x86_64, which is a
huge memory waste.

Thanks to the recent bitmap aggregation, we can easily move this
single-bit-per-block member to a new sub-bitmap.
And since we already have six 16 bits long bitmaps, adding another
bitmap won't even increase any memory usage for x86_64, as we need two
64 bits long anyway.

This will reduce the following memory usages:

- sizeof(struct scrub_sector_verification)
  From 16 bytes to 8 bytes on x86_64.

- scrub_stripe::sectors
  From 16 * 16 to 16 * 8 bytes.

- Per-device scrub_ctx memory usage
  From 128 * (16 * 16) to 128 * (16 * 8), which saves 16KiB memory.

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: handle aligned EOF truncation correctly for subpage cases
Qu Wenruo [Fri, 25 Apr 2025 22:36:50 +0000 (08:06 +0930)]
btrfs: handle aligned EOF truncation correctly for subpage cases

[BUG]
For the following fsx -e 1 run, the btrfs still fails the run on 64K
page size with 4K fs block size:

  READ BAD DATA: offset = 0x26b3a, size = 0xfafa, fname = /mnt/btrfs/junk
  OFFSET      GOOD    BAD     RANGE
  0x26b3a     0x0000  0x15b4  0x0
  operation# (mod 256) for the bad data may be 21
  [...]
  LOG DUMP (28 total operations):
  1(  1 mod 256): SKIPPED (no operation)
  2(  2 mod 256): SKIPPED (no operation)
  3(  3 mod 256): SKIPPED (no operation)
  4(  4 mod 256): SKIPPED (no operation)
  5(  5 mod 256): WRITE    0x1ea90 thru 0x285e0 (0x9b51 bytes) HOLE
  6(  6 mod 256): ZERO     0x1b1a8 thru 0x20bd4 (0x5a2d bytes)
  7(  7 mod 256): FALLOC   0x22b1a thru 0x272fa (0x47e0 bytes) INTERIOR
  8(  8 mod 256): WRITE    0x741d thru 0x13522 (0xc106 bytes)
  9(  9 mod 256): MAPWRITE 0x73ee thru 0xdeeb (0x6afe bytes)
  10( 10 mod 256): FALLOC   0xb719 thru 0xb994 (0x27b bytes) INTERIOR
  11( 11 mod 256): COPY 0x15ed8 thru 0x18be1 (0x2d0a bytes) to 0x25f6e thru 0x28c77
  12( 12 mod 256): ZERO     0x1615e thru 0x1770e (0x15b1 bytes)
  13( 13 mod 256): SKIPPED (no operation)
  14( 14 mod 256): DEDUPE 0x20000 thru 0x27fff (0x8000 bytes) to 0x1000 thru 0x8fff
  15( 15 mod 256): SKIPPED (no operation)
  16( 16 mod 256): CLONE 0xa000 thru 0xffff (0x6000 bytes) to 0x36000 thru 0x3bfff
  17( 17 mod 256): ZERO     0x14adc thru 0x1b78a (0x6caf bytes)
  18( 18 mod 256): TRUNCATE DOWN from 0x3c000 to 0x1e2e3 ******WWWW
  19( 19 mod 256): CLONE 0x4000 thru 0x11fff (0xe000 bytes) to 0x16000 thru 0x23fff
  20( 20 mod 256): FALLOC   0x311e1 thru 0x3681b (0x563a bytes) PAST_EOF
  21( 21 mod 256): FALLOC   0x351c5 thru 0x40000 (0xae3b bytes) EXTENDING
  22( 22 mod 256): WRITE    0x920 thru 0x7e51 (0x7532 bytes)
  23( 23 mod 256): COPY 0x2b58 thru 0xc508 (0x99b1 bytes) to 0x117b1 thru 0x1b161
  24( 24 mod 256): TRUNCATE DOWN from 0x40000 to 0x3c9a5
  25( 25 mod 256): SKIPPED (no operation)
  26( 26 mod 256): MAPWRITE 0x25020 thru 0x26b06 (0x1ae7 bytes)
  27( 27 mod 256): SKIPPED (no operation)
  28( 28 mod 256): READ     0x26b3a thru 0x36633 (0xfafa bytes) ***RRRR***

[CAUSE]
The involved operations are:

  fallocating to largest ever: 0x40000
  21 pollute_eof 0x24000 thru 0x2ffff (0xc000 bytes)
  21 falloc from 0x351c5 to 0x40000 (0xae3b bytes)
  28 read 0x26b3a thru 0x36633 (0xfafa bytes)

At operation #21 a pollute_eof is done, by memory mapped write into
range [0x24000, 0x2ffff).
At this stage, the inode size is 0x24000, which is block aligned.

Then fallocate happens, and since it's expanding the inode, it will call
btrfs_truncate_block() to truncate any unaligned range.

But since the inode size is already block aligned,
btrfs_truncate_block() does nothing and exits.

However remember the folio at 0x20000 has some range polluted already,
although it will not be written back to disk, it still affects the
page cache, resulting the later operation #28 to read out the polluted
value.

[FIX]
Instead of early exit from btrfs_truncate_block() if the range is
already block aligned, do extra filio zeroing if the fs block size is
smaller than the page size and we're truncating beyond EOF.

This is to address exactly the above case where memory mapped write can
still leave some garbage beyond EOF.

Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: handle unaligned EOF truncation correctly for subpage cases
Qu Wenruo [Fri, 25 Apr 2025 22:36:49 +0000 (08:06 +0930)]
btrfs: handle unaligned EOF truncation correctly for subpage cases

[BUG]
The following fsx sequence will fail on btrfs with 64K page size and 4K
fs block size:

  #fsx -d -e 1 -N 4 $mnt/junk -S 36386
  READ BAD DATA: offset = 0xe9ba, size = 0x6dd5, fname = /mnt/btrfs/junk
  OFFSET      GOOD    BAD     RANGE
  0xe9ba      0x0000  0x03ac  0x0
  operation# (mod 256) for the bad data may be 3
  ...
  LOG DUMP (4 total operations):
  1(  1 mod 256): WRITE    0x6c62 thru 0x1147d (0xa81c bytes) HOLE ***WWWW
  2(  2 mod 256): TRUNCATE DOWN from 0x1147e to 0x5448 ******WWWW
  3(  3 mod 256): ZERO     0x1c7aa thru 0x28fe2 (0xc839 bytes)
  4(  4 mod 256): MAPREAD  0xe9ba thru 0x1578e (0x6dd5 bytes) ***RRRR***

[CAUSE]
Only 2 operations are really involved in this case:

  3 pollute_eof 0x5448 thru 0xffff (0xabb8 bytes)
  3 zero from 0x1c7aa to 0x28fe3, (0xc839 bytes)
  4 mapread 0xe9ba thru 0x1578e (0x6dd5 bytes)

At operation 3, fsx pollutes beyond EOF, that is done by mmap()
and write into that mmap() range beyond EOF.

Such write will fill the range beyond EOF, but it will never reach disk
as ranges beyond EOF will not be marked dirty nor uptodate.

Then we zero_range for [0x1c7aa, 0x28fe3], and since the range is beyond
our isize (which was 0x5448), we should zero out any range beyond
EOF (0x5448).

During btrfs_zero_range(), we call btrfs_truncate_block() to dirty the
unaligned head block.
But that function only really zeroes out the block at [0x5000, 0x5fff], it
doesn't bother any range other that that block, since those ranges will
not be marked dirty nor written back.

So the range [0x6000, 0xffff] is still polluted, and later mapread()
will return the poisoned value.

[FIX]
Enhance btrfs_truncate_block() by:

- Pass a @start/@end pair to indicate the full truncation range
  This is to handle the following truncation case:

    Page size is 64K, fs block size is 4K, truncate range is
    [6K, 60K]

    0                      32K                    64K
    |   |///////////////////////////////////|     |
        6K                                  60K

    The range is not aligned for its head block, so we need to call
    btrfs_truncate_block() with @from = 6K, @front = 0, @len = 0.

    But with that information we only know to zero the range [6K, 8K),
    if we zero out the range [6K, 64K), the last block will also be
    zeroed, causing data loss.

  So here we need the full range we're truncating, so that we can avoid
  over-truncation.

- Rename @from to @offset
  As now the parameter is only utilized to locate a block, it's not
  really carrying the old @from meaning well.

- Remove @front parameter
  With the full truncate range passed in, we can determine if the
  @offset is at the head or tail block.

- Skip truncation if @offset is not in the head nor tail blocks
  The call site in hole punch unconditionally call
  btrfs_truncate_block() without even checking the range is aligned or
  not.
  If the @offset is neither in the head nor in tail block, it means we can
  safely ignore it.

- Skip truncate if the range inside the target block is already aligned

- Make btrfs_truncate_block() zero all blocks beyond EOF
  Since we have the original range, we know exactly if we're doing
  truncation beyond EOF (the @end will be (u64)-1).

  If we're doing truncation beyond EOF, then enlarge the truncation
  range to the folio end, to address the possibly polluted ranges.

  Otherwise still keep the zero range inside the block, as we can have
  large data folios soon, always truncating every blocks inside the same
  folio can be costly for large folios.

Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: fix broken drop_caches on extent buffer folios
Boris Burkov [Mon, 17 Mar 2025 23:47:38 +0000 (16:47 -0700)]
btrfs: fix broken drop_caches on extent buffer folios

The (correct) commit e41c81d0d30e ("mm/truncate: Replace page_mapped()
call in invalidate_inode_page()") replaced the page_mapped(page) check
with a refcount check. However, this refcount check does not work as
expected with drop_caches for btrfs's metadata pages.

Btrfs has a per-sb metadata inode with cached pages, and when not in
active use by btrfs, they have a refcount of 3. One from the initial
call to alloc_pages(), one (nr_pages == 1) from filemap_add_folio(), and
one from folio_attach_private(). We would expect such pages to get dropped
by drop_caches. However, drop_caches calls into mapping_evict_folio() via
mapping_try_invalidate() which gets a reference on the folio with
find_lock_entries(). As a result, these pages have a refcount of 4, and
fail this check.

For what it's worth, such pages do get reclaimed under memory pressure,
so I would say that while this behavior is surprising, it is not really
dangerously broken.

When I asked the mm folks about the expected refcount in this case, I
was told that the correct thing to do is to donate the refcount from the
original allocation to the page cache after inserting it.

Therefore, attempt to fix this by adding a put_folio() to the critical
spot in alloc_extent_buffer() where we are sure that we have really
allocated and attached new pages. We must also adjust
folio_detach_private() to properly handle being the last reference to the
folio and not do a use-after-free after folio_detach_private().

extent_buffers allocated by clone_extent_buffer() and
alloc_dummy_extent_buffer() are unmapped, so this transfer of ownership
from allocation to insertion in the mapping does not apply to them.
However, we can still folio_put() them safely once they are finished
being allocated and have called folio_attach_private().

Finally, removing the generic put_folio() for the allocation from
btrfs_detach_extent_buffer_folios() means we need to be careful to do
the appropriate put_folio() in allocation failure paths in
alloc_extent_buffer(), clone_extent_buffer() and
alloc_dummy_extent_buffer().

Link: https://lore.kernel.org/linux-mm/ZrwhTXKzgDnCK76Z@casper.infradead.org/
Tested-by: Klara Modin <klarasmodin@gmail.com>
Reviewed-by: Daniel Vacek <neelx@suse.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Boris Burkov <boris@bur.io>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: use verbose assert at peek_discard_list()
Filipe Manana [Mon, 5 May 2025 15:36:21 +0000 (16:36 +0100)]
btrfs: use verbose assert at peek_discard_list()

We now have a verbose variant of ASSERT() so that we can print the value
of the block group's discard_index. So use it for better problem analysis
in case the assertion is triggered.

Reviewed-by: Boris Burkov <boris@bur.io>
Reviewed-by: Daniel Vacek <neelx@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>
4 weeks agobtrfs: scrub: aggregate small bitmaps into a larger one
Qu Wenruo [Mon, 5 May 2025 10:03:47 +0000 (19:33 +0930)]
btrfs: scrub: aggregate small bitmaps into a larger one

Currently we have several small bitmaps inside scrub_stripe:

- extent_sector_bitmap
- error_bitmap
- io_error_bitmap
- csum_error_bitmap
- meta_error_bitmap
- meta_gen_error_bitmap

All those bitmaps are at most 16 bits long, but unsigned long is
either 32 or 64 (more common) bits.

This means we're wasting 1/2 or 3/4 space for each bitmap.

And we can have 128 scrub_stripe for each device, such wasted space adds up
quickly.

Instead of using a single unsigned long for each bitmap, aggregate them
into a larger bitmap, just like what we're doing for subpage support.

This reduces 24 bytes from each scrub_stripe structure on x86_64
systems.

This will need a lot of macros converting direct bitmap/bit operations into
our scrub_stripe specific helpers, but all those helpers are very small
and can be inlined.

So overall the overhead shouldn't be that huge, and we save quite some
memory space.

Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: scrub: fix a wrong error type when metadata bytenr mismatches
Qu Wenruo [Mon, 5 May 2025 09:26:18 +0000 (18:56 +0930)]
btrfs: scrub: fix a wrong error type when metadata bytenr mismatches

When the bytenr doesn't match for a metadata tree block, we will report
it as an csum error, which is incorrect and should be reported as a
metadata error instead.

Fixes: a3ddbaebc7c9 ("btrfs: scrub: introduce a helper to verify one metadata block")
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: defrag: use list_last_entry() at defrag_collect_targets()
Filipe Manana [Fri, 2 May 2025 10:23:02 +0000 (11:23 +0100)]
btrfs: defrag: use list_last_entry() at defrag_collect_targets()

Instead of using list_entry() against the list's prev entry, use
list_last_entry(), which removes the need to know the last member is
accessed through the prev list pointer and the naming makes it easier
to reason about what we are doing.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: simplify csum list release at btrfs_put_ordered_extent()
Filipe Manana [Thu, 1 May 2025 12:21:34 +0000 (13:21 +0100)]
btrfs: simplify csum list release at btrfs_put_ordered_extent()

Instead of extracting each element by grabbing the list's first member in
a local list_head variable, then extracting the csum with list_entry() and
iterating with a while loop checking for list emptyness, use the iteration
helper list_for_each_entry_safe(). This also removes the need to delete
elements from the list with list_del() since the ordered extent is freed
immediately after.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: simplify extracting delayed node at btrfs_first_prepared_delayed_node()
Filipe Manana [Thu, 1 May 2025 12:11:40 +0000 (13:11 +0100)]
btrfs: simplify extracting delayed node at btrfs_first_prepared_delayed_node()

Instead of grabbing the next pointer from the list and then doing a
list_entry() call, we can simply use list_first_entry(), removing the need
for list_head variable.

Also there's no need to check if the list is empty before attempting to
extract the first element, we can use list_first_entry_or_null(), removing
the need for a special if statement and the 'out' label.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: simplify extracting delayed node at btrfs_first_delayed_node()
Filipe Manana [Thu, 1 May 2025 12:05:10 +0000 (13:05 +0100)]
btrfs: simplify extracting delayed node at btrfs_first_delayed_node()

Instead of grabbing the next pointer from the list and then doing a
list_entry() call, we can simply use list_first_entry(), removing the need
for list_head variable.

Also there's no need to check if the list is empty before attempting to
extract the first element, we can use list_first_entry_or_null(), removing
the need for a special if statement and the 'out' label.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: raid56: use list_last_entry() at cache_rbio()
Filipe Manana [Thu, 1 May 2025 11:54:03 +0000 (12:54 +0100)]
btrfs: raid56: use list_last_entry() at cache_rbio()

Instead of using list_entry() against the list's prev entry, use
list_last_entry(), which removes the need to know the last member is
accessed through the prev list pointer and the naming makes it easier
to reason about what we are doing.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: simplify cow only root list extraction during transaction commit
Filipe Manana [Thu, 1 May 2025 11:47:35 +0000 (12:47 +0100)]
btrfs: simplify cow only root list extraction during transaction commit

There's no need to keep a local variable to extract the first member of
the list and then do a list_entry() call, we can use list_first_entry()
instead, removing the need for the temporary variable and extracting the
first element in a single step.

Also, there's no need to do a list_del_init() followed by list_add_tail(),
instead we can use list_move_tail(). We are in transaction commit critical
section where we don't need to worry about concurrency and that's why we
don't take any locks and can use list_move_tail() (we do assert early at
commit_cowonly_roots() that we are in the critical section, that the
transaction's state is TRANS_STATE_COMMIT_DOING).

Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: simplify getting and extracting previous transaction at clean_pinned_extents()
Filipe Manana [Thu, 1 May 2025 11:23:17 +0000 (12:23 +0100)]
btrfs: simplify getting and extracting previous transaction at clean_pinned_extents()

Instead of detecting if there is a previous transaction by comparing the
current transaction's list prev member to the head of the transaction
list (fs_info->trans_list), use the list_is_first() helper which contains
that logic and the naming makes sense since a new transaction is always
added to the end of the list fs_info->trans_list with list_add_tail().

We are also extracting the previous transaction with list_last_entry()
against the transaction, which is correct but confusing because that
function is usually meant to be used against a pointer to the start of a
list and not a member of a list. It is easier to reason by either calling
list_first_entry() against the list fs_info->trans_list, since we can
never have more than two transactions in the list, or by calling
list_prev_entry() against the transaction. So change that to use the later
method.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: simplify getting and extracting previous transaction during commit
Filipe Manana [Thu, 1 May 2025 11:14:41 +0000 (12:14 +0100)]
btrfs: simplify getting and extracting previous transaction during commit

Instead of detecting if there is a previous transaction by comparing the
current transaction's list prev member to the head of the transaction
list (fs_info->trans_list), use the list_is_first() helper which contains
that logic and the naming makes sense since a new transaction is always
added to the end of the list fs_info->trans_list with list_add_tail().

And instead of extracting the previous transaction with the more generic
list_entry() helper against the current transaction's list prev member,
use the more specific list_prev_entry() helper, which makes it clear what
we are doing and is shorter.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: move transaction aborts to the error site in add_to_free_space_tree()
David Sterba [Wed, 30 Apr 2025 16:45:21 +0000 (18:45 +0200)]
btrfs: move transaction aborts to the error site in add_to_free_space_tree()

Transaction aborts should be done next to the place the error happens,
which was not done in add_to_free_space_tree().

Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: move transaction aborts to the error site in remove_from_free_space_tree()
David Sterba [Wed, 30 Apr 2025 16:45:20 +0000 (18:45 +0200)]
btrfs: move transaction aborts to the error site in remove_from_free_space_tree()

Transaction aborts should be done next to the place the error happens,
which was not done in remove_from_free_space_tree().

Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: move transaction aborts to the error site in convert_free_space_to_extents()
David Sterba [Wed, 30 Apr 2025 16:45:19 +0000 (18:45 +0200)]
btrfs: move transaction aborts to the error site in convert_free_space_to_extents()

Transaction aborts should be done next to the place the error happens,
which was not done in convert_free_space_to_extents().  The DEBUG_WARN()
is removed because we get the abort message.

Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: move transaction aborts to the error site in convert_free_space_to_bitmaps()
David Sterba [Wed, 30 Apr 2025 16:45:18 +0000 (18:45 +0200)]
btrfs: move transaction aborts to the error site in convert_free_space_to_bitmaps()

Transaction aborts should be done next to the place the error happens,
which was not done in convert_free_space_to_bitmaps(). The DEBUG_WARN()
is removed because we get the abort message.

Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: scrub: move error reporting members to stack
Qu Wenruo [Fri, 2 May 2025 00:00:35 +0000 (09:30 +0930)]
btrfs: scrub: move error reporting members to stack

Currently the following members of scrub_stripe are only utilized for
error reporting:

- init_error_bitmap
- init_nr_io_errors
- init_nr_csum_errors
- init_nr_meta_errors
- init_nr_meta_gen_errors

There is no need to put all those members into scrub_stripe, which take
24 bytes for each stripe, and we have 128 stripes for each device.

Instead introduce a structure, scrub_error_records, and move all above
members into that structure.

And allocate such structure from stack inside
scrub_stripe_read_repair_worker().
Since that function is called from a workqueue context, we have more
than enough stack space for just 24 bytes.

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: scrub: update device stats when an error is detected
Qu Wenruo [Wed, 30 Apr 2025 23:07:54 +0000 (08:37 +0930)]
btrfs: scrub: update device stats when an error is detected

[BUG]
Since the migration to the new scrub_stripe interface, scrub no longer
updates the device stats when hitting an error, no matter if it's a read
or checksum mismatch error. E.g:

  BTRFS info (device dm-2): scrub: started on devid 1
  BTRFS error (device dm-2): unable to fixup (regular) error at logical 13631488 on dev /dev/mapper/test-scratch1 physical 13631488
  BTRFS warning (device dm-2): checksum error at logical 13631488 on dev /dev/mapper/test-scratch1, physical 13631488, root 5, inode 257, offset 0, length 4096, links 1 (path: file)
  BTRFS error (device dm-2): unable to fixup (regular) error at logical 13631488 on dev /dev/mapper/test-scratch1 physical 13631488
  BTRFS warning (device dm-2): checksum error at logical 13631488 on dev /dev/mapper/test-scratch1, physical 13631488, root 5, inode 257, offset 0, length 4096, links 1 (path: file)
  BTRFS info (device dm-2): scrub: finished on devid 1 with status: 0

Note there is no line showing the device stats error update.

[CAUSE]
In the migration to the new scrub_stripe interface, we no longer call
btrfs_dev_stat_inc_and_print().

[FIX]
- Introduce a new bitmap for metadata generation errors
  * A new bitmap
    @meta_gen_error_bitmap is introduced to record which blocks have
    metadata generation mismatch errors.

  * A new counter for that bitmap
    @init_nr_meta_gen_errors, is also introduced to store the number of
    generation mismatch errors that are found during the initial read.

    This is for the error reporting at scrub_stripe_report_errors().

  * New dedicated error message for unrepaired generation mismatches

  * Update @meta_gen_error_bitmap if a transid mismatch is hit

- Add btrfs_dev_stat_inc_and_print() calls to the following call sites
  * scrub_stripe_report_errors()
  * scrub_write_endio()
    This is only for the write errors.

This means there is a minor behavior change:

- The timing of device stats error message
  Since we concentrate the error messages at
  scrub_stripe_report_errors(), the device stats error messages will all
  show up in one go, after the detailed scrub error messages:

   BTRFS error (device dm-2): unable to fixup (regular) error at logical 13631488 on dev /dev/mapper/test-scratch1 physical 13631488
   BTRFS warning (device dm-2): checksum error at logical 13631488 on dev /dev/mapper/test-scratch1, physical 13631488, root 5, inode 257, offset 0, length 4096, links 1 (path: file)
   BTRFS error (device dm-2): unable to fixup (regular) error at logical 13631488 on dev /dev/mapper/test-scratch1 physical 13631488
   BTRFS warning (device dm-2): checksum error at logical 13631488 on dev /dev/mapper/test-scratch1, physical 13631488, root 5, inode 257, offset 0, length 4096, links 1 (path: file)
   BTRFS error (device dm-2): bdev /dev/mapper/test-scratch1 errs: wr 0, rd 0, flush 0, corrupt 1, gen 0
   BTRFS error (device dm-2): bdev /dev/mapper/test-scratch1 errs: wr 0, rd 0, flush 0, corrupt 2, gen 0

Fixes: e02ee89baa66 ("btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe infrastructure")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: add support for reclaiming from sub-space space_info
Naohiro Aota [Wed, 23 Apr 2025 02:43:53 +0000 (11:43 +0900)]
btrfs: add support for reclaiming from sub-space space_info

Modify btrfs_async_{data,metadata}_reclaim() to run the reclaim process
on the sub-spaces as well.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: add block reserve for treelog
Naohiro Aota [Wed, 23 Apr 2025 02:43:52 +0000 (11:43 +0900)]
btrfs: add block reserve for treelog

We need to add a dedicated block_rsv for tree-log, because the block_rsv
serves for a tree node allocation in btrfs_alloc_tree_block(). Currently,
tree-log tree uses fs_info->empty_block_rsv, which is shared across trees
and points to the normal metadata space_info. Instead, we add a dedicated
block_rsv and that block_rsv can use the dedicated sub-space_info.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: use proper data space_info for zoned mode
Naohiro Aota [Wed, 23 Apr 2025 02:43:51 +0000 (11:43 +0900)]
btrfs: use proper data space_info for zoned mode

Now that, we have data sub-space for the zoned mode. Tweak some space_info
functions to use proper space_info for a file.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: tweak extent/chunk allocation for space_info sub-space
Naohiro Aota [Wed, 23 Apr 2025 02:43:50 +0000 (11:43 +0900)]
btrfs: tweak extent/chunk allocation for space_info sub-space

Make the extent allocator and the chunk allocator aware of the sub-space.
It now uses BTRFS_SUB_GROUP_DATA_RELOC sub-space for data relocation block
group, and uses BTRFS_SUB_GROUP_TREELOG for metadata tree-log block group.

And, it needs to check the space_info is the right one when a block group
candidate is given. Also, new block group should now belong to the
specified one.

Now that, block_group->space_info is always set before
btrfs_add_bg_to_space_info(), we no longer need to "find" the space_info.
So, rename the variable name to address that as well.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: introduce tree-log sub-space_info
Naohiro Aota [Wed, 23 Apr 2025 02:43:49 +0000 (11:43 +0900)]
btrfs: introduce tree-log sub-space_info

Introduce the tree-log sub-space_info, which is sub-space of
metadata space_info and dedicated for tree-log node allocation.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: introduce btrfs_space_info sub-group
Naohiro Aota [Wed, 23 Apr 2025 02:43:48 +0000 (11:43 +0900)]
btrfs: introduce btrfs_space_info sub-group

Current code assumes we have only one space_info for each block group type
(DATA, METADATA, and SYSTEM). We sometime need multiple space infos to
manage special block groups.

One example is handling the data relocation block group for the zoned mode.
That block group is dedicated for writing relocated data and we cannot
allocate any regular extent from that block group, which is implemented in
the zoned extent allocator. This block group still belongs to the normal
data space_info. So, when all the normal data block groups are full and
there is some free space in the dedicated block group, the space_info
looks to have some free space, while it cannot allocate normal extent
anymore. That results in a strange ENOSPC error. We need to have a
space_info for the relocation data block group to represent the situation
properly.

Adds a basic infrastructure for having a "sub-group" of a space_info:
creation and removing. A sub-group space_info belongs to one of the
primary space_infos and has the same flags as its parent.

This commit first introduces the relocation data sub-space_info, and the
next commit will introduce tree-log sub-space_info. In the future, it could
be useful to implement tiered storage for btrfs e.g. by implementing a
sub-group space_info for block groups resides on a fast storage.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: add space_info parameter for block group creation
Naohiro Aota [Wed, 23 Apr 2025 02:43:47 +0000 (11:43 +0900)]
btrfs: add space_info parameter for block group creation

Add struct btrfs_space_info parameter to btrfs_make_block_group(), its
related functions and related struct. Passed space_info will have a new
block group.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: add space_info argument to btrfs_chunk_alloc()
Naohiro Aota [Wed, 23 Apr 2025 02:43:46 +0000 (11:43 +0900)]
btrfs: add space_info argument to btrfs_chunk_alloc()

Take a btrfs_space_info argument in btrfs_chunk_alloc(). New block group
will belong to that space_info.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: factor out check_removing_space_info() from btrfs_free_block_groups()
Naohiro Aota [Wed, 23 Apr 2025 02:43:45 +0000 (11:43 +0900)]
btrfs: factor out check_removing_space_info() from btrfs_free_block_groups()

Factor out check_removing_space_info() from btrfs_free_block_groups(). It
sanity checks a to-be-removed space_info. There is no functional change.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: factor out do_async_reclaim_{data,metadata}_space()
Naohiro Aota [Wed, 23 Apr 2025 02:43:44 +0000 (11:43 +0900)]
btrfs: factor out do_async_reclaim_{data,metadata}_space()

Factor out the main part of btrfs_async_reclaim_data_space() to
do_async_reclaim_data_space(), so it can take data space_info parameter
it is working on. Do the same for metadata. There is no functional change.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: factor out init_space_info() from create_space_info()
Naohiro Aota [Wed, 23 Apr 2025 02:43:43 +0000 (11:43 +0900)]
btrfs: factor out init_space_info() from create_space_info()

Factor out initialization of the space_info struct, which is used in a
later patch. There is no functional change.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: pass struct btrfs_inode to btrfs_free_reserved_data_space_noquota()
Naohiro Aota [Wed, 23 Apr 2025 02:43:42 +0000 (11:43 +0900)]
btrfs: pass struct btrfs_inode to btrfs_free_reserved_data_space_noquota()

As well as the last patch, pass struct btrfs_inode to the function and
let it distinguish which data space it is working on in a later patch.
There is no functional change with this commit.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: pass btrfs_space_info to btrfs_reserve_data_bytes()
Naohiro Aota [Wed, 23 Apr 2025 02:43:41 +0000 (11:43 +0900)]
btrfs: pass btrfs_space_info to btrfs_reserve_data_bytes()

Pass struct btrfs_space_info to btrfs_reserve_data_bytes() to allow
reserving the data from multiple data space_info candidates.

This is a preparation for the following commits and there is no functional
change.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: make extent unpinning more efficient when committing transaction
Filipe Manana [Tue, 22 Apr 2025 15:21:25 +0000 (16:21 +0100)]
btrfs: make extent unpinning more efficient when committing transaction

At btrfs_finish_extent_commit() we have this loop that keeps finding an
extent range to unpin in the transaction's pinned_extents io tree, caches
the extent state and then passes that cached extent state to
btrfs_clear_extent_dirty(), which will free that extent state since we
clear the only bit it can have set. So on each loop iteration we do a
full io tree search and the cached state is used only to avoid having
a tree search done by btrfs_clear_extent_dirty().

During the lifetime of a transaction we can pin many thousands of extents,
resulting in a large and deep rb tree that backs the io tree. For example,
for the following fs_mark run on a 12 cores boxes:

  $ cat test.sh
  #!/bin/bash

  DEV=/dev/nullb0
  MNT=/mnt/nullb0
  FILES=100000
  THREADS=$(nproc --all)

  echo "performance" | \
      tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor

  mkfs.btrfs -f $DEV
  mount $DEV $MNT

  OPTS="-S 0 -L 8 -n $FILES -s 0 -t $THREADS -k"
  for ((i = 1; i <= $THREADS; i++)); do
      OPTS="$OPTS -d $MNT/d$i"
  done

  fs_mark $OPTS

  umount $MNT

an histogram for the number of ranges (elements) in the pinned extents
io tree of a transaction was the following:

  Count: 76
  Range: 5440.000 - 51088.000; Mean: 27354.368; Median: 28312.000; Stddev: 9800.767
  Percentiles:  90th: 40486.000; 95th: 43322.000; 99th: 51088.000
   5440.000 -  6805.809:     1 ###
   6805.809 - 10652.034:     1 ###
  10652.034 - 13326.178:     3 ########
  13326.178 - 16671.590:     8 ######################
  16671.590 - 20856.773:     7 ####################
  20856.773 - 26092.528:    13 ####################################
  26092.528 - 32642.571:    19 #####################################################
  32642.571 - 40836.818:    17 ###############################################
  40836.818 - 51088.000:     7 ####################

We can improve on this by grabbing the next state before calling
btrfs_clear_extent_dirty(), avoiding a full tree search on the next
iteration which always has an O(log n) complexity while grabbing the next
element (rb_next() rbtree operation) is in the worst case O(log n) too,
but very often much less than that, making it more efficient.

Here follow histograms for the execution times, in nanoseconds, of
btrfs_finish_extent_commit() before and after applying this patch and all
the other patches in the same patchset.

Before patchset:

  Count: 32
  Range: 3925691.000 - 269990635.000; Mean: 133814526.906; Median: 122758052.000; Stddev: 65776550.375
  Percentiles:  90th: 228672087.000; 95th: 265187000.000; 99th: 269990635.000
    3925691.000 -   5993208.660:     1 ####
    5993208.660 -  75878537.656:     4 ##################
   75878537.656 - 115840974.514:    12 #####################################################
  115840974.514 - 176850157.761:     6 ###########################
  176850157.761 - 269990635.000:     9 ########################################

After patchset:

  Count: 32
  Range: 1849393.000 - 231491064.000; Mean: 126978584.625; Median: 123732897.000; Stddev: 58007821.806
  Percentiles:  90th: 203055491.000; 95th: 219952699.000; 99th: 231491064.000
    1849393.000 -   2997642.092:     1 ####
    2997642.092 -  88111637.071:     9 #####################################
   88111637.071 - 142818264.414:     9 #####################################
  142818264.414 - 231491064.000:    13 #####################################################

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: remove variable to track trimmed bytes at btrfs_finish_extent_commit()
Filipe Manana [Tue, 22 Apr 2025 14:19:12 +0000 (15:19 +0100)]
btrfs: remove variable to track trimmed bytes at btrfs_finish_extent_commit()

We don't need to keep track of discarded (trimmed) bytes at
btrfs_finish_extent_commit() but we are declaring a local variable for
that and passing a reference to the btrfs_discard_extent() calls when we
are processing delete block groups. So instead pass NULL to
btrfs_discard_extent() and remove that variable.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: don't BUG_ON() when unpinning extents during transaction commit
Filipe Manana [Tue, 22 Apr 2025 14:04:51 +0000 (15:04 +0100)]
btrfs: don't BUG_ON() when unpinning extents during transaction commit

In the final phase of a transaction commit, when unpinning extents at
btrfs_finish_extent_commit(), there's no need to BUG_ON() if we fail to
unpin an extent range. All that can happen is that we fail to return the
extent range to the in-memory free space cache, meaning no future space
allocations can reuse that extent range while the fs is mounted.

So instead return the error to the caller and make it abort the
transaction, so that the error is noticed and prevent misteriously leaking
space. We keep track of the first error we get while unpinning an extent
range and keep trying to unpin all the following extent ranges, so that
we attempt to do all discards. The transaction abort will deal with all
resource cleanups.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: remove unnecessary NULL checks before freeing extent state
Filipe Manana [Wed, 23 Apr 2025 09:41:31 +0000 (10:41 +0100)]
btrfs: remove unnecessary NULL checks before freeing extent state

When manipulating extent bits for an io tree range, we have this pattern:

   if (prealloc)
          btrfs_free_extent_state(prealloc);

but this is not needed nowadays since btrfs_free_extent_state() ignores
a NULL pointer argument, following the common pattern of kernel and btrfs
freeing functions, as well as libc and other user space libraries.
So remove the NULL checks, reducing source code and object size.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: avoid re-searching tree when setting bits in an extent range
Filipe Manana [Thu, 17 Apr 2025 12:03:42 +0000 (13:03 +0100)]
btrfs: avoid re-searching tree when setting bits in an extent range

When setting bits for an extent range (set_extent_bit()), if the current
extent state record starts after the target range, we always do a jump to
the 'search_again' label, which will cause us to do a full tree search for
the next state if the current state ends before the target range. Unless
we need to reschedule, we can just grab the next state and process it,
avoiding a full tree search, even if that next state is not contiguous, as
we'll allocate and insert a new prealloc state if needed.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: avoid repeated extent state processing when setting extent bits
Filipe Manana [Thu, 17 Apr 2025 15:22:50 +0000 (16:22 +0100)]
btrfs: avoid repeated extent state processing when setting extent bits

When setting bits for an extent range, if we find an extent state with
its start offset greater than current start offset, we insert a new extent
state to cover the gap, with its end offset computed and stored in the
@this_end local variable, and after the insertion we update the current
start offset to @this_end + 1. However if the insert_state() call resulted
in an extent state merge then the end offset of the merged extent may be
greater than @this_end and if that's the case, since we jump to the
'search_again' label, we'll do a full tree search that will leave us in
the same extent state - this is harmless but wastes time by doing a
pointless tree search and extent state processing.

So improve on this by updating the current start offset to the end offset
of the inserted state plus 1. This also removes the use of the @this_end
variable and directly set the value in the prealloc extent state to avoid
any confusion and misuse in the future.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: simplify last record detection at set_extent_bit()
Filipe Manana [Wed, 16 Apr 2025 16:05:11 +0000 (17:05 +0100)]
btrfs: simplify last record detection at set_extent_bit()

There's no need to compare the current extent state's end offset to
(u64)-1 to check if we have the last possible record and to check as
as well if after updating the start offset to the end offset of the
current record plus one we are still inside the target range.

Instead we can simplify and exit if the current extent state ends at or
after the target range and then remove the check for the (u64)-1 as well
as the check to see if the updated start offset (to last_end + 1) is still
inside the target range. Besides the simplification, this also avoids
seaching for the next extent state record (through next_state()) when the
current extent state record ends at the same offset as our target range,
which is pointless and only wastes times iterating through the rb tree.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: exit after state split error at set_extent_bit()
Filipe Manana [Wed, 16 Apr 2025 15:00:28 +0000 (16:00 +0100)]
btrfs: exit after state split error at set_extent_bit()

If split_state() returned an error we call extent_io_tree_panic() which
will trigger a BUG() call. However if CONFIG_BUG is disabled, which is an
uncommon and exotic scenario, then we fallthrough and hit a use after free
when calling set_state_bits() since the extent state record which the
local variable 'prealloc' points to was freed by split_state().

So jump to the label 'out' after calling extent_io_tree_panic() and set
the 'prealloc' pointer to NULL since split_state() has already freed it
when it hit an error.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: exit after state insertion failure at set_extent_bit()
Filipe Manana [Wed, 16 Apr 2025 14:56:53 +0000 (15:56 +0100)]
btrfs: exit after state insertion failure at set_extent_bit()

If insert_state() state failed it returns an error pointer and we call
extent_io_tree_panic() which will trigger a BUG() call. However if
CONFIG_BUG is disabled, which is an uncommon and exotic scenario, then
we fallthrough and call cache_state() which will dereference the error
pointer, resulting in an invalid memory access.

So jump to the 'out' label after calling extent_io_tree_panic(), it also
makes the code more clear besides dealing with the exotic scenario where
CONFIG_BUG is disabled.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: simplify last record detection at btrfs_convert_extent_bit()
Filipe Manana [Wed, 16 Apr 2025 15:18:10 +0000 (16:18 +0100)]
btrfs: simplify last record detection at btrfs_convert_extent_bit()

There's no need to compare the current extent state's end offset to
(u64)-1 to check if we have the last possible record and to check as
as well if after updating the start offset to the end offset of the
current record plus one we are still inside the target range.

Instead we can simplify and exit if the current extent state ends at or
after the target range and then remove the check for the (u64)-1 as well
as the check to see if the updated start offset (to last_end + 1) is still
inside the target range.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: avoid re-searching tree when converting bits in an extent range
Filipe Manana [Wed, 16 Apr 2025 14:43:25 +0000 (15:43 +0100)]
btrfs: avoid re-searching tree when converting bits in an extent range

When converting bits for an extent range (btrfs_convert_extent_bit()), if
the current extent state record starts after the target range, we always
do a jump to the 'search_again' label, which will cause us to do a full
tree search for the next state if the current state ends before the target
range. Unless we need to reschedule, we can just grab the next state and
process it, avoiding a full tree search, even if that next state is not
contiguous, as we'll allocate and insert a new prealloc state if needed.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: avoid repeated extent state processing when converting extent bits
Filipe Manana [Thu, 17 Apr 2025 15:44:34 +0000 (16:44 +0100)]
btrfs: avoid repeated extent state processing when converting extent bits

When converting bits for an extent range, if we find an extent state with
its start offset greater than current start offset, we insert a new extent
state to cover the gap, with its end offset computed and stored in the
@this_end local variable, and after the insertion we update the current
start offset to @this_end + 1. However if the insert_state() call resulted
in an extent state merge then the end offset of the merged extent may be
greater than @this_end and if that's the case, since we jump to the
'search_again' label, we'll do a full tree search that will leave us in
the same extent state - this is harmless but wastes time by doing a
pointless tree search and extent state processing.

So improve on this by updating the current start offset to the end offset
of the inserted state plus 1. This also removes the use of the @this_end
variable and directly set the value in the prealloc extent state to avoid
any confusion and misuse in the future.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: avoid unnecessary next node searches when clearing bits from extent range
Filipe Manana [Wed, 16 Apr 2025 14:24:48 +0000 (15:24 +0100)]
btrfs: avoid unnecessary next node searches when clearing bits from extent range

When clearing bits for a range in an io tree, at clear_state_bit(), we
always go search for the next node (through next_state() -> rb_next()) and
return it. However if the current extent state record ends at or after the
target range passed to btrfs_clear_extent_bit_changeset() or
btrfs_convert_extent_bit(), we are just wasting time finding that next
node since we won't use it in those functions.

Improve on this by skipping the next node search if the current node ends
at or after the target range.

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>
4 weeks agobtrfs: exit after state insertion failure at btrfs_convert_extent_bit()
Filipe Manana [Thu, 10 Apr 2025 16:11:14 +0000 (17:11 +0100)]
btrfs: exit after state insertion failure at btrfs_convert_extent_bit()

If insert_state() state failed it returns an error pointer and we call
extent_io_tree_panic() which will trigger a BUG() call. However if
CONFIG_BUG is disabled, which is an uncommon and exotic scenario, then
we fallthrough and call cache_state() which will dereference the error
pointer, resulting in an invalid memory access.

So jump to the 'out' label after calling extent_io_tree_panic(), it also
makes the code more clear besides dealing with the exotic scenario where
CONFIG_BUG is disabled.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: exit after state split error at btrfs_convert_extent_bit()
Filipe Manana [Thu, 10 Apr 2025 16:01:59 +0000 (17:01 +0100)]
btrfs: exit after state split error at btrfs_convert_extent_bit()

If split_state() returned an error we call extent_io_tree_panic() which
will trigger a BUG() call. However if CONFIG_BUG is disabled, which is an
uncommon and exotic scenario, then we fallthrough and hit a use after free
when calling set_state_bits() since the extent state record which the
local variable 'prealloc' points to was freed by split_state().

So jump to the label 'out' after calling extent_io_tree_panic() and set
the 'prealloc' pointer to NULL since split_state() has already freed it
when it hit an error.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: remove duplicate error check at btrfs_convert_extent_bit()
Filipe Manana [Thu, 10 Apr 2025 15:40:29 +0000 (16:40 +0100)]
btrfs: remove duplicate error check at btrfs_convert_extent_bit()

There's no need to check if split_state() returned an error twice, instead
unify into a single if statement after setting 'prealloc' to NULL, because
on error split_state() frees the 'prealloc' extent state record.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: simplify last record detection at btrfs_clear_extent_bit_changeset()
Filipe Manana [Wed, 9 Apr 2025 17:15:03 +0000 (18:15 +0100)]
btrfs: simplify last record detection at btrfs_clear_extent_bit_changeset()

Instead of checking for an end offset of (u64)-1 (U64_MAX) for the current
extent state's end, and then checking after updating the current start
offset if it's now beyond the range's end offset, we can simply stop if
the current extent state's end is greater than or equals to our range's
end offset. This helps remove one comparison under the 'next' label and
allows to remove the if statement that checks if the start offset is
greater than the end offset under the 'search_again' label.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: avoid extra tree search at btrfs_clear_extent_bit_changeset()
Filipe Manana [Wed, 9 Apr 2025 16:15:35 +0000 (17:15 +0100)]
btrfs: avoid extra tree search at btrfs_clear_extent_bit_changeset()

When we find an extent state that starts before our range's start we
split it and jump into the 'search_again' label with our start offset
remaining the same, making us then go to the 'again' label and search
again for an extent state that contains the 'start' offset, and this
time it finds the same extent state but with its start offset set to
our range's start offset (due to the split). This is because we have
consumed the preallocated extent state record and we may need to split
again, and by jumping to 'again' we release the spinlock, allocate a new
prealloc state and restart the search.

However we may not need to restart and allocate a new extent state in
case we don't find extent states that cross our end offset, therefore
no need for further extent state splits, or we may be able to do an
atomic allocation (which is quick even if it fails). In these cases
it's a waste to restart the search.

So change the behaviour to do the restart only if we need to reschedule,
otherwise fall through - if we need to allocate an extent state for split
operations, we will try an atomic allocation and if that fails we will do
the restart as before.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: use bools for local variables at btrfs_clear_extent_bit_changeset()
Filipe Manana [Thu, 10 Apr 2025 12:26:40 +0000 (13:26 +0100)]
btrfs: use bools for local variables at btrfs_clear_extent_bit_changeset()

Several variables are defined as integers but used as booleans, and the
'delete' variable can be made const since it's not changed after being
declared. So change them to proper booleans and simplify setting their
value.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: add missing error return to btrfs_clear_extent_bit_changeset()
Filipe Manana [Thu, 10 Apr 2025 11:59:05 +0000 (12:59 +0100)]
btrfs: add missing error return to btrfs_clear_extent_bit_changeset()

We have a couple error branches where we have an error stored in the 'err'
variable and then jump to the 'out' label, however we don't return that
error, we just return 0. Normally this is not a problem since those error
branches call extent_io_tree_panic() which triggers a BUG() call, however
it's possible to have rather exotic kernel config with CONFIG_BUG disabled
in which case the BUG() call does nothing and we fallthrough. So make sure
to return the error, not just to fix that exotic case but also to make the
code less confusing. While at it also rename the 'err' variable to 'ret'
since this is the style we prefer and use more widely.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: exit after state split error at btrfs_clear_extent_bit_changeset()
Filipe Manana [Wed, 9 Apr 2025 15:17:16 +0000 (16:17 +0100)]
btrfs: exit after state split error at btrfs_clear_extent_bit_changeset()

If split_state() returned an error we call extent_io_tree_panic() which
will trigger a BUG() call. However if CONFIG_BUG is disabled, which is an
uncommon and exotic scenario, then we fallthrough and hit a use after free
when calling clear_state_bit() since the extent state record which the
local variable 'prealloc' points to was freed by split_state().

So jump to the label 'out' after calling extent_io_tree_panic() and set
the 'prealloc' pointer to NULL since split_state() has already freed it
when it hit an error.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: remove duplicate error check at btrfs_clear_extent_bit_changeset()
Filipe Manana [Wed, 9 Apr 2025 14:27:35 +0000 (15:27 +0100)]
btrfs: remove duplicate error check at btrfs_clear_extent_bit_changeset()

There's no need to check if split_state() returned an error twice, instead
unify into a single if statement after setting 'prealloc' to NULL, because
on error split_state() frees the 'prealloc' extent state record.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: get rid of btrfs_read_dev_super()
Qu Wenruo [Mon, 28 Apr 2025 00:46:19 +0000 (10:16 +0930)]
btrfs: get rid of btrfs_read_dev_super()

The function is introduced by commit a512bbf855ff ("Btrfs: superblock
duplication") at the beginning of btrfs.

It leaved a comment saying we'd need a special mount option to read all
super blocks, but it's never been implemented and there was not
need/request for it. The check/rescue tools are able to start from a
specific copy and use it as primary eventually.

This means btrfs_read_dev_super() is always reading the first super
block, making all the code finding the latest super block unnecessary.

Just remove that function and replace all call sites with
btrfs_read_disk_super(bdev, 0, false).

Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: merge btrfs_read_dev_one_super() into btrfs_read_disk_super()
Qu Wenruo [Mon, 28 Apr 2025 00:36:50 +0000 (10:06 +0930)]
btrfs: merge btrfs_read_dev_one_super() into btrfs_read_disk_super()

We have two functions to read a super block from a block device:

- btrfs_read_dev_one_super()
  Exported from disk-io.c

- btrfs_read_disk_super()
  Local to volumes.c

And they have some minor differences:

- btrfs_read_dev_one_super() uses @copy_num
  Meanwhile btrfs_read_disk_super() relies on the physical and expected
  bytenr passed from the caller.

  The parameter list of btrfs_read_dev_one_super() is more user
  friendly.

- btrfs_read_disk_super() makes sure the label is NUL terminated

We do not need two different functions doing the same job, so merge the
behavior into btrfs_read_disk_super() by:

- Remove btrfs_read_dev_one_super()

- Export btrfs_read_disk_super()
  The name pairs with btrfs_release_disk_super() perfectly.

- Change the parameter list of btrfs_read_disk_super() to mimic
  btrfs_read_dev_one_super()
  All existing callers are calculating the physical address and expect
  bytenr before calling btrfs_read_disk_super() already.

Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: get rid of goto in alloc_test_extent_buffer()
Daniel Vacek [Fri, 25 Apr 2025 07:23:57 +0000 (09:23 +0200)]
btrfs: get rid of goto in alloc_test_extent_buffer()

The `free_eb` label is used only once. Simplify by moving the code inplace.

Signed-off-by: Daniel Vacek <neelx@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: use buffer xarray for extent buffer writeback operations
Josef Bacik [Mon, 28 Apr 2025 14:52:57 +0000 (10:52 -0400)]
btrfs: use buffer xarray for extent buffer writeback operations

Currently we have this ugly back and forth with the btree writeback
where we find the folio, find the eb associated with that folio, and
then attempt to writeback.  This results in two different paths for
subpage ebs and >= page size ebs.

Clean this up by adding our own infrastructure around looking up tagged
ebs and writing the ebs out directly.  This allows us to unify the
subpage and >= pagesize IO paths, resulting in a much cleaner writeback
path for extent buffers.

I ran this through fsperf on a VM with 8 CPUs and 16GiB of RAM.  I used
smallfiles100k, but reduced the files to 1k to make it run faster, the
results are as follows, with the statistically significant improvements
marked with *, there were no regressions.  fsperf was run with -n 10 for
both runs, so the baseline is the average 10 runs and the test is the
average of 10 runs.

smallfiles100k results
      metric           baseline       current        stdev            diff
================================================================================
avg_commit_ms               68.58         58.44          3.35   -14.79% *
commits                    270.60        254.70         16.24    -5.88%
dev_read_iops                  48            48             0     0.00%
dev_read_kbytes              1044          1044             0     0.00%
dev_write_iops          866117.90     850028.10      14292.20    -1.86%
dev_write_kbytes      10939976.40   10605701.20     351330.32    -3.06%
elapsed                     49.30            33          1.64   -33.06% *
end_state_mount_ns    41251498.80   35773220.70    2531205.32   -13.28% *
end_state_umount_ns      1.90e+09      1.50e+09   14186226.85   -21.38% *
max_commit_ms                 139        111.60          9.72   -19.71% *
sys_cpu                      4.90          3.86          0.88   -21.29%
write_bw_bytes        42935768.20   64318451.10    1609415.05    49.80% *
write_clat_ns_mean      366431.69     243202.60      14161.98   -33.63% *
write_clat_ns_p50        49203.20         20992        264.40   -57.34% *
write_clat_ns_p99          827392     653721.60      65904.74   -20.99% *
write_io_kbytes           2035940       2035940             0     0.00%
write_iops               10482.37      15702.75        392.92    49.80% *
write_lat_ns_max         1.01e+08      90516129    3910102.06   -10.29% *
write_lat_ns_mean       366556.19     243308.48      14154.51   -33.62% *

As you can see we get about a 33% decrease runtime, with a 50%
throughput increase, which is pretty significant.

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: set DIRTY and WRITEBACK tags on the buffer_tree
Josef Bacik [Mon, 28 Apr 2025 14:52:56 +0000 (10:52 -0400)]
btrfs: set DIRTY and WRITEBACK tags on the buffer_tree

In preparation for changing how we do writeout of extent buffers, start
tagging the extent buffer xarray with DIRTY and WRITEBACK to make it
easier to find extent buffers that are in either state.

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: convert the buffer_radix to an xarray
Josef Bacik [Mon, 28 Apr 2025 14:52:55 +0000 (10:52 -0400)]
btrfs: convert the buffer_radix to an xarray

In order to fully utilize xarray tagging to improve writeback we need to
convert the buffer_radix to a proper xarray.  This conversion is
relatively straightforward as the radix code uses the xarray underneath.
Using xarray directly allows for quite a lot less code.

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: rename btrfs_discard workqueue to btrfs-discard
David Sterba [Tue, 29 Apr 2025 10:56:41 +0000 (12:56 +0200)]
btrfs: rename btrfs_discard workqueue to btrfs-discard

We use the "btrfs-" prefix for our workqueues, the discard has
underscore instead of dash, so unify it.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: on unknown chunk allocation policy fallback to regular
David Sterba [Wed, 23 Apr 2025 06:29:14 +0000 (08:29 +0200)]
btrfs: on unknown chunk allocation policy fallback to regular

We have only two chunk allocation policies right now and the
switch/cases don't handle an unknown one properly. The error is in the
impossible category (the policy is stored only in memory), we don't have
to BUG(), falling back to regular policy should be safe.

Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: reformat comments in acls_after_inode_item()
David Sterba [Wed, 23 Apr 2025 16:53:59 +0000 (18:53 +0200)]
btrfs: reformat comments in acls_after_inode_item()

Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: switch int dev_replace_is_ongoing variables/parameters to bool
David Sterba [Wed, 23 Apr 2025 16:53:58 +0000 (18:53 +0200)]
btrfs: switch int dev_replace_is_ongoing variables/parameters to bool

Both the variable and the parameter are used as logical indicators so
convert them to bool.

Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: trivial conversion to return bool instead of int
David Sterba [Wed, 23 Apr 2025 16:53:57 +0000 (18:53 +0200)]
btrfs: trivial conversion to return bool instead of int

Old code has a lot of int for bool return values, bool is recommended
and done in new code. Convert the trivial cases that do simple 0/false
and 1/true. Functions comment are updated if needed.

Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: subpage: reject tree blocks which are not nodesize aligned
Qu Wenruo [Wed, 23 Apr 2025 07:06:14 +0000 (16:36 +0930)]
btrfs: subpage: reject tree blocks which are not nodesize aligned

When btrfs subpage support (fs block < page size) was introduced, a
subpage filesystem will only reject tree blocks which cross page
boundaries.

This used to be a compromise to simplify the tree block handling and
still allowing subpage cases to read some old converted filesystems
which did not have proper chunk alignment.

But in practice, suppose we have the following unaligned tree block on a
64K page sized system:

  0                           32K           44K             60K  64K
  |                                         |///////////////|    |

Although btrfs has no problem reading the tree block at [44K, 60K), if
extent allocator is allocating another tree block, it may choose the
range [60K, 74K), as extent allocator has no awareness if it's a subpage
metadata request or not.

Then we'd get -EINVAL from the following sequence:

 btrfs_alloc_tree_block()
 |- btrfs_reserve_extent()
 |  Which returned range [60K, 74K)
 |- btrfs_init_new_buffer()
    |- btrfs_find_create_tree_block()
       |- alloc_extent_buffer()
          |- check_eb_alignment()
     Which returned -EINVAL, because the range crosses page
     boundary.

This situation will not fix itself and should mostly mark the fs
read-only.

Thankfully we didn't really get such reports in the real world because:

- The original unaligned tree block is only caused by older
  btrfs-convert
  It's before the btrfs-convert rework was done in v4.6, where converted
  btrfs filesystem can have metadata block groups which are not aligned
  to nodesize nor stripe size (64K).

  But after btrfs-progs v4.6, all chunks allocated will be stripe (64K)
  aligned, thus no more such problem.

Considering how old the fix is (v4.6 was released almost 10 years ago),
subpage support for btrfs was introduced in v5.15, it should be safe to
reject those unaligned tree blocks.

Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: move folio initialization to one place in attach_eb_folio_to_filemap()
Daniel Vacek [Wed, 23 Apr 2025 08:51:22 +0000 (10:51 +0200)]
btrfs: move folio initialization to one place in attach_eb_folio_to_filemap()

This is just a trivial change. The code looks a bit more readable this way, IMO.

Move initialization of existing_folio to the beginning of the retry loop
so it's set to NULL at one place.

Signed-off-by: Daniel Vacek <neelx@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: raid56: rename parameter err to status in endio helpers
David Sterba [Wed, 23 Apr 2025 15:57:24 +0000 (17:57 +0200)]
btrfs: raid56: rename parameter err to status in endio helpers

Trivial renames to unify the naming of blk_status_t variables/parameters.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: change return type of btrfs_alloc_dummy_sum() to int
David Sterba [Wed, 23 Apr 2025 15:57:23 +0000 (17:57 +0200)]
btrfs: change return type of btrfs_alloc_dummy_sum() to int

The type blk_status_t is from block layer and not related to checksums
in our context. Use int internally and do the conversions to blk_status_t
as needed in btrfs_submit_chunk().

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: rename ret2 to ret in btrfs_submit_compressed_read()
David Sterba [Wed, 23 Apr 2025 15:57:22 +0000 (17:57 +0200)]
btrfs: rename ret2 to ret in btrfs_submit_compressed_read()

We can now rename 'ret2' to 'ret' and use it for generic errors.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: rename ret to status in btrfs_submit_compressed_read()
David Sterba [Wed, 23 Apr 2025 15:57:21 +0000 (17:57 +0200)]
btrfs: rename ret to status in btrfs_submit_compressed_read()

We're using 'status' for the blk_status_t variables, rename 'ret' so we can
use it for generic errors.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: simplify reading bio status in end_compressed_writeback()
David Sterba [Wed, 23 Apr 2025 15:57:20 +0000 (17:57 +0200)]
btrfs: simplify reading bio status in end_compressed_writeback()

We don't need to have a separate variable to read the bio status, 'ret'
works for that just fine so remove 'error'.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: rename error to ret in btrfs_submit_chunk()
David Sterba [Wed, 23 Apr 2025 15:57:19 +0000 (17:57 +0200)]
btrfs: rename error to ret in btrfs_submit_chunk()

We can now rename 'error' to 'ret' and use it for generic errors.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: rename ret to status in btrfs_submit_chunk()
David Sterba [Wed, 23 Apr 2025 15:57:18 +0000 (17:57 +0200)]
btrfs: rename ret to status in btrfs_submit_chunk()

We're using 'status' for the blk_status_t variables, rename 'ret' so we
can use it for proper return type.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: change return type of btrfs_bio_csum() to int
David Sterba [Wed, 23 Apr 2025 15:57:17 +0000 (17:57 +0200)]
btrfs: change return type of btrfs_bio_csum() to int

The type blk_status_t is from block layer and not related to checksums
in our context. Use int internally and do the conversions to blk_status_t
as needed.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: change return type of btree_csum_one_bio() to int
David Sterba [Wed, 23 Apr 2025 15:57:16 +0000 (17:57 +0200)]
btrfs: change return type of btree_csum_one_bio() to int

The type blk_status_t is from block layer and not related to checksums
in our context. Use int internally and do the conversions to blk_status_t
as needed in btrfs_bio_csum().

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: change return type of btrfs_csum_one_bio() to int
David Sterba [Wed, 23 Apr 2025 15:57:15 +0000 (17:57 +0200)]
btrfs: change return type of btrfs_csum_one_bio() to int

The type blk_status_t is from block layer and not related to checksums
in our context. Use int internally and do the conversions to blk_status_t
as needed in btrfs_bio_csum().

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: change return type of btrfs_lookup_bio_sums() to int
David Sterba [Wed, 23 Apr 2025 15:57:14 +0000 (17:57 +0200)]
btrfs: change return type of btrfs_lookup_bio_sums() to int

The type blk_status_t is from block layer and not related to checksums
in our context. Use int internally and do the conversions to blk_status_t
as needed in btrfs_submit_chunk().

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: drop redundant local variable in raid_wait_write_end_io()
David Sterba [Wed, 23 Apr 2025 15:57:13 +0000 (17:57 +0200)]
btrfs: drop redundant local variable in raid_wait_write_end_io()

The bio status is read only once, no variable needed for that.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: merge __setup_root() to btrfs_alloc_root()
David Sterba [Wed, 23 Apr 2025 07:30:48 +0000 (09:30 +0200)]
btrfs: merge __setup_root() to btrfs_alloc_root()

There's only one caller of __setup_root() so merge it there.

Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: use unsigned types for constants defined as bit shifts
David Sterba [Tue, 22 Apr 2025 15:55:41 +0000 (17:55 +0200)]
btrfs: use unsigned types for constants defined as bit shifts

The unsigned type is a recommended practice (CWE-190, CWE-194) for bit
shifts to avoid problems with potential unwanted sign extensions.
Although there are no such cases in btrfs codebase, follow the
recommendation.

Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: remove unused btrfs_io_stripe::length
David Sterba [Tue, 22 Apr 2025 15:32:17 +0000 (17:32 +0200)]
btrfs: remove unused btrfs_io_stripe::length

First added (but not effectively used) in 02c372e1f016e5 ("btrfs: add
support for inserting raid stripe extents"). The structure is
initialized to zeros so the only use in btrfs_insert_one_raid_extent()

    u64 length = bioc->stripes[i].length;
    struct btrfs_raid_stride *raid_stride = &stripe_extent->strides[i];

    if (length == 0)
            length = bioc->size;

the 'if' always happens.

Last use in 4016358e852861 ("btrfs: remove unused variable length in
btrfs_insert_one_raid_extent()") was an obvious cleanup. It seems to be
safe to remove, raid-stripe-tree works without using it since 6.6.

This was found by tool https://github.com/jirislaby/clang-struct .

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: use list_first_entry() everywhere
David Sterba [Tue, 22 Apr 2025 16:21:51 +0000 (18:21 +0200)]
btrfs: use list_first_entry() everywhere

Using the helper makes it a bit more clear that we're accessing the
first list entry.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: convert ASSERT(0) with handled errors to DEBUG_WARN()
David Sterba [Thu, 17 Apr 2025 09:17:03 +0000 (11:17 +0200)]
btrfs: convert ASSERT(0) with handled errors to DEBUG_WARN()

The use of ASSERT(0) is maybe useful for some cases but more like a
notice for developers. Assertions can be compiled in independently so
convert it to a debugging helper.

The difference is that it's just a warning and will not end up in BUG().
The converted cases are in connection with proper error handling.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: convert WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG)) to DEBUG_WARN
David Sterba [Thu, 17 Apr 2025 09:17:02 +0000 (11:17 +0200)]
btrfs: convert WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG)) to DEBUG_WARN

Use the conditional warning instead of typing the whole condition.
Optional message is printed where it seems clear what could be the
problem.

Conversion is left out in btree_csum_one_bio() because of the additional
condition.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: add debug build only WARN
David Sterba [Thu, 17 Apr 2025 09:17:01 +0000 (11:17 +0200)]
btrfs: add debug build only WARN

Add conditional WARN() wrapper that's enabled only in debug build. It
should be used for unexpected conditions that should be noisy.  Use it
instead of ASSERT(0). As it will not lead to BUG() make sure that
continuing is still possible, e.g. the error is handled anyway.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: use verbose ASSERT() in volumes.c
David Sterba [Thu, 17 Apr 2025 09:17:00 +0000 (11:17 +0200)]
btrfs: use verbose ASSERT() in volumes.c

The file volumes.c has about 40 assertions and half of them are suitable
for ASSERT() with additional data.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: enhance ASSERT() to take optional format string
David Sterba [Thu, 17 Apr 2025 09:16:59 +0000 (11:16 +0200)]
btrfs: enhance ASSERT() to take optional format string

Currently ASSERT() prints the stringified condition and without macro
expansions so simple constants like BTRFS_MAX_METADATA_BLOCKSIZE remain
readable in the output.

There are expressions where we'd like to see the exact values but all we
get is something like:

  assertion failed: em->start <= start && start < extent_map_end(em), in fs/btrfs/extent_map.c:613

It would be nice to be able to print any additional information to help
understand the problem. With some preprocessor magic and compile-time
optimizations we can enhance ASSERT to work like that as well:

  ASSERT(value > limit, "value=%llu limit=%llu", value, limit);

with free-form printk arguments that will be part of the assertion
message.

Pros:
- helps debugging and understanding reported problems
- the optional format is verified at compile-time

Cons:
- increases the .ko size
- writing the assertion code is repetitive (condition, format, values)
- format and variable type must match (extra lookup)
- needs gcc 8.x and newer, otherwise it's the short format

Recommended use is for non-trivial expressions, so basic ASSERT(value) can be
used for pointers or sometimes integers.

The format has been slightly updated to also print the result of the
evaluation of the condition, appended to the stringified condition as
"condition :: <value>".

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
4 weeks agobtrfs: remove BTRFS_REF_LAST from enum btrfs_ref_type
Yangtao Li [Thu, 17 Apr 2025 14:26:49 +0000 (08:26 -0600)]
btrfs: remove BTRFS_REF_LAST from enum btrfs_ref_type

Commit b28b1f0ce44c ("btrfs: delayed-ref: Introduce better documented
delayed ref structures") introduced BTRFS_REF_LAST, which can be used
for sanity checking, e.g. in switch/case or for loops.

In btrfs_ref_type() there is an assertion

  ASSERT(ref->type == BTRFS_REF_DATA || ref->type == BTRFS_REF_METADATA);

to validate the values so we don't need the ending enum.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>