Skip to content

Test for-next ARM64 64K (regular, SELF) - #1630

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

Test for-next ARM64 64K (regular, SELF)#1630
kdave wants to merge 10000 commits into
ci-arm-kvmfrom
for-next

Conversation

@kdave

@kdave kdave commented Apr 17, 2026

Copy link
Copy Markdown
Member

No description provided.

@adam900710
adam900710 force-pushed the for-next branch 2 times, most recently from ad252c6 to af81080 Compare April 18, 2026 04:42
@kdave
kdave force-pushed the for-next branch 2 times, most recently from 30c6cb0 to 73d4bbd Compare April 22, 2026 19:47
@kdave
kdave force-pushed the for-next branch 2 times, most recently from 26f5cfa to 2189fe7 Compare April 24, 2026 11:09
@kdave
kdave force-pushed the for-next branch 2 times, most recently from 5280eae to 52d1b61 Compare April 27, 2026 14:33
@adam900710
adam900710 force-pushed the for-next branch 3 times, most recently from 40c2283 to 09752d4 Compare April 28, 2026 00:45
@kdave
kdave force-pushed the for-next branch 2 times, most recently from 29451dd to dc188da Compare April 28, 2026 06:01
@adam900710
adam900710 force-pushed the for-next branch 2 times, most recently from 4a55cf6 to 436ac81 Compare May 3, 2026 08:53
@fdmanana
fdmanana force-pushed the for-next branch 2 times, most recently from e32c6db to 49a0b34 Compare May 4, 2026 15:50
@kdave
kdave force-pushed the for-next branch 4 times, most recently from 4137f02 to f2ac86e Compare May 12, 2026 15:03
@kdave
kdave force-pushed the for-next branch 2 times, most recently from db2485b to 0c78978 Compare May 16, 2026 00:59
fdmanana and others added 30 commits August 7, 2026 19:17
We are using atomic types for the log_commit array of struct btrfs_root
but all we need is simple booleans. The log_commit array elements are
always protected by the root's log_mutex, both for writes and reads, so
we can use a simple boolean. The use of atomics if from the very early
days of the log tree code where the access to the fields was not protected
by any lock.

So switch to simple booleans, which results in cheaper code and slightly
reduces the object size too.

Reviewed-by: Boris Burkov <boris@bur.io>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Inside reflink.c we still have a lot of functions passing VFS inode
pointers, then internally convert them into btrfs_inode pointers.

For example, inside btrfs_clone(), we have 12 BTRFS_I() call sites,
while only 3 callsites that really require a VFS inode pointer.

Do the cleanup to convert the following functions to pass a btrfs_inode
pointer instead of a vanilla inode pointer:

- btrfs_clone()
- btrfs_extent_same_range()
- clone_finish_inode_update().

Which covers all ad-hoc BTRFS_I() call sites inside reflink.c.

Reviewed-by: Daniel Vacek <neelx@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>
… start

btree_writepages() writes the btree inode's dirty metadata in ascending
logical address order. On a zoned filesystem only one metadata and one
system block group is active for writing at a time, and
check_bg_is_active() (via btrfs_check_meta_write_pointer()) pivots the
active block group as writeback moves from one block group to the next.

If the active block group sits at a higher logical address than another
block group that also holds dirty metadata, the ascending walk reaches
the lower one first and, to write it, has to finish the active block
group and activate the lower one. It cannot finish a block group that
still has unsent IO, and during WB_SYNC_ALL && !for_sync (commit)
writeback it deliberately refuses to wait for that IO under
fs_info->zoned_meta_io_lock, as that can deadlock. The pivot thus cannot
issue the submission itself either, so it gives up:
btrfs_check_meta_write_pointer() returns -EAGAIN, which
btrfs_write_and_wait_transaction() treats as fatal and aborts the
transaction, forcing the filesystem read-only. This happens
intermittently under metadata-heavy relocation (e.g. fstests btrfs/187).

Flush the active metadata and system block groups at the start of
btree_writepages(), under the fs_info->zoned_meta_io_lock it already
holds, so they have no unsent IO left and the later pivot can finish
them and make forward progress.

