Skip to content

Test for-next (regular, SELF kvm) - #1624

Open
kdave wants to merge 10000 commits into
ci-kvmfrom
for-next
Open

kdave wants to merge 10000 commits into
ci-kvmfrom
for-next

Conversation

@kdave

@kdave kdave commented Mar 5, 2026 •

Copy link
Copy Markdown
Member

Keep this open, the build tests are on self-hosted workers.

@kdave

ghost commented Mar 5, 2026

Copy link
Copy Markdown
Member Author

Re #1623 .

@kdave kdave changed the title Test for-next (regular, GH kvm) 2 Test for-next (regular, GH kvm) Mar 5, 2026
@kdave kdave closed this Mar 5, 2026
@kdave kdave reopened this Mar 5, 2026
@kdave
kdave force-pushed the ci-kvm branch 2 times, most recently from 69fc6c9 to 98bf7e7 Compare March 5, 2026 23:30
@kdave
kdave force-pushed the for-next branch 3 times, most recently from 934d926 to 0cb5a8a Compare March 13, 2026 11:46
@kdave
kdave force-pushed the ci-kvm branch 3 times, most recently from 2cd3911 to c09d7cb Compare March 13, 2026 19:11
@kdave
kdave force-pushed the for-next branch 8 times, most recently from c61e262 to daed989 Compare March 17, 2026 16:00
@kdave
kdave force-pushed the for-next branch 4 times, most recently from 9dc51d6 to d76ed94 Compare March 19, 2026 13:21
Johannes Thumshirn and others added 30 commits October 29, 2026 14:05
wait_eb_writebacks() iterates the fs_info->buffer_tree xarray and waits
for the writeback of these extent-buffers. Waiting for writeback needs
to be done without the rcu_read_lock() held (because it can sleep) and
thus the loop drops the rcu_read_lock() before calling into
wait_on_extent_buffer_writeback(). But extent_buffers are freed through
RCU, so if btrfs_release_extent_buffer_rcu() is scheduled while we're
still waiting on the writeback, the extent_buffer will be freed causing
a use-after-free.

Similar to what is done in find_extent_buffer_nolock(), get a reference
to the extent_buffer before calling into
wait_on_extent_buffer_writeback() so a successful writeback does not
free the extent_buffer while we still have a reference to it.

Fixes: 2dd7e7b ("btrfs: zoned: wait for extent buffer IOs before finishing a zone")
Reviewed-by: Daniel Vacek <neelx@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
btrfs_reset_unused_block_groups() drops the block-group from
fs_info->unused_bgs before performing the work that can actually fail,
the actual submission of a REQ_OP_ZONE_RESET.

Every failed operation after this leaves the block-group dangling,
physically reset, but the in-memory structures aren't and it is not
requeued to be reset on fs_info->unused_bgs.

Instead of bailing out, re-add this block-group to fs_info->unused_bgs
list.

Fixes: 453a73c ("btrfs: zoned: reclaim unused zone by zone resetting")
Reviewed-by: Daniel Vacek <neelx@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The two functions btrfs_free_reserved_bytes() and
btrfs_reset_unused_block_groups() both manipulate
space_info->bytes_zone_unusable without using the accessor function
btrfs_space_info_update_bytes_zone_unusable() potentially risking an
underflow (in case of btrfs_reset_unused_block_groups()).

Use btrfs_space_info_update_bytes_zone_unusable() in both function, in
the case of btrfs_free_reserved_bytes() it is mostly for documenting
purposes.

Reviewed-by: Daniel Vacek <neelx@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The on-disk btrfs_chunk structure uses u16 for both num_stripes and
sub_stripes, so there is no need to use "int" to represent them in
btrfs_chunk_map.

Follow the on-disk format to use u16 for those members, including
verified_stripes too.

And since verified_stripes is u16, it no longer completely fill the
hole between @refs and @start.
Instead use @num_stripes and @sub_stripes to fill the hole, and leave a 6
bytes hole between @verified_stripes and @stripes.

