| Age | Commit message (Collapse) | Author |
|---|
|
|
|
https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git
|
|
https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs
Pull vfs fixes from Christian Brauner:
- vfs: Preserve the ACL_DONT_CACHE state in forget_cached_acl().
ACL_DONT_CACHE is meant to be a permanent opt-out from ACL caching
which FUSE relies on for servers that don't negotiate FUSE_POSIX_ACL.
The helper replaced it with ACL_NOT_CACHED, silently re-enabling the
cache, and as fuse doesn't invalidate the cache for such servers a
properly timed get_acl() returned stale ACLs. Comes with a fuse
selftest reproducing this.
- pidfs:
- Preserve PIDFD_THREAD when a thread pidfd is reopened via
open_by_handle_at(). PIDFD_THREAD shares the O_EXCL bit which
do_dentry_open() strips after the flags have been validated, so
the reopened pidfd silently became a process pidfd. Comes with a
selftest.
- Add a pidfs_dentry_open() helper so the regular pidfd allocation
path and the file handle path share the code that forces O_RDWR
and reapplies the pidfd flags that do_dentry_open() strips.
- Handle FS_IOC32_GETVERSION in the compat ioctl path.
- Make pidfs_ino_lock static.
- iomap:
- Fix the block range calculation in ifs_clear_range_dirty() so a
partial clear doesn't drop the dirty state of blocks the range
only partially covers.
- Support invalidating partial folios so a partial truncate or hole
punch with blocksize < foliosize doesn't leave stale dirty bits
behind.
- Only set did_zero when iomap_zero_iter() actually zeroed
something.
- Guard ifs_set_range_dirty() and ifs_set_range_uptodate() against
zero-length ranges where the unsigned last-block calculation
underflows and bitmap_set() writes far beyond the ifs->state
allocation.
- Don't merge ioends with different io_private values as the merge
could leak or corrupt the private data of the individual ioends.
- exec:
- Raise bprm->have_execfd only once the binfmt_misc interpreter has
actually been opened. The flag was set as soon as a matching 'O'
or 'C' entry was found. If the interpreter open failed with
ENOEXEC the exec fell through to the next binary format with
have_execfd raised but no executable staged and begin_new_exec()
NULL derefed past the point of no return.
- Fix an unsigned loop counter wrap in transfer_args_to_stack() on
nommu. An overlong argument or environment string pushes bprm->p
below PAGE_SIZE, the stop index becomes zero, and the loop never
terminates, wrapping its counter and copying garbage from in
front of the page array into the new process stack.
- Make binfmt_elf_fdpic only honour the first PT_INTERP like
binfmt_elf does. Each additional PT_INTERP overwrote the previous
interpreter, leaking the name allocation and the interpreter file
reference together with the write denial open_exec() took,
leaving the file unwritable for as long as the system runs.
- overlayfs:
- Compare the full escaped xattr prefix including the trailing dot.
An xattr like "trusted.overlay.overlayfoo" was misclassified as
an escaped overlay xattr.
- Check read access to the copy_file_range() source with the
source's mounter credentials.
- super: Thawing a filesystem whose block device was frozen with
bdev_freeze() deadlocked. Dropping the last block layer freeze
reference from under s_umount ends up in fs_bdev_thaw() which
reacquires s_umount on the same task. Pin the superblock with an
active reference instead and call bdev_thaw() without holding
s_umount.
- procfs: Return EACCES instead of success when the ptrace access check
for namespace links fails.
- afs: Use afs_dir_get_block() rather than afs_dir_find_block() for
block 0 in afs_edit_dir_remove(), matching afs_edit_dir_add().
- Push the memcg gating of ->nr_cached_objects() down into the btrfs
and shmem callbacks instead of skipping every callback during
non-root memcg reclaim. The blanket check short-circuited XFS whose
inode reclaim hook is intentionally driven from per-memcg contexts to
free memcg-charged slab.
- eventpoll: Pin files while checking reverse paths.
Since struct file became SLAB_TYPESAFE_BY_RCU a concurrent close
could free and recycle the file under the check which then took and
dropped the f_lock of whatever live file now occupies that slot.
* tag 'vfs-7.2-rc5.fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs: (24 commits)
super: fix emergency thaw deadlock on frozen block devices
pidfs: make pidfs_ino_lock static
eventpoll: pin files while checking reverse paths
fs: push nr_cached_objects memcg gating into individual filesystems
afs: Fix afs_edit_dir_remove() to get, not find, block 0
iomap: prevent ioend merge when io_private differs
iomap: add comments for ifs_clear/set_range_dirty()
iomap: fix out-of-bounds bitmap_set() with zero-length range
iomap: fix incorrect did_zero setting in iomap_zero_iter()
iomap: support invalidating partial folios
iomap: correct the range of a partial dirty clear
fs/super: fix emergency thaw double-unlock of s_umount
pidfs: handle FS_IOC32_GETVERSION in compat ioctl
ovl: check access to copy_file_range source with src mounter creds
proc: Fix broken error paths for namespace links
pidfs: add pidfs_dentry_open() helper
selftests/pidfd: check PIDFD_THREAD survives open_by_handle_at()
pidfs: preserve thread pidfds reopened by file handle
ovl: fix trusted xattr escape prefix matching
selftests/fuse: add ACL_DONT_CACHE regression test
...
|
|
Commit 0baad6f9b997 ("fs/super: skip non-memcg-aware nr_cached_objects
in memcg slab shrink") added a check in fs/super.c that skipped every
->nr_cached_objects() hook whenever the shrinker was invoked for a
non-root memcg, on the assumption that none of them honour sc->memcg.
That assumption is wrong for XFS, whose inode-reclaim hook is
intentionally driven from per-memcg contexts to free memcg-charged
slab. Encoding a blanket "never memcg-aware" policy in fs/super.c
short-circuits that path.
Push the check down into the callbacks whose counters really are
irrelevant to per-memcg reclaim - btrfs_nr_cached_objects() and
shmem_unused_huge_count() - and drop the fs/super.c gate. Each
filesystem can now lift the restriction independently if its counter
later grows memcg awareness, without touching fs/super.c.
Introduce mem_cgroup_shrink_is_root() in <linux/memcontrol.h> so the
callbacks don't open-code "sc->memcg is NULL or root".
Fixes: 0baad6f9b997 ("fs/super: skip non-memcg-aware nr_cached_objects in memcg slab shrink")
Acked-by: Qi Zheng <qi.zheng@linux.dev>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
Signed-off-by: Usama Arif <usama.arif@linux.dev>
Link: https://patch.msgid.link/20260715103516.2410175-1-usama.arif@linux.dev
Acked-by: David Sterba <dsterba@suse.com>
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
# Conflicts:
# fs/btrfs/raid-stripe-tree.c
|
|
|
|
|
|
btrfs_record_root_in_trans() has a lockless fast path for shareable
roots. It skips reloc_mutex when root->last_trans matches the current
transaction and BTRFS_ROOT_IN_TRANS_SETUP is clear.
The writer side publishes that state in two phases: it sets
IN_TRANS_SETUP before updating root->last_trans, then clears the bit
after btrfs_init_reloc_root() finishes. However, the reader-side
smp_rmb() is before both loads, so it does not order the last_trans load
against the later bit test. A reader can observe the new last_trans value
while missing the setup bit and return before the relocation-root setup
is complete.
Read root->last_trans first, then issue the read barrier before testing
IN_TRANS_SETUP. Also use clear_bit_unlock() for the writer's final clear
and test_bit_acquire() for the successful fast path, so the lockless
return observes the setup done before the bit was cleared.
Fixes: 7585717f304f ("Btrfs: fix relocation races")
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Any commits after this one are for testing and evaluation only.
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
That macro is only utilized 4 times, all inside file.c, while we have
tons of open-coded usages. And since it's a macro, there is no proper
type checks at all.
There isn't much need for such a rarely utilized macro.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Commit f9a48549a15a ("btrfs: inhibit extent buffer writeback to prevent
COW amplification") tracks the extent buffers a transaction handle has
inhibited in a per-handle xarray. Keying the tracking to the transaction
handle is correct, but using an xarray for it causes two problems in
production.
First, a write_iops regression. Every COW calls
btrfs_inhibit_eb_writeback() from btrfs_force_cow_block() and
should_cow_block(), which does an xa_store() keyed by eb->start. The
kernel test robot reported a 22.6% fio.write_iops regression on a
single-task 4k randwrite workload (ftruncate ioengine, buffered IO) on
btrfs. The cost is the per-COW xarray store done on every COW'd block.
Replacing it with a non-allocating fixed buffer recovers the lost
throughput, and that buffer does more per-COW bookkeeping yet still
recovers, so the cost is the xarray operation itself rather than the
extra tracking work.
Second, an unbounded cleanup walk. btrfs_uninhibit_all_eb_writeback()
iterates every eb the handle inhibited with xa_for_each(). A single
handle that COWs a very large number of blocks (inode eviction, or
truncate of a file with many extents, where btrfs_truncate_inode_items()
loops over many search_again descents under one handle) makes that walk
arbitrarily long. It runs in __btrfs_end_transaction() before
num_writers is dropped, so it blocks the committing thread; this shows up
as multi-second stalls and RCU stall reports.
Replace the xarray with a fixed inline array on btrfs_trans_handle,
managed with a CLOCK (second-chance) eviction policy. Inhibiting a buffer
becomes an array append with no allocation and no tree walk, and the
end-of-handle cleanup is bounded by the array size.
The set that actually needs protection is the working set the handle
revisits across search_again descents, the search path frontier, which is
on the order of the tree height. It is not every block the handle ever
COWs. should_cow_block() re-inhibiting an already tracked buffer marks it
referenced, so revisited buffers survive eviction while write-once buffers
are reclaimed first. A small fixed buffer is therefore enough where a
non-evicting array would either overflow or have to grow without bound.
BTRFS_INHIBITED_EBS_SLOTS is 8 and the reference bits pack into a u32.
The CLOCK eviction is what justifies the extra complexity over a plain
non-evicting array. The test workload stresses amplification: it removes
16 heavily fragmented 64 MiB files in one transaction while background
writeback keeps writing out in-use metadata. A re-COW event is a buffer
already COWed in the running transaction that was written back and then
COWed again; the figure below is the ratio of re-COW events to first-COW
events summed across the eviction (n=5, lower is better):
tracking re-COW per first-COW
no inhibition 6.1
non-evicting array, 32 slots 3.8
CLOCK array, 8 slots (this patch) 1.6
unbounded xarray (reverted) 1.4
The non-evicting array fills with write-once buffers and stops covering
the buffers the handle keeps revisiting, so even at four times the slots
it leaves most of the amplification. CLOCK evicts the cold buffers and
keeps the revisited ones, recovering almost all of the unbounded benefit.
The eviction policy, not the buffer size, is what closes the gap.
eb->writeback_inhibitors and the WB_SYNC_ALL bypass in
lock_extent_buffer_for_io() are unchanged, so fsync and commit behavior
are unaffected. A reference is taken on each tracked buffer so it cannot
be freed while the array points at it; eviction drops that reference and
the inhibitor count.
There's another testing report, showing 20% latency improvement on
reflink and deduplication synthetic benchmark. Full detailed report at
https://github.com/lcf0399/linux-regression-evidence/tree/main/btrfs-remap-writeback-inhibition-v2 .
Link: https://lore.kernel.org/all/CANGjgd=fQkHht2PdDi-+EAdzWH7UtxxWhhJ7b80Rr17PbpgxOw@mail.gmail.com/
Reported-by: kernel test robot <oliver.sang@intel.com>
Fixes: f9a48549a15a ("btrfs: inhibit extent buffer writeback to prevent COW amplification")
Closes: https://lore.kernel.org/oe-lkp/202603112240.f7605968-lkp@intel.com
Tested-by: Chengfeng Lin <lin2530632123@gmail.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Sun YangKai <sunk67188@gmail.com>
Signed-off-by: Leo Martins <loemra.dev@gmail.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Avoid allocating the heuristic buckets separately from the workspace,
the lifetime is the same.
The new size of struct heuristic_ws is 2112. SLUB merges same/similar
sized structures for the named caches, so there's a chance such size
already exists on the system, like below:
$ grep 2112 /proc/slabinfo
sighand_cache 593 1335 2112 15 8
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
For a filesystem which has btrfs read-only property set to true, all
write operations including acl and xattr should be denied. However, acl
can still be set even if btrfs ro property is true.
This happens because no function on the set_acl code path checks the root
is readonly or not. It was checked in btrfs_setxattr_trans() but got
removed in commit 353c2ea735e4 ("btrfs: remove redundant readonly root
check in btrfs_setxattr_trans")
That commit didn't check if all the callers properly check the root's
read-only flag. A previous fix is commit b51111271b03 ("btrfs: check if
root is readonly while setting security xattr").
Always check if the root is read-only before performing the set acl
operation.
Fixes: 353c2ea735e4 ("btrfs: remove redundant readonly root check in btrfs_setxattr_trans")
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Sun YangKai <sunyangkai@fnnas.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
During relocation recovery, each fs root gets a reference to its relocation
root. If loading or adding a later root fails, or if the first transaction
commit fails, btrfs_recover_relocation() jumps to out_unset before
merge_reloc_roots() and clean_dirty_subvols().
put_reloc_control() drops the list-owned relocation root references, but it
does not clear fs_root->reloc_root or drop the references owned by those
pointers. Mount cleanup only drops them when BTRFS_FS_ERROR is set, so an
error such as -ENOMEM while processing a later root can leave references
behind.
Keep temporary references to the fs roots associated during recovery. On
failure, clear their reloc_root pointers and drop the corresponding
references. Once the first transaction commit succeeds, drop only the
temporary fs root references and let the normal merge and cleanup paths
handle the relocation roots.
Fault injection on a pending-relocation image confirmed the cleanup gap.
With an injected first-commit failure, 25 fs roots had reloc_root set with
fs_error=0. With this fix, the same failure path drops that count to 0
before mount fails.
Fixes: f44deb7442ed ("btrfs: hold a ref on the root->reloc_root")
CC: stable@vger.kernel.org
Signed-off-by: Guanghui Yang <3497809730@qq.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Add sctx NULL check in the for loop condition of the sort_clone_roots
cleanup path for consistency with the else branch.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Commit 5387bd958180 ("btrfs: raid56: remove sector_ptr structure")
converted the bio-list membership checks from sector pointers to
physical addresses. The two conversions in rmw_assemble_write_bios()
kept their polarity (skip the sector when it is NOT in the bio list,
i.e. when there is nothing to write), but scrub_assemble_read_bios()
has the opposite polarity -- skip the sector when it IS in the bio
list, because then there is nothing to read -- and the conversion
flipped it:
- sector = sector_in_rbio(rbio, stripe, sectornr, 1);
- if (sector)
+ paddr = sector_paddr_in_rbio(rbio, stripe, sectornr, 1);
+ if (paddr == INVALID_PADDR)
continue;
Since a parity-scrub rbio's bio list only holds the empty completion
bio, the result is that scrub_assemble_read_bios() submits no reads at
all. finish_parity_scrub() then compares the parity it computes from
the (cached, correct) data stripes against whatever happens to be in
the freshly allocated, uninitialized stripe pages:
- if the garbage differs from the computed parity, the sector is
"repaired" and written back -- accidentally producing the correct
on-disk result;
- if a recycled page happens to still hold the old (correct) parity
content, the sector is deemed clean, dropped from dbitmap, and the
actually-corrupt on-disk parity is left in place. (Scrub reports
no errors either way: there is no counter for P/Q corruption by
design, so the bug here is purely the failure to read and repair.)
The second case is intermittent because it depends on page-allocator
recycling. Observed with fstests btrfs/297 (raid5, 2 devices): the
corrupted P stripe intermittently stays corrupt after a scrub --
roughly 1/10 runs on x86-64 KVM and up to 7/8 on a UML build whose
timing favors page reuse.
Since the bio-list check can never be true for a parity-scrub rbio --
raid56_parity_alloc_scrub_rbio() adds a single empty completion bio
(asserting bi_size == 0), bio_paddrs[] is only populated by
index_rbio_pages() which is never called for BTRFS_RBIO_PARITY_SCRUB,
and rbio_can_merge() refuses to merge rbios of different operations --
remove the dead check entirely and assert the invariant instead, as
suggested by Qu Wenruo.
After this fix the injected corruption is read, detected and repaired
in every run (8/8 UML, 10/10 KVM), and the new assertion never fires
across the full fstests raid group.
Fixes: 5387bd958180 ("btrfs: raid56: remove sector_ptr structure")
CC: stable@vger.kernel.org # 7.1+
Suggested-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mykola Lysenko <nickolay.lysenko@gmail.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Recently kernel RAID56 lib is trying to remove the unexpected
single-data-RAID56 (2 disks RAID5 or 3 disk RAID5) support, meanwhile
btrfs still supports such setup, which means in the long run btrfs has
to handle such corner case by ourselves.
Thankfully single-data-RAID56 is really RAID1/RAID1C3, since data and
P/Q stripes all match each other, rotation also makes no difference.
This patch will disguise those single-data-RAID56 chunks as
RAID1/RAID1C3 chunks.
This is done at two timings:
- Chunk read
- Chunk allocation
This is done by introducing btrfs_chunk_map::on_disk_type member, which
stores the type read from the on-disk metadata.
Meanwhile btrfs_chunk_map::type is calculated using on_disk_type.
For most profiles @type matches @on_disk_type, but for
single-data-RAID56, the @type will be RAID1/RAID1C3.
This method has a minimal impact on the fs, all other operations like
scrub and read-repair, are all based on the chunk map type, so the
disguise method will require no extra modification to those call sites.
Although there are still some locations that are checking against
block_group->flags, e.g. scrub. Those call sites will still get extra
limits assuming the bg is RAID56. But it should not cause any extra
problem.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
In the function fill_dummy_bgs(), bg->flags is assigned twice.
Just remove the second assignment.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Those two members are read from on-disk metadata, but never utilized.
And for new chunks we always set those members to BTRFS_STRIPE_LEN
anyway.
Thus there is no need to keep them inside btrfs_chunk_map.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
[FALSE ALERTS]
There is a bug report that the warning inside
invalidate_and_check_btree_folios() got triggered during btrfs/298:
BTRFS info (device sdd): first mount of filesystem f9bf732a-a19b-44b9-99a7-614ddff168e2
BTRFS info (device sdd): using crc32c checksum algorithm
BTRFS error (device sdd): failed to find fsid cb2fdb42-b638-4f2f-badd-4127467ba674 when attempting to open seed devices
BTRFS error (device sdd): failed to read chunk tree: -2
------------[ cut here ]------------
WARNING: disk-io.c:3342 at invalidate_and_check_btree_folios+0x260/0x3c0 [btrfs], CPU#4: mount/125993
CPU: 4 UID: 0 PID: 125993 Comm: mount Tainted: G W OE 7.1.0-rc7-custom+ #1 PREEMPT(full)
Hardware name: QEMU KVM Virtual Machine, BIOS edk2-20250812-19.fc42 08/12/2025
Call trace:
invalidate_and_check_btree_folios+0x260/0x3c0 [btrfs] (P)
open_ctree+0x1f50/0x23b0 [btrfs]
btrfs_get_tree+0x89c/0xc48 [btrfs]
vfs_get_tree+0x30/0x110
vfs_cmd_create+0x58/0xe8
__arm64_sys_fsconfig+0x39c/0x518
invoke_syscall.constprop.0+0x48/0x120
el0_svc_common.constprop.0+0x40/0xe8
do_el0_svc+0x24/0x38
el0_svc+0x50/0x310
el0t_64_sync_handler+0xa0/0xe8
el0t_64_sync+0x198/0x1a0
---[ end trace 0000000000000000 ]---
BTRFS warning (device sdd): unable to release extent buffer 365985792 owner 3 gen 17 refs 3 flags 0x5
[CAUSE]
In that invalidate_and_check_btree_folios() we wait for the eb to finish
its read, then check if it's only held by us and the btree inode.
If not, then do a warning as it may be still held, and could cause
problems.
But there is a small window where the check can lead to false alerts:
Thread A (Read endio) | Thread B (Unmount)
----------------------------------+-------------------------------------
end_bbio_meta_read() |
| The eb has one extra ref held |
| by the reader, and has |
| EXTENT_BUFFER_READING flag set | invalidate_and_check_btree_folios()
| | |
|- clear_extent_buffer_reading() | |
| | |- wait_on_bit_io();
| | | The EXTENT_BUFFER_READING flag is
| | | cleared
| | |- if (refcount_read(eb->refs) > 2)
| | The eb is held by the read, us
| | and btree inode, thus it
| | will trigger the warning
|- free_extent_buffer() |
[FIX]
Introduce a helper, free_extent_buffer_clear_reading().
If the new parameter, @clear_reading, is set, we will hold the spinlock
at the beginning of free_extent_buffer_clear_reading() to make sure the
EXTENT_BUFFER_READING flag is cleared inside the same critical section
of decreasing refs.
Now free_extent_buffer() will just call
free_extent_buffer_clear_reading() with @clear_reading set to false, so
no behavior change.
But for end_bbio_meta_read(), it will not clear_extent_buffer_reading()
directly, but pass @clear_reading as true.
Then inside invalidate_and_check_btree_folios(), hold the refs_lock
before reading refs.
So that we eliminate the race window completely.
Reported-by: Su Yue <glass.su@suse.com>
Link: https://lore.kernel.org/linux-btrfs/DC0C775E-13B3-47D9-9AB2-895BB11C029D@suse.com/
Fixes: 83f7e52b7ed1 ("btrfs: warn about extent buffer that can not be released")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
A fully truncated ordered extent (truncated_len == 0) wrote no data, so its
->csum_list is empty and btrfs_finish_ordered_zoned() trips:
assertion failed: !list_empty(&ordered->csum_list), in fs/btrfs/zoned.c:2141
Since commit 66ff4d366e7e a short or cancelled direct IO write finishes the
unsubmitted ordered extent as truncated with uptodate = true instead of
setting BTRFS_ORDERED_IOERR, so it now reaches btrfs_finish_ordered_zoned()
rather than being skipped by the IOERR check in btrfs_finish_ordered_io().
generic/208 hits this on a zoned filesystem.
Return early for these, like the BTRFS_ORDERED_PREALLOC case; there is no
zone append result to record and btrfs_finish_one_ordered() skips them too.
Fixes: 66ff4d366e7e ("btrfs: fix false IO failure after falling back to buffered write")
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
commit 095be159f3eb ("btrfs: unify folio dirty flag clearing") replaced
the folio_clear_dirty_for_io() call in extent_write_cache_pages() with a
plain folio_test_dirty() check. Besides clearing the dirty flag,
folio_clear_dirty_for_io() also calls folio_mkclean(), which write-protects
the shared mmap PTEs mapping the folio. Note that we still do call
folio_clear_dirty_for_io() later in submit_one_sector() when we clear
dirty on the last sector of the folio (the only sector for non-subpage
cases). But we lost this early call in extent_write_cache_pages().
Without the extra write-protection, a process with the file mmap-ed can
modify a sector while it is being used by writeback in a way that
expects a stable folio (checksumming, compressing, copying, etc...)
without faulting, which manifests as a handful of concrete bugs.
1. For large folios or subpage sectorsize, it is possible to submit a bio
which does not cover the whole folio. When this happens, we will have a
bio in flight for a folio that we have *not* called
folio_clear_dirty_for_io() on. If a task with an existing mmap-ed PTE
writes (without faulting..) in this window, it can result in
corruptions. If the write arrives while the checksumming or writing itself
is underway, this can result in an invalid checksum and later corruption
reports on read. If the write arrives after checksumming/writing is done
but before the last sector dirty is cleared, then the write is present
in page cache but doesn't affect the dirty tracking and will be lost
when the folio is fully finished being submitted and the dirty bit
is cleared. This results in losing the write even if fsync() is called.
2. For zoned submissions which are done in batch separate from the main
extent_writepage() loop, we also risk csum violations for those
submissions. Zoned writes are clamped to max_zone_append_size and are
not aligned with folios, so a submission can span two folios. The first
folio being processed in extent_write_cache_pages() will call
extent_write_locked_range() which will submit the partial range of the
next folio, while the rest of that folio could still be dirty. So
clearing dirty on the submitted sectors doesn't call
folio_clear_dirty_for_io() and we have the same issue. Since
extent_write_cache_pages() skips these batch submitted folios (they are
already marked for writeback from submission by the preceding folio), we
must add the extra write protection in lock_delalloc_folios().
3. For inline extents this will subtly risk losing writes that happen
after/while we copy the inline extent but before we clear dirty on
the folio.
4. For folios spanning EOF, mmap could tamper with the zeroed bytes past
EOF and cause them to be persisted where future faults would improperly
see them instead of zeros.
5. Finally, for compressed extents, we risk modifying the folios while we
work on compressing them which will result in corrupted compressed data.
Specifically, in run_delalloc_compressed() we queue up work to do
compress_file_range() in BTRFS_COMPRESSION_CHUNK_SIZE (512K) chunks which
will call btrfs_folio_clamp_clear_dirty() on the range. For non-subpage,
this will always clear the whole folio, safely. For subpage, we risk a
partial clear here as well. In particular, imagine a 2M folio broken up
into 512K chunks of work which might start compression work on one chunk
before all the chunks compress_file_range() workers have gotten far
enough to finish clearing all the dirty bitmaps of the folio and getting
to folio_clear_dirty_for_io(). Large folios on the edges of submission
ranges are similarly at risk to be only partly cleared.
This particular gap was introduced by a second patch in the same series:
commit a4ef54dbb576 ("btrfs: make extent_range_clear_dirty_for_io() to handle sector size < page size cases")
We cannot simply restore the call to folio_clear_dirty_for_io() because
that also drops the dirty flag off the folio which violates invariants
introduced for large folios by
commit 334509ce9d07 ("btrfs: use dirty flag to check if an ordered extent needs to be truncated")
and results in failing to invalidate clean folios past i_size, resulting
in deadlocks.
Therefore, to fix it, leave the existing semantics w.r.t. the folio's
dirty flag (to preserve the correct invalidate behavior) but ensure that
the other aspect of folio_clear_dirty_for_io(), folio_mkclean(), is run
on the folio when we lock it for writeback.
Finally, to help prevent similar regressions in the future, add a debug
warning that triggers at the known corruption sites if we have failed to
write protect the folio.
Assisted-by: LLM (debug, reproduce, research fix, review patch)
Fixes: 095be159f3eb ("btrfs: unify folio dirty flag clearing")
Fixes: a4ef54dbb576 ("btrfs: make extent_range_clear_dirty_for_io() to handle sector size < page size cases")
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Boris Burkov <boris@bur.io>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
[COMPILER WARNING]
With GCC 11.5.0 and KASAN enabled on arm, the following warning is
triggered during compiling:
In file included from ./include/asm-generic/rwonce.h:26,
from ./arch/arm64/include/asm/rwonce.h:81,
from ./include/linux/compiler.h:369,
from ./include/linux/array_size.h:5,
from ./include/linux/kernel.h:16,
from fs/btrfs/ioctl.c:6:
In function ‘instrument_copy_from_user_before’,
inlined from ‘_inline_copy_from_user’ at ./include/linux/uaccess.h:184:2,
inlined from ‘copy_from_user’ at ./include/linux/uaccess.h:222:9,
inlined from ‘btrfs_ioctl_get_csums.isra’ at fs/btrfs/ioctl.c:5220:6:
./include/linux/kasan-checks.h:38:27: warning: ‘args’ may be used uninitialized [-Wmaybe-uninitialized]
38 | #define kasan_check_write __kasan_check_write
./include/linux/instrumented.h:146:9: note: in expansion of macro ‘kasan_check_write’
146 | kasan_check_write(to, n);
| ^~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c: In function ‘btrfs_ioctl_get_csums.isra’:
./include/linux/kasan-checks.h:20:6: note: by argument 1 of type ‘const volatile void *’ to ‘__kasan_check_write’ declared here
20 | bool __kasan_check_write(const volatile void *p, unsigned int size);
| ^~~~~~~~~~~~~~~~~~~
fs/btrfs/ioctl.c:5201:43: note: ‘args’ declared here
5201 | struct btrfs_ioctl_get_csums_args args;
| ^~~~
[POSSIBLE FALSE ALERTS]
This seems to be a false alert from certain GCC versions.
The @args is immediately over-written by copy_from_user(), and there is
no code touching that @args until copy_from_user() finished correctly.
[WORKAROUND]
Initialize 'args' to zero, which suppresses the warning.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
[BUG]
There is a lockdep report related to device scan:
======================================================
WARNING: possible circular locking dependency detected
7.2.0-20260712.rc2.git0.e3321fa3034d.300.fc44.s390x+debug #1 Not tainted
------------------------------------------------------
(udev-worker)/1653 is trying to acquire lock:
0000006919232220 (&type->i_mutex_dir_key#2){++++}-{3:3}, at: lookup_slow+0x3e/0x70
but task is already holding lock:
00000069238564d8 (&fs_devs->device_list_mutex){+.+.}-{3:3}, at: device_list_add.constprop.0+0x148/0xc60
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #5 (&fs_devs->device_list_mutex){+.+.}-{3:3}:
lock_acquire+0x150/0x3f0
__mutex_lock+0xba/0xdc0
mutex_lock_nested+0x32/0x40
write_all_supers+0x7a/0x670
btrfs_sync_log+0xae6/0xdd0
btrfs_sync_file+0x4fa/0x7a0
__s390x_sys_fsync+0x52/0xa0
__do_syscall+0x172/0x750
system_call+0x72/0x90
-> #4 (&fs_info->tree_log_mutex){+.+.}-{3:3}:
lock_acquire+0x150/0x3f0
__mutex_lock+0xba/0xdc0
mutex_lock_nested+0x32/0x40
btrfs_sync_log+0xaba/0xdd0
btrfs_sync_file+0x4fa/0x7a0
__s390x_sys_fsync+0x52/0xa0
__do_syscall+0x172/0x750
system_call+0x72/0x90
-> #3 (btrfs_trans_num_extwriters){.+.+}-{0:0}:
lock_acquire+0x150/0x3f0
join_transaction+0x108/0x680
start_transaction+0x21a/0x660
btrfs_join_transaction+0x32/0x40
btrfs_dirty_inode+0x52/0xf0
touch_atime+0x90/0xc0
filemap_read+0x446/0x450
vfs_read+0x208/0x370
ksys_read+0x88/0x120
__do_syscall+0x172/0x750
system_call+0x72/0x90
-> #2 (btrfs_trans_num_writers){.+.+}-{0:0}:
reacquire_held_locks+0x14c/0x240
__lock_release.isra.0+0xd8/0x380
lock_release+0xf6/0x270
percpu_up_read+0x28/0xf0
__btrfs_end_transaction+0x178/0x1f0
btrfs_dirty_inode+0x82/0xf0
touch_atime+0x90/0xc0
btrfs_file_mmap_prepare+0x8c/0xa0
__mmap_region+0x214/0x780
mmap_region+0x108/0x160
do_mmap+0x402/0x5a0
vm_mmap_pgoff+0x156/0x230
ksys_mmap_pgoff+0x17e/0x220
__s390x_sys_old_mmap+0xa8/0x140
__do_syscall+0x172/0x750
system_call+0x72/0x90
-> #1 (&mm->mmap_lock){++++}-{3:3}:
lock_acquire+0x150/0x3f0
__might_fault+0x7a/0xa0
filldir64+0x11c/0x210
offset_readdir+0x92/0x200
iterate_dir+0xcc/0x2d0
__do_sys_getdents64+0x7a/0x130
__do_syscall+0x172/0x750
system_call+0x72/0x90
-> #0 (&type->i_mutex_dir_key#2){++++}-{3:3}:
check_prev_add+0x160/0xf40
__lock_acquire+0x12aa/0x15a0
lock_acquire+0x150/0x3f0
down_read+0x5a/0x280
lookup_slow+0x3e/0x70
path_lookupat+0x1f0/0x370
filename_lookup+0xce/0x1f0
kern_path+0x48/0x70
is_same_device+0x146/0x300
device_list_add.constprop.0+0x1be/0xc60
btrfs_scan_one_device+0x13a/0x2f0
btrfs_control_ioctl+0x110/0x1e0
__s390x_sys_ioctl+0xfa/0x130
__do_syscall+0x172/0x750
system_call+0x72/0x90
other info that might help us debug this:
Chain exists of:
&type->i_mutex_dir_key#2 --> &fs_info->tree_log_mutex --> &fs_devs->device_list_mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&fs_devs->device_list_mutex);
lock(&fs_info->tree_log_mutex);
lock(&fs_devs->device_list_mutex);
rlock(&type->i_mutex_dir_key#2);
*** DEADLOCK ***
2 locks held by (udev-worker)/1653:
#0: 0000016c727051c8 (uuid_mutex){+.+.}-{3:3}, at: btrfs_control_ioctl+0x102/0x1e0
#1: 00000069238564d8 (&fs_devs->device_list_mutex){+.+.}-{3:3}, at: device_list_add.constprop.0+0x148/0xc60
stack backtrace:
CPU: 2 UID: 0 PID: 1653 Comm: (udev-worker) Not tainted 7.2.0-20260712.rc2.git0.e3321fa3034d.300.fc44.s390x+debug #1 PREEMPT
Hardware name: IBM 3931 A01 701 (LPAR)
Call Trace:
[<0000016c70680e3e>] dump_stack_lvl+0xae/0x108
[<0000016c7078aa44>] print_circular_bug+0x1a4/0x230
[<0000016c7078ac5c>] check_noncircular+0x18c/0x1b0
[<0000016c7078c030>] check_prev_add+0x160/0xf40
[<0000016c7078fbaa>] __lock_acquire+0x12aa/0x15a0
[<0000016c7078fff0>] lock_acquire+0x150/0x3f0
[<0000016c7180e2fa>] down_read+0x5a/0x280
[<0000016c70b88dde>] lookup_slow+0x3e/0x70
[<0000016c70b8f5d0>] path_lookupat+0x1f0/0x370
[<0000016c70b900ae>] filename_lookup+0xce/0x1f0
[<0000016c70b90218>] kern_path+0x48/0x70
[<0000016c70f5b4a6>] is_same_device+0x146/0x300
[<0000016c70f67cfe>] device_list_add.constprop.0+0x1be/0xc60
[<0000016c70f688da>] btrfs_scan_one_device+0x13a/0x2f0
[<0000016c70ed9cb0>] btrfs_control_ioctl+0x110/0x1e0
[<0000016c70b97d0a>] __s390x_sys_ioctl+0xfa/0x130
[<0000016c718004d2>] __do_syscall+0x172/0x750
[<0000016c718155d2>] system_call+0x72/0x90
[CAUSE]
Btrfs device scan will call is_same_device() with device_list_mutex
held. But is_same_device() will call kern_path() which will do path
resolution and lock the inode.
So device scan has the following lock sequence:
mutex_lock(device_list_mutex) from device_list_add()
|
v
inode_lock_shared() from lookup_slow() during kern_path().
Meanwhile another thread is fsyncing, which has the following
lock sequence:
inode_lock() from btrfs_inode_lock() inside btrfs_direct_write()
|
v
mutex_lock(tree_log_mutex() from btrfs_sync_log(), which is further
triggered from
iomap_dio_complete()->generic_write_sync()->btrfs_sync_file().
|
v
mutex_lock(device_list_mutex) from write_all_supers() inside
btrfs_sync_log().
So the device scan has a reversed lock sequence, compared to the fsync
one, this means we can have the following deadlock:
Device scan | Fsync
----------------------------------------+--------------------------------
device_list_mutex locked |
| inode locked
| try to lock device_list_mutex
try to lock inode |
[FIX]
Instead of a full path lookup, use dev_t to determine if two device
paths are pointing to the same block device.
Inside kernel dev_t is going to uniquely determine a block device, and
the device path lookup is already done by lookup_bdev(), which is done
without device_list_mutex held, thus no reversed locking sequence.
Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Link: https://lore.kernel.org/linux-btrfs/5a9d9847-4ae6-43c4-afdc-6e5fa51d6117@linux.ibm.com/
Fixes: 2e8b6bc0ab41 ("btrfs: avoid unnecessary device path update for the same device")
Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
reserve_chunk_space() stores the return value of
btrfs_zoned_activate_one_bg() in ret. The helper can return 1 after
successfully activating a block group, but ret is later used to decide
whether to reserve metadata for chunk tree updates.
As a result, successful activation skips btrfs_block_rsv_add() and leaves
trans->chunk_bytes_reserved unchanged. Use a separate variable for the
activation result so positive success does not affect the later
reservation. Keep activation failures in ret instead of returning early so
the function uses the common tail path.
Fixes: b6a98021e401 ("btrfs: zoned: activate necessary block group")
CC: stable@vger.kernel.org
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Guanghui Yang <3497809730@qq.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Commit 7425a2894019 ("btrfs: introduce btrfs_bio_for_each_block_all()
helper") uses the new helper to replace the nested loop inside
verify_bio_data_sectors(), which simplifies the code.
However that also changed the behavior of "continue" when a block has no
data checksum.
Previously the "continue" will skip the old for() loop, which will also
increase @total_sector_nr.
Now the "continue" will skip the new btrfs_bio_for_each_block_all()
loop, which doesn't update @total_sector_nr.
This means if we hit a block that has no data checksum, we will skip all
the remaining blocks no matter if they have data checksum.
As @total_sector_nr will never be updated, and that test_bit() will
always return false.
Fix it by increasing @total_sector_nr before calling "continue".
Fixes: 7425a2894019 ("btrfs: introduce btrfs_bio_for_each_block_all() helper")
Reviewed-by: Daniel Vacek <neelx@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The reclaim_mark field in struct btrfs_block_group was a u64 that was
incremented when marking block groups for reclaim during sweeping, but
the actual counter value was never used - only the zero/non-zero state
mattered for determining if a block group needed reclaim.
Convert it to a bool to properly reflect its usage and reduce memory
footprint by 8 bytes. Update assignments to use true/false instead of
increment and zero.
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Sun YangKai <sunk67188@gmail.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The 'idmap' parameter is derived from 'file' that we also pass to
__btrfs_ioctl_snap_create(), assign it inside the function.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
When rescue=ibadroots ignores a failure to load the raid stripe tree root,
fs_info->stripe_root remains NULL. After the rescue mount proceeds, reading
file data that requires the raid stripe tree reaches
btrfs_get_raid_extent_offset().
Currently btrfs_search_slot() handles the NULL root and returns -EINVAL.
This avoids a NULL pointer dereference, but provides no diagnostic and
incorrectly describes missing filesystem metadata as an invalid argument.
Check stripe_root before allocating a path, emit a rate-limited error with
the logical address, and return -EUCLEAN.
Lookups with a valid stripe root are unchanged.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Dongjiang Zhu <zhudongjiang@fnnas.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
[BUG]
Mounting with rescue=ibadroots after corrupting the block group tree
root triggers a NULL pointer dereference:
BUG: kernel NULL pointer dereference, address: 0000000000000100
RIP: 0010:btrfs_update_global_block_rsv+0x9d/0x1c0 [btrfs]
Call Trace:
fill_dummy_bgs+0xd4/0x120 [btrfs]
open_ctree+0xc6e/0x1ca0 [btrfs]
btrfs_get_tree+0x50d/0xa40 [btrfs]
The same crash occurs with a corrupted raid stripe tree root, via
btrfs_read_block_groups() instead of fill_dummy_bgs().
[CAUSE]
With rescue=ibadroots, btrfs_read_roots() allows the mount to continue
when either root cannot be read, leaving the corresponding root pointer
NULL while its on-disk feature bit remains set.
btrfs_update_global_block_rsv() then dereferences the missing root based
on the feature bit alone.
[FIX]
Rescue mounts are fully read-only and cannot start transactions, so the
global reserve is never consumed. Under btrfs_is_full_ro(), mark the
reserve as full and return before performing the accounting.
And since we need to check if the fs is mount fully RO, export
fs_is_full_ro() as btrfs_is_full_ro(), and move it to fs.h.
Fixes: 8dbfc14fc736 ("btrfs: account block group tree when calculating global reserve size")
Fixes: 515020900d44 ("btrfs: read raid stripe tree from disk")
Suggested-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Dongjiang Zhu <zhudongjiang@fnnas.com>
[ Squash the fs_is_full_ro() export commit into this one. ]
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
btrfs_reset_unused_block_groups() resets a block group's zone and sets
alloc_offset back to 0 so the space can be reused, but it leaves
meta_write_pointer pointing at the previous end of the zone.
Once the block group is reactivated and reused for metadata, newly
allocated tree blocks live before that stale write pointer.
btrfs_check_meta_write_pointer() then sees them behind the write pointer,
so they can never be written out in sequential order: the dirty extent
buffers are stranded and pin their btree_inode folios until unmount.
Reset meta_write_pointer back to the start of the block group for
metadata and system block groups.
Fixes: 453a73c3069a ("btrfs: zoned: reclaim unused zone by zone resetting")
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 writing out metadata extent buffers in a zoned filesystem,
btree_writepages() holds fs_info->zoned_meta_io_lock across the whole
writeback loop, including the call to btrfs_check_meta_write_pointer() ->
check_bg_is_active().
For the tree-log block group, check_bg_is_active() may fail to activate
the zone and fall back to btrfs_zone_finish_one_bg() to free an active
zone. That path waits for the running transaction to commit while still
holding zoned_meta_io_lock, but the committer needs that same lock to
write out the tree extents, so the two tasks deadlock:
Task A (kworker, metadata writeback) Task B (fsstress, transaction commit)
------------------------------------ -------------------------------------
wb_workfn() btrfs_commit_transaction(T)
btree_writepages() btrfs_write_and_wait_transaction()
btrfs_zoned_meta_io_lock() btrfs_write_marked_extents()
btrfs_check_meta_write_pointer() btree_writepages()
check_bg_is_active() [treelog_bg] btrfs_zoned_meta_io_lock()
btrfs_zone_finish_one_bg() <blocks on zoned_meta_io_lock,
btrfs_zone_finish() held by Task A>
do_zone_finish()
btrfs_inc_block_group_ro()
btrfs_wait_for_commit()
<blocks waiting for commit
of transaction T, done by
Task B>
The sibling branch in check_bg_is_active() already drops zoned_meta_io_lock
around do_zone_finish() for this exact reason. Do the same in the tree-log
branch: release the lock around btrfs_zone_finish_one_bg() and re-acquire
it afterwards. The lock only protects fs_info->active_{meta,system}_bg,
which this branch does not touch, and ctx->zoned_bg keeps a reference to
the block group across the unlock, so nothing is lost while the lock
is dropped.
This hang occasionally reproduces with fstests generic/475 on a zoned
btrfs filesystem.
Fixes: 13bb483d32ab ("btrfs: zoned: activate metadata block group on write time")
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>
|
|
[BUG]
The following script can lead to unexpected qgroup rescan failure:
# mkfs.btrfs -f -O quota $dev
# mount $dev $mnt
# mount -o remount,rescue=ibadroots $mnt
^^^^^ This above command is expected to fail
# btrfs quota rescan -w $mnt
^^^^^ The above qgroup rescan is not expected to fail
# btrfs qgroup show $mnt
WARNING: qgroup data inconsistent, rescan recommended
Qgroupid Referenced Exclusive Path
-------- ---------- --------- ----
0/5 16.00KiB 16.00KiB <toplevel>
The above short script will be converted to a proper fstests case.
[CAUSE]
Inside btrfs_reconfigure(), if either btrfs_check_options() or
btrfs_check_features() failed, we will always have
BTRFS_FS_STATE_REMOUNTING set for the fs until the next successful
remount.
That BTRFS_FS_STATE_REMOUNTING flag will interrupt several operations,
including:
- Qgroup rescan
- Auto defrag
- Space reclaim
[FIX]
Change the error handling of btrfs_check_options() and
btrfs_check_features() to goto restore label.
Fixes: eddb1a433f26 ("btrfs: add reconfigure callback for fs_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>
|
|
Currently we're using scrub_stripe::folios[] to store all contents of a
stripe.
This means we need all the extra work to handle things like sub-page
cases, and also require larger folios to handle bs > ps cases.
On the other hand, it's not hard to allocate a 64K large folio to cover
the full stripe, getting rid of the cross-page handling.
Furthermore, even if that large folio allocation failed, we can still
use vmalloc() to allocate a virtually contiguous space and still get rid
of cross-page handling.
This patch will go with kvmalloc() to allocate 64K of memory for
the stripe buffer, thus getting rid of all the complex cross-page
handling.
The following aspects can be greatly simplified:
- Checksum verification for both data and metadata
No more per-page iteration, all in one go.
- RAID56 data caching
Just copy the buffer into the RAID56 pages.
- No more kaddr/paddr grabbing
For most cases the virtual address is enough for csum calculation and
io submission.
- Bio assembly
There is already the helper bio_add_vmalloc() to queue vmallocated
memory into a bio.
Although it means we have something else to be concerned about:
- Bio assembly
If the memory is vmallocated, we need to use bio_add_vmalloc()
Otherwise use the existing bio_add_page().
- Read endio
For vmallocated memory, we need to call
invalidate_kernel_vmap_range().
- Scrub bbio bvec size
Since scrub_stripe::buffer is kvmallocated, we also need to enlarge
the scrub bbio, to be able to handle the worst case, where all 64KiB is
allocated by discontiguous 4K physical pages.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Currently calc_sector_number() is implemented by comparing the first
bvec of the bbio against all blocks inside a scrub_stripe.
This implementation is a little inefficient, and depends on how the
scrub buffer is implemented.
One of the reason implementing such complex function is that, we do not
save the original bvec_iter inside a write btrfs_bio.
Although a read bbio has btrfs_bio::saved_iter to get the original
logical bytenr, it's not implemented for write bios.
On the other hand, since commit 81cea6cd7041 ("btrfs: remove
btrfs_bio::fs_info by extracting it from btrfs_bio::inode"), we always
set the btrfs_bio::file_offset as the logical bytenr for scrub, and that
member will not be modified during IO.
So this means we have a stable way to determine the logical bytenr for a
scrub bio, now calc_sector_number() is just as simple as:
return (bbio->file_offset - stripe->logical) >> sectorsize_bits;
Since we're here, also add an ASSERT() to make sure the bbio is inside
the stripe, and change the return type to unsigned int to be extra safe.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
For both scrub_repair_read_endio() and scrub_read_endio(), they share
the same bitmap update and bio put. Factor out the common code into a
helper to reduce duplication.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The last user of this macro is removed in commit 001e3fc263ce ("btrfs:
scrub: remove scrub_block and scrub_sector structures").
Now that macro is only utilized in an ASSERT(), which no longer makes
much sense.
Just remove it completely.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
There's no point in having a label where under it we do nothing but return
a variable. So remove it and directly return where we used to goto.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
btrfs_dio_iomap_begin() calls btrfs_get_extent(), which returns an
extent map reference that must be dropped on all exit paths.
For direct writes into a NOCOW range, btrfs_get_blocks_direct_write()
keeps using that extent map and asks btrfs_create_dio_extent() to
allocate the ordered extent. If that fails, for example because
btrfs_alloc_ordered_extent() fails, the function returns the error
without dropping the input extent map. The PREALLOC path avoided this by
dropping the input extent map before replacing it with the newly created
one.
Check the error from btrfs_create_dio_extent() before replacing the
map and drop the input extent map on failure.
Fixes: 5f9a8a51d8b9 ("Btrfs: add semaphore to synchronize direct IO writes with fsync")
CC: stable@vger.kernel.org
Reviewed-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
This mount option is marked deprecated since the introduction of
"rescue=" mount option group, in v5.9.
That's already a long time ago, and it should be safe to completely
remove the old "usebackuproot" mount option now.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Neal Gompa <neal@gompa.dev>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The mount option "rescue=all" should be a shortcut to include all
"rescue=" mount options. But unfortunately "rescue=usebackuproot" is not
included.
Include that option so "rescue=all" has a better chance to mount a
corrupted fs.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Neal Gompa <neal@gompa.dev>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
According to btrfs(5) man page, all rescue options should require a
read-only mount.
But that read-only check is only introduced for newer rescue options,
not for the pre-existing "usebackuproot" one.
Furthermore, a filesystem that requires "rescue=" mount option already
means it's corrupted, even if "rescue=usebackuproot" allowed the fs to
be mounted RW, one should not trust such fs anymore until a
comprehensive btrfs-check run and proper evaluation.
Change the behavior to match the document, and since
"rescue=usebackuproot" is now a full RO mount option, it is no longer a
one-shot option, therefore remove it from btrfs_clear_oneshot_options().
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Neal Gompa <neal@gompa.dev>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The 'tree_id' parameter in btrfs_search_path_in_tree() was only being
used in order to fetch the root tree to be considered for the
search. For this same reason this function was also requiring a 'struct
btrfs_fs_info' parameter. This commit replaces these two parameters with
a single 'struct btrfs_root' one, which identifies from which root tree
the search should happen.
This function only has one caller, the inode lookup ioctl, which knows
how to provide the root tree for each case. In fact, if args->treeid ==
0, then we don't even have to allocate a new root tree object, and we
can reuse the one provided by the ioctl system call, thus avoiding an
extra allocation.
Signed-off-by: Miquel Sabaté Solà <mssola@mssola.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Previously btrfs forces direct writes to fall back to buffered ones if the
inode has data checksum or the profile has duplication.
That fallback is to avoid the content being modified that the final
content may mismatch with the checksum or the other mirrors.
That brings a pretty huge performance cost, which already caused some
concern at that time.
But later upstream commit c9d114846b38 ("iomap: add a flag to bounce
buffer direct I/O") introduced a new method by copying the content into
new pages, and do all the operations based on the newly allocated pages.
So let btrfs to utilize the new flag for direct writes if we require
stable folios.
There is a quick benchmark, using the following fio setup:
fio --name=randwrite --filename $mnt/foobar --ioengine=libaio --size=4G \
--rw=randwrite --iodepth=64 --runtime=60 --time_based --direct=1 \
--bs=$blocksize
Unit is MiB/s.
Blocksize | Zero-copy (*) | Buffered | Bounce
-----------+---------------+----------+-----------
4K | 35.1 | 17.1 | 33.8
64K | 522 | 251 | 492
*: This is done by reverting the commit 968f19c5b1b7 ("btrfs: always
fallback to buffered write if the inode requires checksum")
Although with page bouncing the performance is only around 95% of
true-zero copy, it's still almost double the performance of buffered
fallback.
There will be a small change in behavior, since we're using
IOMAP_DIO_BOUNCE flag to allocate new folios, NOWAIT flag will
immediately fail.
So for true NOWAIT direct IOs, NODATASUM and RAID0/SINGLE profiles are
still required.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
[BUG]
Syzbot reported a bug that there can be conflicting OEs for the same
range:
BTRFS critical (device loop4): panic in insert_ordered_extent:264: overlapping ordered extents, existing oe file_offset 16384 num_bytes 430080 flags 0x1089, new oe file_offset 16384 num_bytes 430080 flags 0x80 (errno=-17 Object alrea[ 179.162726][ T6897] BTRFS critical (device loop4): panic in insert_ordered_extent:264: overlapping ordered extents, existing oe file_offset 16384 num_bytes 430080 flags 0x1089, new oe file_offset 16384 num_bytes 430080 flags 0x80 (errno=-17 Object already exists)
------------[ cut here ]------------
kernel BUG at fs/btrfs/ordered-data.c:264!
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/09/2026
RIP: 0010:btrfs_alloc_ordered_extent+0x943/0xad0
Call Trace:
<TASK>
cow_file_range+0x744/0x12a0
fallback_to_cow+0x5ea/0xa00
run_delalloc_nocow+0x110c/0x17a0
btrfs_run_delalloc_range+0xbe4/0x1c20
writepage_delalloc+0x104d/0x1ba0
btrfs_writepages+0x1667/0x28b0
do_writepages+0x338/0x560
filemap_fdatawrite_range+0x1f2/0x300
btrfs_fdatawrite_range+0x54/0xf0
btrfs_direct_write+0x6a0/0xc30
btrfs_do_write_iter+0x329/0x790
do_iter_readv_writev+0x624/0x8d0
vfs_writev+0x34c/0x990
__se_sys_pwritev2+0x17a/0x2a0
do_syscall_64+0x174/0x580
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
---[ end trace 0000000000000000 ]---
[CAUSE]
Since commit ff66fe666233 ("btrfs: fix incorrect buffered IO fallback
for append direct writes"), if the direct IO finished short, we will
revert the isize back to the original one, so that append writes can be
respected during the buffered fallback.
Normally we rely on lock_and_cleanup_extent_if_need() function during
buffered writeback to wait for any existing ordered extents.
But that ordered extent waiting only happens if the start_pos is inside
the isize.
Since we have reverted the isize during failed direct IO, we will not
wait for any ordered extents.
This means we can have a race where the direct IO OE is still in the
tree, finished but not yet removed, then we're inserting the OE for the
buffered write, causing the above crash.
[FIX]
Make the OE wait to be unconditional, to handle the reverted isize
situation.
And since lock_and_cleanup_extent_if_need() now either lock the
extents or return -EAGAIN, also remove the branches that handles
no-extent-locked cases, and rename it to remove the "_if_need" suffix.
The following micro benchmark shows the runtime difference for
btrfs_buffered_write(), doing `xfs_io -f -c "pwrite 0 1m"` workload,
all values are the average runtime in nano seconds.
function runtime | before | after
-----------------------------------+-------------+---------------
lock_and_cleanup_extent_if_need() | 58.2 | 183.0
btrfs_buffered_write() | 2115.6 | 2973.3
The overall runtime of btrfs_buffered_write() is still pretty
tiny (still less than 3 micro seconds), I'd say the extra cost is still
acceptable.
An alternative to fix this problem is to wait ordered extents during
iomap_end() where the isize revert is done.
But that solution will break nowait requirement, as if a nowait direct
IO finished short, we have to wait for the OEs unconditionally or the
next append buffered IO can still hit the same problem.
So here we have to move the wait cost to buffered write, but at least
the code is slightly more streamline.
Reported-by: syzbot+ba2afde329fc27e3f22e@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=ba2afde329fc27e3f22e
Fixes: ff66fe666233 ("btrfs: fix incorrect buffered IO fallback for append direct writes")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
There's no need to call list_del_init() against each entry when freeing
the list, as the list is local and we are freeing the entry.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
When freeing the entries from the list there is no need to initialize
the list member in an entry, since we are immediately freeing it. So use
simple list_del() instead of list_del_init().
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Use AUTO_KFREE() for the folios array, avoiding two kfree() calls, one of
them in a very specific error path.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
There's no need to have one list for each loop to defrag each subrange and
then another one to free each subrange (struct defrag_target_range).
We can do it in a single loop, freeing each subrange after defragging,
plus no need to delete each subrange from the list since we immediately
free it.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