Fixes: 13bb483 ("btrfs: zoned: activate metadata block group on write time")
Assisted-by: LLM (debugging, commit message)
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
On a zoned filesystem a freed tree block is not cleared but kept dirty
and flagged EXTENT_BUFFER_ZONED_ZEROOUT, so a later writeback zeroes it
out and advances the zone write pointer. A transaction abort turns the
filesystem read-only before that writeback runs, so these buffers stay
dirty and stranded ahead of the write pointer where btree_writepages()
can no longer write them. They survive to the final iput() of the btree
inode at unmount, which submits the write after the endio workqueues are
gone, hanging unmount in folio_wait_writeback().

Clear the dirty state of such buffers when cleaning up the aborted
transaction, where the buffer tree still references all of them.

Assisted-by: LLM (debugging, commit message)
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
On a zoned filesystem a freed tree block is kept dirty and flagged
EXTENT_BUFFER_ZONED_ZEROOUT so a later writeback zeroes it out and
advances the zone write pointer. Unsynced tree-log updates (e.g. from
rename or link) leave such buffers behind when the log is freed at
commit, and across log generations they can end up ahead of the write
pointer behind a hole, so btree_writepages() can never write them.
During normal operation the space is later reclaimed by a zone reset; at
unmount it is not, and the buffers survive to the final iput() of the
btree inode, which hangs in folio_wait_writeback() once the endio
workqueues are stopped.

They cannot be written back from where they are freed (free_log_tree(),
inside the committing transaction) without deadlocking against that
commit, and they are stale anyway, not referenced by the committed
superblock. Drop their dirty state in close_ctree(), before
btrfs_stop_all_workers().

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
On a zoned filesystem a freed-but-still-dirty tree block is written out
as zeros (EXTENT_BUFFER_ZONED_ZEROOUT) only to keep the zone write
pointer advancing. btree_csum_one_bio() implemented this by memzeroing
the extent buffer's own folios before submission.

That destroys the in-memory buffer while it may still be referenced. In
particular btrfs_free_tree_block() can run on it afterwards and reads
the header to add a delayed reference; once the header has been zeroed
it frees bytenr 0 and corrupts the extent tree (the
btrfs_header_bytenr(buf) != 0 ASSERT in btrfs_free_tree_block(), or an
"unable to find ref" abort). It is flaky and reproduces under fsstress,
e.g. generic/461 and generic/013.

Write the zeros to disk from the shared zero page instead and leave the
extent buffer content untouched, so any later reference - including the
delayed reference from btrfs_free_tree_block() - still sees a valid
header. end_bbio_meta_write() now clears writeback on the buffer's own
folios, as the bio no longer carries them.

Fixes: aa6313e ("btrfs: zoned: don't clear dirty flag of extent buffer")
Assisted-by: LLM (debugging, commit message)
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The @EnD parameter for all extent io tree helpers is inclusive, but
the call site in extent_fiemap() is passing an exclusive end into
btrfs_lock_extent(), which will step into the next block unexpectedly.

Pass the inclusive end into btrfs_lock_extent() and
btrfs_unlock_extent().

Fixes: ac3c0d3 ("btrfs: make fiemap more efficient and accurate reporting extent sharedness")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
In btrfs_extent_same_range() and btrfs_clone_files(), the range passed
into btrfs_lock_extent() is not aligned at its end, because we can
reflink until the EOF, which may not be block aligned.

Although this is not a big deal, for the sake of consistency, and to
prepare for the upcoming stricter alignment check, pass an aligned range
end to btrfs_lock_extent() and btrfs_unlock_extent().

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Extent maps have the extra validation since commit 3f255ec ("btrfs:
introduce extra sanity checks for extent maps"), but extent states do
not have a similar check.

Introduce a basic alignment check for the following call sites, so that
we can cover all extent states inserted into the tree:

- insert_state_fast()
- insert_state()
- split_state()

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
…_data()

In that function, we round down the start position and round up the
ending position.

But during the calculation of @len, we use "round_up(start + len,
sectorsize)", which is the rounded up end position, not the rounded up
length.

Which results a much larger length, and later we are still using
"start + len", which is completely incorrect.

Fix it by declaring a local @aligned_start and @aligned_len and use them
instead.

Fixes: bc42bda ("btrfs: qgroup: Fix qgroup reserved space underflow by only freeing reserved ranges")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
In preparation for preallocating extent_buffer data, factor eb
initialization away from specifically allocating it. This allows us to
allocate the eb, bfs, folios, etc. together in the main search_slot code
paths, but still share initialization code with the dummy/test/clone
allocation paths.

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Boris Burkov <boris@bur.io>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
In further preparation for supporting NOFAIL allocations with retries
outside the critical section, add a struct to carry the extent_buffer
and btrfs_folio_state we need to allocate.