This doesn't save any space for btrfs_chunk_map, but leaves enough space
for future expansion.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Currently btrfs doesn't load btrfs_chunk::stripe_len into
btrfs_chunk_map, as tree-checker will reject any chunk whose stripe_len
is not 64K.

But for the future we will want stripe length other than 64K.

As a preparation for the variable stripe length in the future, load
btrfs_chunk::stripe_len into btrfs_chunk_map::stripe_len and
stripe_shift.

For now stripe_len is still 64K, no matter if it's loaded from the
on-disk metadata or newly allocated chunk.
As tree-checker still rejects any chunk whose stripe_len is not 64K.

The two new members fit into the existing hole of btrfs_chunk_map, so
the size of btrfs_chunk_map stay the same.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The new member will be utilized by various path that no direct
btrfs_chunk_map is available, but a stripe length is still required.

The new member is located inside a hole between @refs and @fs_info, so
no size increase for btrfs_io_context.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Several points to note:

- scrub_ctx::stripes[] stripe_len updates
  We always pre-initialize scrub_ctx::stripes[] at the very beginning of
  scrub, without holding any btrfs_chunk_map.

  So init_scrub_stripe() still uses BTRFS_STRIPE_LEN, and all allocated
  memory are based on that stripe length.

  But before scrubbing each chunk, update_scrub_stripe_len() will be
  called to update the scrub_stripe::stripe_len.

- Stripe_len should not exceed BTRFS_STRIPE_LEN
  Since scrub_stripe's heap memory are all based on BTRFS_STRIPE_LEN,
  any stripe_len larger than BTRFS_STRIPE_LEN will cause
  access-beyond-boundary.

  Thankfully there is no plan to support such stripe_len >
  BTRFS_STRIPE_LEN any time soon.

- alloc_scrub_bbio() changes
  Previously we ensure BTRFS_STRIPE_LEN >= PAGE_SIZE, so
  BTRFS_STRIPE_LEN >> PAGE_SHIFT can never be zero.

  But now stripe_len can be smaller than PAGE_SIZE. E.g. stripe_len 4K,
  page size 64K.

  So we have to use "round_up(stripe_len, PAGE_SIZE) >> PAGE_SHIFT"
  instead.

  Finally alloc_scrub_bbio() is always called with a scrub_stripe
  pointer as the @Private parameter, change the type from (void *) to
  (struct scrub_stripe *), so that we can grab the stripe_len without
  a new parameter.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
That helper is currently using a fixed BTRFS_STRIPE_LEN, change it to
accept a 'u32 stripe_len'.

Also add an ASSERT() to make sure the @stripe_len passed in is power of
2, and no larger than BTRFS_STRIPE_LEN.

There is a special case where we still pass BTRFS_STRIPE_LEN into
btrfs_stripe_nr_to_offset(), that's for chunk allocation to determine
the minimal device extent size.

In that case, since our future planned map->stripe_len is always power
of 2 and no larger than BTRFS_STRIPE_LEN, we are totally fine to use the
old BTRFS_STRIPE_LEN.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
There are still some BTRFS_STRIPE_LEN usage remaining, but they are all
for chunk allocation.

There is one more refactor for is_same_chunk_map(), as there are more
members to compare, the parameter list can go longer and longer.

Instead of passing a new @stripe_len member, pass extent_buffer and
btrfs_chunk pointers, and grab the needed member inside that function.

And doing "if (unlikely()) return false" for each member check.

Although this make is_same_chunk_map() more complex, considering it's
only called for system chunks, it should not bring too much overhead.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
So that raid56 can support stripe length other than BTRFS_STRIPE_LEN.

There are several extra things to note:

- btrfs_raid_bio::stripe_npages is removed
  With stripe_len < BTRFS_STRIPE_LEN, there is no guarantee that a page
  won't cross multiple stripes.

  E.g. with 4K stripe_len and sectorsize, 2 data stripes, with 64K page
  size, one page will cover the full stripe.

  So stripe_npages no longer makes sense, just remove it.