Refactor the allocation pathways to use the new struct but with no
functional change. Wire empty prealloc structs in from callers.

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Boris Burkov <boris@bur.io>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Now that we have the btrfs_eb_prealloc struct to carry the allocation
and the "needs prealloc" signal, wire that up between the various
search_slot style callers down into alloc_extent_buffer.

If the prealloc struct indicates that it supports a nowait try, then
alloc_extent_buffer tries to allocate NOWAIT. If that succeeds, great.
Otherwise, we return EAGAIN and signal via the struct that preallocation
is required. The caller then does the allocation and tries again with
the eb, bfs, and folios wired through in the prealloc struct.

If unlock-and-allocate retries are not supported then we just use the
normal gfp flags like before.

Note that there are still two GFP_NOFS allocations, as far as I know,
that happen under the lock and cannot be preallocated:
  - the __xa_cmpxchg to insert the eb into the eb xarray
  - the xarray allocations for filemap_add_folio to add the folios to
    the btree_inode mapping.

The former we could wire up with xa_reserve if we signaled the "prealloc
start" back up to the retry point. However, since there is no concept of
reservation in the filemap xarray, it seemed relatively unhelpful to
bother. These allocations are relatively small cached slab allocations,
so hopefully we can move the needle on reclaim stalls without reserving
them.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Boris Burkov <boris@bur.io>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
extent_buffer readahead should not be able to painfully stall a
search_slot and hog tree locks by getting stuck in direct reclaim.

If the allocation fails, that is fine, we simply fail to do the
readahead in that case.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Boris Burkov <boris@bur.io>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Inspired by the previous crash exposed by generic/795, we want to make
sure every folio from btrfs page cache is properly aligned to block
size.

This is especially important for bs > ps support, as every btrfs
infrastructure, e.g. extent map and extent state, requires strong block
alignment checks.

Furthermore, also output the minimal folio order from the inode mapping,
which is the determining factor during debugging, helping a lot pinning
down the final cause.

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
If we the no-holes feature is enabled (a default since btrfs-progs 5.15),
when doing a full fsync we always iterate of all leaves in the subvolume
root that contain file extent items in order to detect holes between them.

This can take a lot of time for files with a large number of extents.
But if we know there are no prealloc extents and the amount of space
(uncompressed space) is greater than or equals to the i_size of the
inode, then we cannot have holes and therefore avoid searching for
them. So skip the search if those conditions are met.

The following test script was used:

  $ cat test.sh
  #!/bin/bash

  MNT=/mnt/nullb0
  DEV=/dev/nullb0

  umount $MNT &> /dev/null
  mkfs.btrfs -f $DEV
  mount $DEV $MNT

  # 256M gives 64K extents of 4K each.
  FILE_SIZE=$((256 * 1024 * 1024))
  touch $MNT/foobar

  for ((i = 0; i < $FILE_SIZE; i += 8192)); do
      xfs_io -c "pwrite -S 0xab $i 4K" $MNT/foobar > /dev/null
  done

  xfs_io -c "fsync" $MNT/foobar

  for ((i = 4096; i < $FILE_SIZE; i += 8192)); do
     xfs_io -c "pwrite -S 0xab $i 4K" $MNT/foobar > /dev/null
  done

  # unmount and mount, clear caches and ensure the next fsync is a
  # full sync.
  umount $MNT
  mount $DEV $MNT

  # Do some change to the file in order to fsync.
  xfs_io -c "pwrite -S 0xcd 0 4K" $MNT/foobar > /dev/null

  T0=$(date +%s%N)
  xfs_io -c "fsync" $MNT/foobar
  T1=$(date +%s%N)

  echo
  echo "Took $(( (T1 - T0) / 1000 ))us"

  umount $MNT

Before this change:

  Took 28721us

After this change:

  Took 5453us

That's about 5.3x times faster.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
btrfs_init_dev_replace_tgtdev() allocates the replacement target
before looking up its dev_t and initializing its zoned device
information. If either lookup_bdev() or btrfs_get_dev_zone_info()
fails, the device has not been linked into fs_devices->devices yet,
but the error path only drops the block device file reference.