- alloc_rbio_data_pages()/alloc_rbio_parity_pages() fallback
  For the same above reason, a page can contain both data and parity
  blocks.
  In that case we can no longer only allocate pages for data/parity.

  Add a fallback that if stripe_len is smaller than page size, always
  allocate all the pages.

  This is not the best solution, but should be good enough for now until
  the variable stripe_len is properly tested.

- steal_rbio() fallback
  If stripe_len is smaller than PAGE_SIZE, it's hard to only steal the
  data pages.

  Again skip the stealing if stripe_len is smaller than PAGE_SIZE.

Reviewed-by: Johannes Thusmhirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
There are still several call sites using BTRFS_STRIPE_LEN, but that's
when doing the basic zoned checks.

For other call sites with a btrfs_chunk_map, use map->stripe_len instead
of the hardcoded BTRFS_STRIPE_LEN.

And since we're here, replace the following pattern

	if (val & BTRFS_STRIPE_LEN_MASK)

with
	if (!IS_ALIGNED(val, stripe_len))

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The new limits are:

- Stripe_len must be no smaller than sectorsize
  As sectorsize is still the minimal unit.

- Stripe_len must be no larger than BTRFS_STRIPE_LEN
  We still have a lot of things requiring on BTRFS_STRIPE_LEN alignment,
  e.g. dev extent and chunk allocation.
  A stripe_len larger than BTRFS_STRIPE_LEN will cause problems for the
  existing chunk allocator.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
There are several BUG()/BUG_ON() inside raid56.c, which cause
checkpatch warning if code is touching them. This is especially annoying
if using b4, with btrfs-workflow git hooks applied, which makes b4 to
wait for user input but without any output.

Address them by removing one very impossible BUG(), and using ASSERT() for
the remaining ones:

- __remove_rbio_from_cache()
- rbio_is_full()
- unlock_stripe()
  Replace the BUG_ON() with ASSERT().

- finish_parity_scrub()
  Remove the BUG(), as alloc_rbio() has already properly calculated
  real_stripes and nr_data using the number of parity.

  Furthermore, initialize @has_qstripe at the beginning of the function
  using the bioc->map_type directly.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
An extent in a block-group that uses the RAID stripe-tree has its logical
to physical mapping stored in that tree, which only becomes persistent
when the stripe-tree is written out at transaction commit. But the RAID
stripe-tree is not logged in the log tree.

A fsync therefore logs a file-extent item whose logical range cannot be
resolved after a crash. Replay restores the file-extent item and the
file has the correct size, but reading it fails with EIO and nothing is
reported.

Force a full commit when logging such an extent. Extents reach the log
both from log_one_extent() and from copy_items(), so check in both.
copy_items() skips extents from a past transaction, their mapping is
already committed, the same reason their checksums are not logged.

Fixes: 02c372e ("btrfs: add support for inserting raid stripe extents")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
When using the no-holes feature (enabled by default since btrfs-progs
v5.15) and doing an incremental send, we can miss issuing a hole punching
command when the following conditions happen:

1) A hole is punched in a tail section of a file extent item that happens
   to be the last item in a leaf (for example the extent covers the file
   range [1M, 3M) and a hole is punched in the range [2M, 3M)). Between
   the end of this file extent item's range and the i_size, there are
   other file extent items;

2) The next leaf was not modified in the send snapshot, remaining shared
   with the parent snapshot.

In this scenario we are unable to detect a hole was punched and never
issue a hole punch command because with no-holes there we don't have an
explicit file extent item to mark a hole.

Fix this by teaching maybe_send_hole() to detect such case.
A test case for fstests will follow soon.

Reported-by: Tristan Elvers <tristanelvers@gmail.com>
Link: https://lore.kernel.org/linux-btrfs/CAFoLKK+aErV4p6ZJPnVeH5SHQ5++0f=8ZgxPWe-E-CkeKBs8PA@mail.gmail.com/
Fixes: 16e7549 ("Btrfs: incompatible format change to remove hole extents")
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
…e_info()

There's no point in setting the block group's space_info member as the
space_info was extracted from the block group early when the variable was
declared and did not change.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
It's not used, so remove it.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The pattern "rbio->nr_data * rbio->stripe_nsectors" is utilized more
than 5 times inside raid56.c, it's worthy to extract it into a new
helper.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The function steal_rbio() is utilized to use the pages that belongs to
uptodate data blocks, without doing a page copy.

That is done by just updating the dest->stripe_pages[] pointer to use
the point inside @src directly.

That function previously will reject stripe_len < PAGE_SIZE cases, as a
page can contain both data and P/Q blocks.

But that shouldn't be a big deal, as long as the corresponding P/Q
blocks are not marked uptodate, we can still steal the page that also
contains P/Q blocks.

So rework steal_rbio() to support stripe_len < PAGE_SIZE by:

- Calculate the last page index correctly
  Instead of iterating all pages but skip P/Q pages, calculate the last
  page index properly.

  The last page may contain both data and P/Q blocks, but we can still
  steal the page.

- Update the bitmap in one go
  Instead of updating the stripe_uptodate_bitmap for every page, we can
  do the update in just one go, out of the main loop.

  And that update should not touch the P/Q blocks.

- Remove unused helpers
  * page_nr_to_sector_nr()
  * page_nr_to_num_sectors()
  * steal_rbio_page()
  * is_data_stripe_page()

  * full_page_sectors_uptodate()
    This one is very annoying, because of its __maybe_unused
    attribution.

  They are based on the old BTRFS_STRIPE_LEN >= PAGE_SIZE assumption,
  and no longer utilized.

- Make index_stripe_sectors() update the stripe_paddrs[] without a page
  This doesn't cause any functional change for now, as there is no one
  accessing the source stripe_paddrs[] after a steal.

  But this should still make the function a little more robust.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The functions alloc_rbio_parity_pages() and alloc_rbio_data_pages()
didn't support stripe_len < PAGE_SIZE cases, and fallback to
alloc_rbio_pages() instead.

That's mostly to avoid the case where both data and P/Q blocks can exist
in the same page.

However even in that case, it's not a big deal, the existing
infrastructures like index_stripe_sectors() can handle it correctly.

Update those functions to support stripe_len < PAGE_SIZE cases, with two
points to note:

- Extra attention needed for the round direction
  For alloc_rbio_data_pages() we need to round up, so the last data page
  gets allocated.

  For alloc_rbio_parity_pages() we need to round down, so the first
  P/Q page get allocated.

- Mixed page allocation
  We can call alloc_rbio_parity_pages() first, then
  alloc_rbio_data_pages().

  In that case, it's alloc_pages_bulk() saving us, which always skips
  allocated slot. Thus we're still safe.

Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
We are manually mapping the folio, copying from it with memcpy() and then
unmapping, but this can all simply be done with memcpy_from_folio() that
is provided by the MM layer. So use memcpy_from_folio() instead.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshrin@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
We are manually mapping the folio, copying to it with memcpy() and then
unmapping, but this can all simply be done with memcpy_to_folio()  that
is provided by the MM layer. So use memcpy_to_folio() instead.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshrin@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
do_zone_finish() calls btrfs_wait_ordered_roots(), which holds
fs_info->ordered_operations_mutex while waiting for writeback. It can be
reached from writeback itself:

  cow_file_range()
  `-> btrfs_reserve_extent()
    `-> find_free_extent()
      `-> btrfs_zone_finish_one_bg()
        `-> do_zone_finish()

If another task holds the mutex and waits for that writeback, this
deadlocks.

In general, we want to ensure all writes in the block group to
complete before finishing the zone, as relocation does. However, the
current callers do not need the wait (or cannot wait) in do_zone_finish():

- Relocation already waits for reservations and ordered extents itself
  before calling btrfs_zone_finish().
- btrfs_delete_unused_bgs() checks that ->reserved is zero and sets the
  block group read-only while holding groups_sem, so there can be no
  in-flight data write.
- btrfs_zone_finish_one_bg() is called from the allocation path and
  cannot wait.