Free the allocated device on this error path to release its name,
allocation state, zone info, and the device itself.

The issue was found by a failure-path metadata residual analyzer and
verified with targeted failure injection on v6.14.

Assisted-by: Codex:gpt-5
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Guanghui Yang <3497809730@qq.com>
btrfs_dev_replace_start() adds the replacement target to the device
list before marking block groups to copy. If marking fails, returning
directly leaves the target linked and keeps the device accounting
incremented.

Jump to the existing cleanup path so the target device is removed and
released on failure.

The issue was found by a failure-path metadata residual analyzer and
verified with targeted failure injection on v6.14.

Assisted-by: Codex:gpt-5
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Guanghui Yang <3497809730@qq.com>
When creating the first metadata chunk for a sprout filesystem,
create_chunk() adds the new device to the transaction dev_update_list
through device->post_commit_list.

If the subsequent system chunk creation fails, btrfs_init_new_device()
aborts the transaction and releases the device while post_commit_list is
still linked. This triggers a warning in btrfs_free_device() and leaves
the transaction list referencing freed memory.

Detach the device while holding chunk_mutex before releasing it.

Fixes: bbbf724 ("btrfs: combine device update operations during transaction commit")
Assisted-by: Codex:gpt-5
Cc: stable@vger.kernel.org
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Guanghui Yang <3497809730@qq.com>
[ Fix too long lines in the commit message. ]
Signed-off-by: Qu Wenruo <wqu@suse.com>
btrfs_init_new_device() switches latest_dev and possibly s_bdev from the
seed device to the new sprout device before creating the first writable
chunks.

If chunk creation or the subsequent sprout setup fails, the error path
releases the new device without switching those pointers back.
btrfs_show_devname() can then dereference the freed latest_dev and crash.

Restore the active device pointers to the latest seed device before
removing and releasing the failed sprout device.

Fixes: b7cb29e ("btrfs: update latest_dev when we create a sprout device")
Assisted-by: Codex:gpt-5
Cc: stable@vger.kernel.org
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Guanghui Yang <3497809730@qq.com>
[ Fix too long lines in the commit message. ]
Signed-off-by: Qu Wenruo <wqu@suse.com>
btrfs_init_new_device() calls btrfs_setup_sprout() before creating the
first writable chunks for a seed filesystem. That moves the seed devices
out of fs_info->fs_devices, clears the seeding state and installs a new
fsid for the sprout filesystem.

If a later step fails, the error path removes the new device but leaves
fs_info->fs_devices in the partially initialized sprout state.
The mounted filesystem can then be left with no open devices after the
failed device add.

Add the inverse of btrfs_setup_sprout() and use it from the error path
so the mounted seed filesystem is restored before the temporary
seed_devices copy is released.

Fixes: 2b82032 ("Btrfs: Seed device support")
Assisted-by: Codex:gpt-5
Cc: stable@vger.kernel.org
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Guanghui Yang <3497809730@qq.com>
[ Fix a conflict with per-profile available space, revert sprout before
  updating per-profile available space estimation, fix too long lines in
  the commit message ]
Signed-off-by: Qu Wenruo <wqu@suse.com>
btrfs_insert_orphan_item() allocated a btrfs_path with btrfs_alloc_path()
which returns -ENOMEM on failure. It is called from btrfs_orphan_add(),
so a path allocation failure there turns a recoverable error into a
transaction abort.

btrfs_path is only ~112 bytes, so allocate it on the stack instead.

Assisted-by: LLM
Suggested-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
btrfs_del_orphan_item() allocated a btrfs_path with btrfs_alloc_path()
which returns -ENOMEM on failure. It is called from btrfs_orphan_del(),
and btrfs_link() turns any error from it into a transaction abort. So a
path allocation failure there (reachable via linkat() on an O_TMPFILE
under memory pressure) turns a recoverable error into an abort.

btrfs_path is only ~112 bytes, so allocate it on the stack instead.
Unlike the insert case there are multiple exit points after the search,
so release the path via a common out: label.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Qu Wenruo <wqu@suse.com>
…t phases

Split btrfs_insert_delayed_dir_index() into three functions using a new
btrfs_dir_index_prealloc struct to bundle the pre-allocated resources:

- btrfs_prealloc_delayed_dir_index(): allocates the struct and performs
  the two GFP_NOFS allocations (delayed node + delayed item) that can
  fail with -ENOMEM. Returns the struct, or ERR_PTR on failure.
- btrfs_insert_delayed_dir_index_prealloc(): populates the item data,
  inserts into the rb-tree, and reserves metadata space. Cannot fail
  with -ENOMEM since all allocations were done in the prealloc step.
- btrfs_free_delayed_dir_index_prealloc(): frees pre-allocated
  resources when the caller's btree insertion fails. Tolerates NULL.

The prealloc is returned as a pointer rather than filled into a
caller-provided struct, so that a plain NULL means "no prealloc" and
callers do not need a separate flag to track whether one exists. It is
consumed (and freed) by either the commit or the free helper, so
ownership is unambiguous.

The original btrfs_insert_delayed_dir_index() is refactored into a thin
wrapper that calls the prealloc and commit functions.

This split allows callers to move the fallible memory allocations before
the point of no return (the DIR_ITEM btree insertion), so that -ENOMEM
can be returned cleanly without aborting the transaction.

Assisted-by: LLM
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Move the delayed dir index allocation in btrfs_insert_dir_item() before
the insert_with_overflow() call that modifies the btree. Previously, the
allocations happened after the DIR_ITEM was already inserted, meaning an
ENOMEM failure left the btree in a partially-modified state that could
only be resolved by aborting the transaction.

Add an optional caller-provided btrfs_dir_index_prealloc parameter to
btrfs_insert_dir_item(). When non-NULL, ownership of the prealloc
transfers to btrfs_insert_dir_item(). When NULL, it allocates internally.
All existing callers pass NULL to preserve the current behavior.

Since ownership transfers, btrfs_insert_dir_item() must free the prealloc
on every path that does not commit it. Route all such exits (including
the early path allocation failure) through a common out_free_prealloc
label, rather than keying cleanup on need_delayed_index.

Remove the btrfs_insert_delayed_dir_index() wrapper, as there are no
more callers.

Assisted-by: LLM
Suggested-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Now that btrfs_insert_dir_item() returns -ENOMEM before modifying the
btree (thanks to delayed dir index pre-allocation), callers can handle
ENOMEM gracefully instead of aborting the transaction.

- btrfs_add_link(): add -ENOMEM to the recoverable errors alongside
  -EEXIST and -EOVERFLOW.
- btrfs_create_new_inode(): on -ENOMEM from btrfs_add_link(), orphan the
  newly-created inode instead of aborting. The inode item was already
  written with nlink 1, and discard_new_inode() marks it bad so eviction
  won't delete it. So clear_nlink() alone is not enough: persist nlink 0
  via btrfs_update_inode(), otherwise orphan cleanup would see nlink > 0,
  drop the orphan item, and leak the inode. Fall back to aborting only if
  that update also fails.

This turns a filesystem-killing abort into a graceful -ENOMEM return for
create(), mkdir(), mknod(), symlink(), and link() under memory pressure.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Qu Wenruo <wqu@suse.com>
For rename() without an overwrite target, pre-allocate the delayed
dir index before any btree modifications so that ENOMEM can be returned
before the source is unlinked from the old directory.

Add a prealloc parameter to btrfs_add_link() that allows callers to
pass pre-allocated delayed dir index resources. When provided,
btrfs_add_link() takes ownership: it either passes the prealloc to
btrfs_insert_dir_item() (which commits or frees it), or frees it
on early error. All existing callers pass NULL to preserve the current
behavior.

In btrfs_rename(), when new_inode is NULL (no overwrite), call
btrfs_prealloc_delayed_dir_index() before the first btree modification
and pass the result through to btrfs_add_link(). If the prealloc fails,
-ENOMEM is returned before any btree state has changed. The local
prealloc pointer is cleared once ownership passes to btrfs_add_link(),
so the out_fail path only frees one we still own.

For overwrite rename (new_inode != NULL), the transaction still aborts
on ENOMEM since earlier unlink operations have already made irreversible
btree modifications.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Qu Wenruo <wqu@suse.com>
In the function btrfs_dev_replace_start(), we have several error paths
that assigns replace_task without reverting it back to NULL.

There are two involved error paths:

- There is already a running dev-replace
  Then replace_task is over-written to the current task.
  This is the one with long running effect.