Dropping the wait is still safe. Once the block group is read-only,
->reserved can only drop, and for data it only drops after the ordered
extent's I/O has completed. So if ->reserved is zero, no data write is
left in flight. If it is non-zero, do_zone_finish() returns -EAGAIN,
which all callers handle.

This makes btrfs_zone_finish_one_bg() fail with -EAGAIN when it races
with an allocation in the selected block group.

Fixes: 393f646 ("btrfs: zoned: finish least available block group on data bg allocation")
Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
When mounting a filesystem on a zoned drive passed into a VM via a
virtio-blk disk lockdep sporadically complains about a possible deadlock
between BTRFS' dev_replace::rwsem and virtio-blk's vdev_mutex:

 ======================================================
 WARNING: possible circular locking dependency detected
 7.3.0-rc6+ #317 Not tainted
 ------------------------------------------------------
 mount/7418 is trying to acquire lock:
 ffff888102c07860 (&vblk->vdev_mutex){+.+.}-{4:4}, at: virtblk_report_zones+0xda/0x440

 but task is already holding lock:
 ffff8881fd725ea8 (&fs_info->dev_replace.rwsem){++++}-{4:4}, at: btrfs_load_block_group_zone_info+0x186/0xe50

 which lock already depends on the new lock.

 the existing dependency chain (in reverse order) is:

 -> #4 (&fs_info->dev_replace.rwsem){++++}-{4:4}:
        down_read+0x42/0x170
        btrfs_map_block+0x1a6/0xe70
        btrfs_submit_bbio+0x1a3/0x910
        submit_one_bio+0x93/0xd0
        btrfs_readahead+0x1c4/0x200
        read_pages+0x56/0x200
        page_cache_ra_order+0x27c/0x410
        filemap_get_pages+0x12f/0x4a0
        filemap_read+0xed/0x400
        vfs_read+0x240/0x340
        ksys_read+0x61/0xd0
        do_syscall_64+0xa3/0x4f0
        entry_SYSCALL_64_after_hwframe+0x76/0x7e

 -> #3 (mapping.invalidate_lock#3){++++}-{4:4}:
        down_read+0x42/0x170
        filemap_fault+0xb0/0x1a40
        __do_fault+0x34/0xc0
        __handle_mm_fault+0x919/0x1db0
        handle_mm_fault+0x97/0x240
        do_user_addr_fault+0x21d/0x690
        exc_page_fault+0x8e/0x230
        asm_exc_page_fault+0x26/0x30

 -> #2 (vm_lock){++++}-{0:0}:
        __vma_start_exclude_readers+0x91/0x1b0
        __vma_start_write+0x47/0xd0
        vma_expand+0x4c/0x2a0
        relocate_vma_down+0x126/0x200
        setup_arg_pages+0x240/0x3e0
        load_elf_binary+0x1be/0x1860
        bprm_execve+0x2d2/0x7b0
        kernel_execve+0x107/0x130
        kernel_init+0xc2/0x130
        ret_from_fork+0x1ff/0x2f0
        ret_from_fork_asm+0x1a/0x30

 -> #1 (&mm->mmap_lock){++++}-{4:4}:
        __might_fault+0x68/0xa0
        _copy_to_user+0x22/0x70
        blkdev_copy_zone_to_user+0x22/0x40
        virtblk_report_zones+0x297/0x440
        blkdev_report_zones_ioctl+0xfd/0x130
        blkdev_ioctl+0x20f/0x2c0
        __x64_sys_ioctl+0x86/0xd0
        do_syscall_64+0xa3/0x4f0
        entry_SYSCALL_64_after_hwframe+0x76/0x7e

 -> #0 (&vblk->vdev_mutex){+.+.}-{4:4}:
        __lock_acquire+0x15a8/0x2850
        lock_acquire+0xea/0x2f0
        __mutex_lock+0xa7/0x10d0
        virtblk_report_zones+0xda/0x440
        blkdev_report_zones_cached+0x162/0x190
        btrfs_get_dev_zones+0xdc/0x2c0
        btrfs_load_block_group_zone_info+0x2b6/0xe50
        btrfs_read_block_groups+0x338/0xc80
        open_ctree+0x1aca/0x24b3
        btrfs_get_tree.cold+0x105/0x26c
        vfs_get_tree+0x28/0xb0
        __do_sys_fsconfig+0x324/0x690
        do_syscall_64+0xa3/0x4f0
        entry_SYSCALL_64_after_hwframe+0x76/0x7e

 other info that might help us debug this:

 Chain exists of:
   &vblk->vdev_mutex --> mapping.invalidate_lock#3 --> &fs_info->dev_replace.rwsem

  Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   rlock(&fs_info->dev_replace.rwsem);
                                lock(mapping.invalidate_lock#3);
                                lock(&fs_info->dev_replace.rwsem);
   lock(&vblk->vdev_mutex);

  *** DEADLOCK ***

 locks held by mount/7418: 3, last CPU#0:
  #0: ffff888110aa6868 (&fc->uapi_mutex){+.+.}-{4:4}, at: __do_sys_fsconfig+0x2ae/0x690
  #1: ffff888112ba20e8 (&type->s_umount_key#31/1){+.+.}-{4:4}, at: sget_fc+0x148/0x8d0
  #2: ffff8881fd725ea8 (&fs_info->dev_replace.rwsem){++++}-{4:4}, at: btrfs_load_block_group_zone_info+0x186/0xe50

Holding the rwsem was widened in 0090d6e ("btrfs: zoned: fix
use-after-free due to race with dev replace"), but it now also covers
calling into blkdev_report_zones_cached(). The device replace ioctl()
only works on a mounted filesystem and the only caller with setting
'new' to false is read_one_block_group() which is only called during
mount, so it is safe to drop the rwsem before calling into
blkdev_report_zones_cached().

Fixes: 0090d6e ("btrfs: zoned: fix use-after-free due to race with dev replace")
Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
btrfs_clone_dev_zone_info() duplicates the original orig_dev->zone_info
with kmemdup(), which also copies the seq_zones, empty_zones and
active_zones pointers. The clone then replaces them one by one with
fresh bitmaps. If one of the allocations fails, the error path frees all
three pointers, including those not yet replaced, which still point to
the bitmaps owned by the original device.

Clear the three pointers right after kmemdup() so the error path only frees
what this function allocated.

Fixes: 21e61ec ("btrfs: zoned: clone zoned device info when cloning a device")
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
Signed-off-by: David Sterba <dsterba@suse.com>
If an RMW work failed early, e.g. alloc_rbio_parity_pages() failed, it
won't get RBIO_RMW_LOCKED_BIT set, and get rbio_orig_end_io() called.

In that case, there can be a race between bio_list_get() on the rbio,
and new incoming bio merging into the rbio:

 A is the rbio that rbio_orig_end_io() is running on.
 a is the higher layer bio that generated the rbio A.

 B is the new rbio that will be merged into A.
 b is the higher layer bio that generated the rbio B.

   rbio_orig_end_io(A) worker         |       lock_stripe_add(B)
--------------------------------------+---------------------------------
rbio_orig_end_io(A)                   | lock_stripe_add(B)
|- cur = bio_list_get(A)              | |- rbio_can_merge(A, B)
|  |- bio = A->bio_list.head;         | |  which returned true
|  |  which bio a is returned.        | |
|  |                                  | |
|  |- A->bio_list.tail = NULL;        | |
|  |                                  | |
|  |                                  | |- merge_rbio(A, B)
|  |                                  | |  |- bio_list_merge_init()
|  |                                  | |     |- if (A->bio_list.tail)
|  |                                  | |     |  A->bio_list.tail is
|  |                                  | |     |  still NULL
|  |                                  | |     |
|  |                                  | |     |- A->bio_list.head = b
|  |                                  | |     |- A->bio_list.tail = b
|  |- A->bio_list.head = NULL;        | |- return 1;
|  |- return bio;                     |    Caller expect rbio A to
|     which is bio a.                 |    complete bio b.
|
|- extra = bio_list_get()
|  Which is NULL
|
|- rbio_endio_bio_list(cur);
   Which only ends bio a.

In above case, the bio_list race can get bio b lost, and never ended.
Thankfully this race window is very tiny, so not practical in the real
world.

Fix it by setting RBIO_RMW_LOCKED_BIT for error path before calling
rbio_orig_end_io().
And since we're ensuring all RMW rbios have RBIO_RMW_LOCKED_BIT bit set
before calling rbio_orig_end_io(), add two ASSERT()s for that and remove
the old comment.

Fixes: 5eb30ee ("btrfs: raid56: introduce the main entrance for RMW path")
Assisted-by: LLM (find and analyze the bug)
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
If rmw_rbio() hit an error after cache_rbio_pages() call, the rbio still
have RBIO_CACHE_READY_BIT set, thus can be utilized for future page
stealing.

But that rbio didn't really reach the disk, thus using the pages for
future steal can cause data inconsistency between in-memory cache and
on-disk data.

Fix the problem by clearing the RBIO_CACHE_READY_BIT on the error path
of rmw_rbio().

Fixes: 5eb30ee ("btrfs: raid56: introduce the main entrance for RMW path")
Assisted-by: LLM (find the bug)
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
When putting a workspace we always protect the workspace for the maximum
level from reclaim, since it can be used not just for that level but as
well for any level below it. However the logic is flawed because if we do
the put for a workspace at level 10 for example (maximum is level 15) and
there is currently no idle workspace for the maximum level because all
are currently being used, we don't add the workspace to the lru list and
that can result in leaking it if we don't get more compression or
decompression requests for level 10 or any level below 10.

Fix this by skipping the addition to the lru list only if the level of
the workspace matches the maximum level too (besides the list of idle
workspaces for the maximum level being empty).

Fixes: 3f93aef ("btrfs: add zstd compression level support")
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
…space manager

The workspace manager is per filesystem and we have access to the fs_info
structure of the filesystem in zstd_alloc_workspace_manager(), so pass it
to the btrfs_warn() call so that we get a better log message (including
the device, state, etc).

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
[BACKGROUND]
In the very early days, commit 325cd4b ("Btrfs: properly set
blocksize when adding new device.") set the bdev blocksize as at that
time btrfs is using __bread() for super block IO, which went through
buffer heads thus requires the strict block size limit.

Then btrfs super block IO moved away from buffered heads, by commit
8f32380 ("btrfs: use the page cache for super block reading") and
commit 314b6dd ("btrfs: use bios instead of buffer_heads from super
block writeout").

So after that there is already no strict blocksize requirement for
btrfs, except the bdev's block size should not exceed 4K, which is also
the super block size of btrfs.

Btrfs data read/writes all follow sectorsize, which can be larger than
4K.
Btrfs metadata read/writes all follow nodesize, which is already 16K by
default.

Furthermore, there is already a bug report that set_blocksize() can
trigger a lockdep warning from btrfs, as set_blocksize() will acquire
inode lock, and under certain races can lead to lockdep warnings.

[ENHANCEMENT]
Remove the set_blocksize() calls, instead call bdev_validate_blocksize()
inside btrfs_open_device_deny_freeze() and btrfs_get_bdev_and_sb().

Btrfs doesn't use buffer heads, thus no strict set_blocksize()
requirement. But we still want to call bdev_validate_blocksize(), just in
case where a future block device with larger blocksize (e.g. 16K) shows
up.

For such devices btrfs super block bios will be rejected, so we want to call
bdev_validate_blocksize() early to reject such devices.

Furthermore, the bdev_validate_blocksize() itself is lockless, thus it
should resolve the lockdep report from syzbot.

Reported-by: syzbot+5c01398781fb51df783a@syzkaller.appspotmail.com
Link: https://lore.kernel.org/linux-btrfs/6adcb3be.4ce6c07b.2dfc0c.0005.GAE@google.com/
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.