- The btrfs_start_transaction() call failed
  This is much harder to hit though.

This can result the replace_task check inside btrfs_map_block() to be
incorrectly triggered, not taking dev_replace->rwsem, and may get an
incorrect/stale view on replace related structures.

Thankfully this bug is very hard to hit.

As dev-replace is an exclusive operation, thus if there is already
a running replace, a new one will be rejected early without reaching
btrfs_dev_replace_start().

The only remaining case is a suspended replace, which is much harder to
hit, e.g. requiring async dev-replace conflicting with another exclusive
operation, then a new replace is started.

Fix the problem by:

- Moving the replace_task assignment after replace_state check in
  btrfs_dev_replace_start()

- Reset replace_task to NULL if btrfs_start_transaction() failed
  in btrfs_dev_replace_start()

- Reset replace_task to NULL for all paths of
  btrfs_dev_replace_finishing()

This is reported by Sashiko, which found the existing bug during review
of another patch, and since the bug is an existing one, it's not shown in
the summary, but only in the detail page.

Link: https://sashiko.dev/#/patchset/tencent_853134544C3CE88A219EEB21346E2510D308%40qq.com
Fixes: 8cca35c ("btrfs: don't take dev_replace rwsem on task already holding it")
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Setting the compression property on an inode also sets BTRFS_INODE_COMPRESS
on it, and btrfs_inode_flags_to_fsflags() reports that back as FS_COMPR_FL
to FS_IOC_GETFLAGS. chattr(1), like any other FS_IOC_SETFLAGS caller, reads
the current flags, flips only the bit the user asked for and writes the
whole set back, so a request as unrelated as "chattr +i" reaches
btrfs_fileattr_set() with FS_COMPR_FL set.

btrfs_fileattr_set() takes that as a request to enable compression and
overwrites the compression property with the algorithm from the mount
options, falling back to zlib when the filesystem was not mounted with
-o compress. The algorithm the user selected is silently replaced:

  # btrfs property set /mnt/foo compression zstd
  # btrfs property get /mnt/foo compression
  compression=zstd
  # chattr +i /mnt/foo
  # btrfs property get /mnt/foo compression
  compression=zlib

Every chattr operation triggers this, not just +i, and directories are
affected as well, so files created afterwards inherit the wrong algorithm
too. On a filesystem mounted with -o compress=lzo the property is replaced
with lzo instead. Recovering needs a chattr -i first, because the immutable
flag rejects the setxattr that "btrfs property set" issues.

Prefer the algorithm recorded in the compression property and only fall
back to the mount default when there is no property, so that unrelated
flag changes no longer overwrite the user's choice. Inodes that have the
compress flag set but no property still get the default, so they behave
as before.

Signed-off-by: Sam Ho <samho@synology.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
scrub_stripe_report_errors() iterates over the sectors of a stripe, but
every message it emits passes stripe->logical, the address of the first
sector of the 64KiB stripe, rather than the address of the sector being
reported. The physical address is likewise computed once, before the
loop, from stripe->logical.

This matters because scrub_print_common_warning() uses that logical
address for the backref walk which produces the "root %llu inode %llu
offset %llu ... (path: ...)" part of the message. As the address is
always the stripe base, the reported root/inode/offset/path can identify
a different file from the one whose sector actually failed.

A 64KiB stripe routinely spans several extents belonging to unrelated
files. On the machine where this was found, the stripe at logical
0x17D9380000 holds four sectors of /usr/share/plasma/emoji/bg.dict, then
a file inside a docker volume, then sectors referenced only by
snapshots. Every error anywhere in that stripe is attributed to bg.dict.

The effect is visible statistically: across ten months and four kernel
series that machine logged 81 distinct flagged logical addresses, and
every one of them is exactly 64KiB aligned. Since BTRFS_STRIPE_LEN is
64KiB and stripe->logical is stripe aligned by construction, real
failures distributed across sectors could not produce that.

Report the address of the sector actually being examined. Adding the
sector offset to the physical address is valid because BTRFS_STRIPE_LEN
is the unit contiguous on a single device for every profile, so a stripe
never crosses a device boundary.

Signed-off-by: James C. Owens <jamesowens@optonline.net>
Fixes: 0096580 ("btrfs: scrub: introduce error reporting  functionality for scrub_stripe")
Reviewed-by: Qu Wenruo <wqu@suse.com>
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.