| Age | Commit message (Collapse) | Author |
|---|
|
Now that all NFS client code uses inode->i_ino directly to store and
access the 64-bit NFS fileid, the separate fileid field in struct
nfs_inode is unused. Remove it to save 8 bytes per NFS inode.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
Now that inode->i_ino stores the full 64-bit NFS fileid, replace all
uses of NFS_FILEID(), set_nfs_fileid(), and direct nfsi->fileid
accesses with inode->i_ino throughout the NFS client.
Remove the NFS_FILEID() and set_nfs_fileid() helper functions from
include/linux/nfs_fs.h since they are no longer needed.
Also fix two pre-existing truncation bugs in nfs4trace.h where fileid
trace fields were declared as u32 instead of u64.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
Now that inode->i_ino stores the full 64-bit NFS fileid, the
nfs_compat_user_ino64() function is no longer needed.
generic_fillattr() already copies inode->i_ino into stat->ino, so the
explicit override in nfs_getattr() is also redundant.
Also remove the now-unused nfs_fileid_to_ino_t() and
nfs_fattr_to_ino_t() helper functions that were used to XOR-fold
64-bit fileids into the old unsigned long i_ino.
Keep the enable_ino64 module parameter as a deprecated stub that
accepts but ignores the value, logging a notice when set. This avoids
breaking existing configurations that pass nfs.enable_ino64 on the
kernel command line or in modprobe.d.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
Now that inode->i_ino is a 64-bit value, store the full NFS fileid in
it directly instead of an XOR-folded hash. This makes NFS_FILEID() and
set_nfs_fileid() operate on inode->i_ino rather than the separate
nfsi->fileid field.
Since iget5_locked() and ilookup5() now accept a u64 hashval, pass the
full fileid as the hash parameter directly.
Convert direct nfsi->fileid accesses in nfs_check_inode_attributes(),
nfs_update_inode(), and nfs_same_file() to use inode->i_ino.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
|
|
The sole reason this hw exists is to let the branch clock downstream of
it keep running, with the PHY disengaged. This is not possible with the
current implementation, as the enabled status is hijacked to mean
"enabled" = "use fast/PHY source" and "disabled" = "use XO source".
This is an issue, since the mux enable state follows that of the child
branch, making the desired "child enabled, MUX @ XO" combination
impossible.
Solve that by implementing ratesetting. Because PHY clock rates may
change at runtime and aren't really deterministic from Linux, assume
ULONG_MAX as "fast clock" and 19.2 MHz as XO. All the branches in
question already set CLK_SET_RATE_PARENT, so everything works out.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Link: https://lore.kernel.org/r/20260409-topic-phy_fastclk-v1-1-6b4aaee56b90@oss.qualcomm.com
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
|
|
The 1152MHz frequency entry for the a53 currently selects a multiplier of 62, giving 1190MHz. This changes the mulitiplier to 60 giving the intended 1152MHz.
Signed-off-by: Phillip Varney <pbvarney@protonmail.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Fixes: 0c6ab1b8f894 ("clk: qcom: Add A53 PLL support")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Link: https://lore.kernel.org/r/20260605005502.313928-1-pbvarney@protonmail.com
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
|
|
ntfs_check_restart_area() validates that the $LogFile restart area and
its trailing log client record array fit within the system page size:
u16 ra_ofs, ra_len, ca_ofs;
...
ra_len = ca_ofs + le16_to_cpu(ra->log_clients) *
sizeof(struct log_client_record);
if (ra_ofs + ra_len > le32_to_cpu(rp->system_page_size) || ...)
return false;
ra_len is u16, but the right-hand side is computed in size_t
(sizeof(struct log_client_record) == 160). Both ca_ofs and log_clients
come straight from the on-disk restart area. With an on-disk
log_clients of 410 the product 410 * 160 = 65600; adding ca_ofs and
storing into the u16 ra_len truncates modulo 65536 (e.g. ca_ofs 64
gives ra_len 128), so the "fits in the page" check passes even though
the client array described by log_clients extends far beyond the page.
ntfs_check_log_client_array() then walks the array bounded only by the
on-disk log_clients count:
cr = ca + idx;
if (cr->prev_client != LOGFILE_NO_CLIENT) ...
For log_clients 410 it dereferences records up to ca + 409 * 160,
~64 KiB past the kvzalloc(system_page_size) restart-page buffer -- an
out-of-bounds read of attacker-controlled extent, reachable when a
crafted NTFS image is mounted (load_and_check_logfile() at mount time).
This is the in-kernel analogue of CVE-2022-30789, fixed in the ntfs-3g
userspace driver but never in this revived classic driver.
Compute the restart-area length in a u32 so the existing bounds check
rejects an over-large client array instead of being defeated by the
truncation. Widen ra_ofs and ca_ofs to u32 as well: both are loaded
from __le16 on-disk fields and every comparison already promotes to
int/size_t, so this changes no result and keeps the declaration uniform.
Fixes: 1e9ea7e04472 ("Revert "fs: Remove NTFS classic"")
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The $MFT attribute-list walk in ntfs_read_inode_mount() validates each
entry only with "(u8 *)al_entry + 6 > al_end" and
"(u8 *)al_entry + le16_to_cpu(al_entry->length) > al_end", but then reads
al_entry->lowest_vcn (an __le64 at offset 8) and al_entry->mft_reference
(offset 16) -- fields beyond the 6 bytes proven in range. al_entry->length
is attacker-controlled and only required non-zero, so a short entry (e.g.
length 8) placed at the tail passes both checks while the lowest_vcn /
mft_reference reads fall past al_end.
al_end is ni->attr_list + attr_list_size (the on-disk size); the buffer is
kvzalloc(round_up(attr_list_size, SECTOR_SIZE)), so the sector rounding
usually absorbs the over-read -- but when attr_list_size is a multiple of
SECTOR_SIZE there is no slack and a crafted $MFT attribute list produces an
out-of-bounds read at mount time.
Validate the entry with ntfs_attr_list_entry_is_valid() (added in patch
1/3) before dereferencing it, matching the bound the other attribute-list
walks now use. The validator already requires the length to cover the fixed
header, which makes the separate "!al_entry->length" check redundant, so
drop it too.
Fixes: 1e9ea7e04472 ("Revert "fs: Remove NTFS classic"")
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
When resolving an attribute lookup with a non-zero @lowest_vcn,
ntfs_external_attr_find() peeks at the next $ATTRIBUTE_LIST entry to
decide whether to keep searching, but bounds that not-yet-validated
entry only with "(u8 *)next_al_entry + 6 < al_end" (which proves just
bytes 0..6 are in range) and "(u8 *)next_al_entry + length <= al_end"
with an attacker-controlled, non-8-aligned length. It then reads
next_al_entry->lowest_vcn (an __le64 at offset 8) and the name at
next_al_entry->name_offset, both of which can lie past al_end -- the
exact end of the kvmalloc'd attribute-list buffer (allocated at the
on-disk attr_list_size, no rounding). A crafted on-disk $ATTRIBUTE_LIST
whose last entry sits a few bytes before al_end therefore yields a slab
out-of-bounds read when the inode is read.
Validate the look-ahead entry with ntfs_attr_list_entry_is_valid() (added
in patch 1/3) before dereferencing lowest_vcn and the name, so the same
fixed-header, length and name bounds the main attribute-list walk uses now
guard this read too.
Fixes: 1e9ea7e04472 ("Revert "fs: Remove NTFS classic"")
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A base inode's $ATTRIBUTE_LIST is sanity-checked by load_attribute_list()
only on the non-resident path; ntfs_read_locked_inode() copies a *resident*
attribute list into ni->attr_list with a plain memcpy() and no validation
at all. Every subsequent walk of ni->attr_list --
ntfs_external_attr_find(), ntfs_inode_attach_all_extents() and
ntfs_attrlist_need() -- then trusts the entries are well-formed and reads
attr_list_entry fixed-header fields
(lowest_vcn at offset 8, mft_reference at offset 16, and the name) with
bounds that assume validation already happened. A crafted resident
attribute list therefore reaches those walks unvalidated and can drive
out-of-bounds reads of the attribute-list buffer.
load_attribute_list() itself reads ale->name_offset (offset 7),
ale->mft_reference (offset 16) and the name length under only an
"al < al_start + size" bound, so its own validation loop can over-read the
fixed header of a truncated trailing entry by a few bytes.
Factor the per-entry validation into ntfs_attr_list_entry_is_valid(),
which requires each entry's fixed header (offsetof(struct
attr_list_entry, name)) to be in range before any field is dereferenced,
that ale->length is a multiple of 8 covering the fixed header plus the
name, and that the entry is in use and carries a live MFT reference.
ntfs_attr_list_is_valid() walks the buffer with it and checks the entries
tile it exactly. Use the list validator in load_attribute_list()
(replacing the open-coded loop, closing its own over-read) and on the
resident path in ntfs_read_locked_inode() (which previously skipped
validation entirely); patches 2/3 reuse the per-entry helper at the other
two attribute-list walks.
Fixes: 1e9ea7e04472 ("Revert "fs: Remove NTFS classic"")
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Replace sscanf() with kstrtoul() in cur_state_store(), as kstrto<type>
is preferred over single-variable sscanf().
Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>
[ rjw: Changelog edits ]
Link: https://patch.msgid.link/20260606210420.2311145-3-ovidiu.panait.oss@gmail.com
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
|
|
Generally, kstrtoint() is preferred to sscanf() in kernel code, so
replace the latter with the former in tt_del_tz() and tt_get_tt_zone().
Signed-off-by: Ovidiu Panait <ovidiu.panait.oss@gmail.com>
[ rjw: Changelog rewrite ]
Link: https://patch.msgid.link/20260606210420.2311145-2-ovidiu.panait.oss@gmail.com
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
|
|
log_new_dir_dentries() is an important step called during a fsync, as
well as during rename and link operations on inodes that were previously
logged. Add trace events for when entering and exiting that function.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
log_all_new_ancestors() is an important step called during a fsync, as
well as during rename and link operations on inodes that were previously
logged. Add trace events for when entering and exiting that function.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
btrfs_log_all_parents() is an important step called during a fsync, as
well as during rename and link operations on inodes that were previously
logged. Add trace events for when entering and exiting that function.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
btrfs_log_inode() is one of the most important steps called during a fsync,
as well as during rename and link operations on inodes that were previously
logged. Add trace events for when entering and exiting that function.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We use this unnamed enum for the log mode and then pass it around log
functions as an int type with the odd name "inode_only" which suggests a
boolean. So add a name to the enum and change the type everywhere to that
enum and rename the parameters to something more clear - "log_mode".
Also move the enum into tree-log.h - it will be used later by new trace
events.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
btrfs_log_inode_parent() is one of the most important steps called during
a fsync operation as well as during rename and link operations on inodes
that were previously logged. Add trace events for when entering and
exiting that function.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Currently we only have a trace event for when a fsync operation starts,
but this alone is not very helpful. Add a trace event for when fsync
finishes, which reports its return value, so that using tracing we can
see which other trace events happened in between (several will be added
soon for inode logging steps) and even measure execution time.
So rename the existing trace event btrfs_sync_file to
btrfs_sync_file_enter and add the trace event btrfs_sync_file_exit.
The naming is similar to what ext4 does (ext4_sync_file_enter and
ext4_sync_file_exit) and with similar information reported.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If we can skip logging the inode during fsync, we check for writeback
errors in the inode's mapping by calling filemap_check_wb_err() and then
jump to the 'out_release_extents' label, which in turn jumps to the 'out'
label under which we check again for a writeback error by calling
file_check_and_advance_wb_err(). So the filemap_check_wb_err() ends up
being redundant. This happens since commit 333427a505be ("btrfs: minimal
conversion to errseq_t writeback error reporting on fsync").
Remove the filemap_check_wb_err() call.
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 value of 'ret' can never be greater than zero when we reach the end of
btrfs_sync_file() but we have this ternary operator converting any such
value into -EIO. This logic exists since the first fsync implementation,
added in 2007 by commit 8fd17795b226 ("Btrfs: early fsync support"), when
all that fsync did was simply to commit a transaction, but even a call to
btrfs_commit_transaction() could never return a value greater than zero.
So stop checking for a greater than zero value and assert that 'ret' is
never greater than zero, to catch any eventual regression during future
development.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Currently the trace event is fired only when a transaction is fully
complete (its state is TRANS_STATE_COMPLETED). However during a
transaction commit we go through several states and as soon as the
state reaches TRANS_STATE_UNBLOCKED, another transaction can start.
Therefore it's useful to track every transaction state changed during
the commit of a transaction, so that we can see if a new transaction
is started before the current one is completed. Add the transaction
state to the transaction commit event and call the event everytime
we change the transaction state during commit.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
While tracing it's useful to know not just when a transaction is committed
or aborted, but also when a new one is started. So add a trace event for
transaction starts.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
While tracing it's useful to know not just when a transaction is committed
but also when one is aborted. So add a trace event for transaction aborts.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Include the in_fsync value from the transaction handle so that we can know
if a transaction commit was triggered by a fsync call.
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 transaction commit tracepoint prints fs_info->generation as if it
were the ID of the committed transaction but this does not always match
that ID. This is because the trace point is called in the transaction
commit path after the transaction is in the TRANS_STATE_COMPLETED state,
which means another transaction may have already started (which can happen
as soon as the transaction state was set to TRANS_STATE_UNBLOCKED), in
which case fs_info->generation was incremented and does not correspond
to the committed transaction anymore.
So fix this by passing a transaction handle to the trace event instead of
fs_info. This will also allow later for the trace event to dump other
useful information about the transaction.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We are not committing a transaction there, plus in subsequent patches we
want to change the argument for the trace event to be a transaction handle
instead of fs_info and in this context we don't have a transaction handle
(struct btrfs_trans_handle, only a struct btrfs_transaction). So remove the
call to the trace point.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
We are not committing a transaction there, plus in subsequent patches we
want to change the argument for the trace event to be a transaction handle
instead of fs_info and in this context we don't have a transaction handle
(struct btrfs_trans_handle, only a struct btrfs_transaction). So remove the
call to the trace point.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
A transaction commit is global, not per root, and we are currently always
emitting a root id field matching the root tree for no good reason at all,
causing confusion for no reason at all. So remove the root field.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
There is no need to add a double negation (!!) to the update field because
the field has a boolean type.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
This introduces extra checks using the previous key.
If there is a previous key, we can do extra validations:
- The previous key is FREE_SPACE_INFO
This means the current extent/bitmap should be inside the
free space info key range.
And matches the type of the free space info.
- The previous key is FREE_SPACE_EXTENT or FREE_SPACE_BITMAP
In that case both the current and previous key should belong to the same
block group.
Thus the key type must match, and no overlap between the two keys.
These extra checks are inspired by the recently added type checks during
free space tree loading.
The new tree-checker checks will allow earlier detection, but the
loading time checks are still needed, as the tree-checker checks are
still inside the same leaf, not matching per-bg level checks.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Add an extra check to ensure the free space extent/bitmap and space info
keys won't overflow.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Currently both check_free_space_extent() and check_free_space_bitmap()
share a very common validation on the keys.
Extract them into a helper, check_free_space_common_key(), and
change the output string ("extent" or "bitmap") depending on the key type.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Btrfs has an internal flag/subpage bitmap called ordered, which is to
indicate that a block has corresponding ordered extent covering it.
However this requires extra synchronization between the inode ordered
tree, and the folio flag/subpage bitmap, not to mention we need to
maintain the extra folio flag with subpage bitmap.
As a step to align btrfs_folio_state more closely to iomap_folio_state,
remove the btrfs specific ordered flag/bitmap.
This will also save us 64 bytes for the bitmap of a huge folio.
Since we're here, also update the ASCII graph of the bitmap, as there
are only 3 sub-bitmaps now, show all sub-bitmaps directly.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
This involves:
- The ASSERT() inside end_bbio_data_write()
It's only an ASSERT() and it has never been triggered as far as I
know.
- btrfs_migrate_folio()
Since all folio_test_ordered() usage will be removed, there is no need to
copy the folio ordered flag.
- The ASSERT() inside btrfs_invalidate_folio()
This one has its usefulness as it indeed caught some bugs during
development.
But that's the last user and will not be worth the folio flag or the
subpage bitmap.
This will allow btrfs to finally remove the ordered flags.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Currently there are only two folio ordered flag users:
- extent_writepage_io()
To ensure the folio range has an ordered extent covering it.
This is from the legacy COW fixup mechanism, which is already removed
and only a simple check is left.
- btrfs_invalidate_folio()
This is to avoid race with end_bbio_data_write(), where
btrfs_finish_ordered_extent() will be called to handle the OE
finishing.
But for btrfs_invalidate_folio() we have already waited for the folio
writeback to finish, and locked the folio.
This means we can use the dirty flag to check if a range is already
submitted or not.
If the OE range is not dirty, it means the range has been submitted and
its dirty flag was cleared. And since we have already waited for
writeback, the endio function will handle the OE finishing.
Thus if the range is not dirty, we must skip the range.
If the OE range is dirty, it means we have allocated an ordered extent but
have not yet submitted the range. And that's exactly the case where we need
to truncate the ordered extent.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Currently during folio writeback, we call folio_clear_dirty_for_io()
before extent_writepage(), which causes folio dirty flag to be cleared,
but without touching the subpage bitmaps.
This works fine for the bio submission path, as we always call
btrfs_folio_clear_dirty() to clear the subpage bitmap.
But this is far from consistent, thus this patch is going to unify the
behavior to always use btrfs_folio_clear_dirty() helper to clear both
folio flag and subpage bitmap.
This involves:
- Replace folio_clear_dirty_for_io() with folio_test_dirty()
There is only one call site calling folio_clear_dirty_for_io() outside
of subpage.c, that's inside extent_write_cache_pages() just before
extent_writepage().
- Make btrfs_invalidate_folio() clear dirty range for the whole folio
The function btrfs_invalidate_folio() is also called during
extent_writepage().
If we had a folio completely beyond isize, we call
folio_invalidate() -> btrfs_invalidate_folio() to free the folio.
Since we no longer have folio_clear_dirty_for_io() to clear the folio
dirty flag, we must manually clear the folio dirty flag for the
to-be-invalidated folio, and also clear the PAGECACHE_TAG_DIRTY tag.
The tag clearing is done using a new helper,
btrfs_clear_folio_dirty_tag(), which is almost the same as the old
btree_clear_folio_dirty_tag(), but with minor improvements including:
* Remove the folio_test_dirty() check
We have already done an ASSERT().
* Add an ASSERT() to make sure folio is mapped
- Add extra ASSERT()s before clearing folio private
During development I hit dirty folios without the private flag set,
and that caused a lot of ASSERT()s.
The reason is that btrfs_invalidate_folio() is relying on the dirty
flag being cleared when it's called from extent_writepage().
Add extra ASSERT()s inside clear_folio_extent_mapped() to catch
wild dirty/writeback flags.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Currently btrfs detects dirty folio which doesn't have an ordered extent
at extent_writepage_io(), but that is not ideal:
- The check is not handling all dirty blocks
We can have multiple blocks inside a large folio, but the whole folio
is marked ordered as long as there is one ordered extent in the range.
We can still hit cases where some dirty blocks do not have
corresponding ordered extents.
Instead of checking the folio ordered flags, do the check at
alloc_new_bio(), where we're already searching for ordered extents for
writebacks.
If we didn't find an ordered extent, we should already give an error
message and notify the caller there is something wrong.
This allows us to check every block that goes through
submit_extent_folio().
With this new and more reliable check, we can remove the old check.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Currently there are two members inside btrfs_folio_state that are related
to locked bitmap:
- locked sub-bitmap inside btrfs_folio_state::bitmaps[]
The enum btrfs_bitmap_nr_locked determines the sub-bitmap.
- btrfs_folio_state::nr_locked
Which records how many blocks are locked inside the folio.
The locked sub-bitmap is a btrfs specific per-block tracking mechanism,
which is mostly for async-submission, utilized by compressed writes.
The sub-bitmap itself is a super set of nr_locked, as it can provide a
more reliable tracking.
But the sub-bitmap itself can be pretty large for the incoming huge
folio, 2M sized folio for 4K page size, meaning 512 bits for one
sub-bitmap.
Furthermore, in the long run compression will be reworked to get rid of
async-submission completely, there is not much need for a full
sub-bitmap to track the locked status.
This patch removes the locked sub-bitmap and only relies on @nr_locked
atomic to do the tracking.
This can also save 64 bytes from btrfs_folio_state::bitmaps[] for a huge
folio.
This will reduce some safety checks, as previously if a block is not
locked, btrfs_folio_end_lock()/btrfs_folio_end_lock_bitmap() will find
out that, and skip reducing @nr_locked for that block, and avoid
under-flow.
But this safety net itself shouldn't be necessary in the first place.
If we're unlocking a block that is not locked, it's a bug in the logic,
and we should catch it, not silently ignoring it.
Thus I believe the removal of the extra safety net should not be a
problem.
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
ROOT_REF and ROOT_BACKREF items contain a struct btrfs_root_ref followed
by the subvolume name. Several readers assume that this layout is already
valid and then use the on-disk name length directly. A corrupted item can
therefore make those readers address bytes outside the item, and
BTRFS_IOC_GET_SUBVOL_INFO can copy too many bytes into its fixed-size UAPI
name buffer.
Validate ROOT_REF and ROOT_BACKREF items in tree-checker before any reader
uses them. Reject records that do not contain a non-empty name, whose
name_len does not exactly describe the remaining item payload, or whose
name exceeds BTRFS_NAME_LEN.
For BTRFS_IOC_GET_SUBVOL_INFO, copy only the validated on-disk name_len
instead of deriving the copy length from the item size. The ioctl result is
zeroed when allocated. That leaves the existing trailing zero byte
untouched.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
btrfs_load_free_space_tree() reads FREE_SPACE_INFO once and then chooses
the bitmap or extent loader for all following free-space records until the
next FREE_SPACE_INFO item. Those loaders currently enforce the selected
record type only with ASSERT().
On production builds without CONFIG_BTRFS_ASSERT, a malformed free-space
tree can therefore be decoded in the wrong mode. An EXTENT item can reach
btrfs_free_space_test_bit() as bitmap data, while a BITMAP item can be
added as a full free extent. The latter corrupts the in-memory free-space
cache and the former can read beyond the item payload.
Sanitizer validation reported:
general protection fault
Call trace:
assert_eb_folio_uptodate() (fs/btrfs/extent_io.c:4134)
extent_buffer_test_bit() (?:?)
btrfs_free_space_test_bit() (fs/btrfs/free-space-tree.c:518)
srso_alias_return_thunk() (arch/x86/include/asm/nospec-branch.h:375)
__entry_text_end() (?:?)
__asan_memcpy() (mm/kasan/shadow.c:103)
read_extent_buffer() (?:?)
load_free_space_bitmaps() (fs/btrfs/free-space-tree.c:1548)
btrfs_get_32() (fs/btrfs/free-space-tree.c:?)
btrfs_set_16() (fs/btrfs/free-space-tree.c:?)
kmem_cache_alloc_noprof() (?:?)
btrfs_load_free_space_tree() (fs/btrfs/free-space-tree.c:1685)
load_free_space_tree_for_test() (?:?)
rcu_disable_urgency_upon_qs() (kernel/rcu/tree.c:721)
vprintk_emit() (?:?)
__up_write() (kernel/locking/rwsem.c:1401)
clone_commit_root_for_test() (?:?)
test_extent_as_bitmap_mode_mismatch() (?:?)
kmem_cache_free() (?:?)
btrfs_free_path() (fs/btrfs/free-space-tree.c:1449)
__add_block_group_free_space() (fs/btrfs/free-space-tree.c:20)
run_test() (?:?)
do_raw_spin_unlock() (?:?)
btrfs_test_free_space_tree() (fs/btrfs/tests/free-space-tree-tests.c:547)
btrfs_test_qgroups() (fs/btrfs/tests/qgroup-tests.c:462)
btrfs_run_sanity_tests() (fs/btrfs/free-space-tree.c:?)
init_btrfs_fs() (fs/btrfs/super.c:2690)
do_one_initcall() (init/main.c:1382)
__kasan_kmalloc() (?:?)
rcu_is_watching() (?:?)
do_initcalls() (init/main.c:1457)
kernel_init_freeable() (init/main.c:1674)
kernel_init() (init/main.c:1584)
ret_from_fork() (?:?)
__switch_to() (?:?)
ret_from_fork_asm() (?:?)
Validate every post-info key before decoding it. Reject keys whose type
does not match the mode selected by FREE_SPACE_INFO, and reject keys
whose range extends past the block group, returning -EUCLEAN instead of
feeding the wrong record type to the bitmap or extent decoder.
Also reject zero-length FREE_SPACE_EXTENT items in tree-checker, matching
the existing FREE_SPACE_BITMAP zero-length check. This keeps the loader
range check simple and prevents a zero-length extent item from being a
valid on-disk free-space record.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The iterator is used only once and within build_backref_tree() so we can
avoid one allocation and place it on stack.
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The fs_info is available everywhere and we don't need to store it inside
a structure that is used within one function only, which is
build_backref_tree(). The size of btrfs_backref_iter is now 48 bytes.
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The btree inode is very different from regular data inodes, as the btree
inode is never exposed to user space operations.
All operations are either initiated by btrfs metadata operations, or MM
layer like memory pressure to release folios.
This means we never need to handle partial folio invalidation inside
btree_invalidate_folio().
With that said, we can slightly simplify the btree folio invalidation
by:
- Add ASSERT()s to make sure the range covers the whole folio
- Remove "if (start > end)" check
As the range always covers the full folio, that check is always
false and can be removed.
- Open code extent_invalidate_folio()
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The function extent_invalidate_folio() has only a single caller inside
btree_invalidate_folio().
There is no need to export such a function just for a single caller inside
another file.
Unexport extent_invalidate_folio() and move it to disk-io.c.
And since we're moving the code, update the commit to match the current
style, and remove the seemingly stale comment on the extent state
removal, it's better explained by the comment just before
btrfs_unlock_extent().
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
fill_holes() currently merges a punched hole with either the previous
or the next file extent item, but never both in the same call. When
holes are punched in a non-sequential order this leaves consecutive
hole items in the inode's subvolume tree that should have been collapsed
into a single one.
This is a minor metadata optimization that reduces the number of file
extent items when holes are punched in non-sequential order. While
having extra file extent items is harmless and has no functional
impact, reducing metadata overhead can benefit workloads with heavily
fragmented hole patterns.
For example:
fallocate -p -o 4K -l 4K ${FILE}
fallocate -p -o 12K -l 4K ${FILE}
fallocate -p -o 8K -l 4K ${FILE}
After the third punch the [4K, 8K) and [12K, 16K) holes become
adjacent to the new [8K, 12K) hole, but fill_holes() merges only one
side and leaves two separate hole items ([4K, 12K) and [12K, 16K))
instead of the expected single [4K, 16K) hole item.
Fix this by checking both path->slots[0] - 1 and path->slots[0] in one
pass:
- If only the previous slot is mergeable, extend it forward as
before.
- If only the next slot is mergeable, extend it backward and update
its key offset as before.
- If both are mergeable, extend the previous item to cover the new
hole plus the next item, and remove the redundant next item with
btrfs_del_items().
Because the merge path may now delete an item, switch the initial
btrfs_search_slot() call from a plain lookup (ins_len = 0) to a
search-for-deletion (ins_len = -1), so the leaf is prepared for a
possible item removal.
Note: This optimization only applies to filesystems without the
NO_HOLES feature enabled. Since NO_HOLES is now the default, this
primarily benefits older filesystems or those explicitly created with
NO_HOLES disabled.
Signed-off-by: Dave Chen <davechen@synology.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
When we unmount the fs or during mount failures, btrfs will call
invalidate_inode_pages() to release all btree inode folios.
However that function can return -EBUSY if any folios can not be
invalidated.
This can be caused by:
- Some extent buffers are still held by btrfs
This is a logic error, as we should release all tree root nodes
during unmount and mount failure handling.
- Some extent buffers are under readahead and haven't yet finished
These are much rarer but valid cases.
In that case we should wait for those extent buffers.
Introduce a new helper invalidate_and_check_btree_folios() which will:
- Call invalidate_inode_pages2() and catch its return value
If it returned 0 as expected, that's great and we can call it a day.
- Otherwise go through each extent buffer in buffer_tree
Increase the ref by one first for the eb we're checking.
This is to ensure the eb won't be freed after the readahead is
finished.
For ebs that still have EXTENT_BUFFER_READING flag, wait for them to
finish first.
After waiting for the readahead, check the refs of the eb and if it's
still dirty.
If the eb ref count is greater than 2 (one for the buffer tree, one
held by us), it means we are still holding the extent buffer somewhere
else, which is a code bug.
If the eb is still dirty, it means a bug in transaction handling, e.g.
the bug fixed by patch "btrfs: only release the dirty pages io tree
after successful writes".
For either case, show a warning message about the eb, including its
bytenr, owner, refs and flags.
And if it's a debug build, also trigger WARN_ON_ONCE() so that fstests
can properly catch such situation.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=221270
Reported-by: AHN SEOK-YOUNG <iamsyahn@gmail.com>
CC: Teng Liu <27rabbitlt@gmail.com>
Tested-by: Teng Liu <27rabbitlt@gmail.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
If report_rb_range() is inlined into its single caller (check_eb_range()),
we end up with a larger module size, which is undesirable and does not
provide any advantage since this code is for a cold path which we don't
expect to ever hit.
Add the noinline attribute to report_rb_range() and while at it also make
it return void as it always returns true.
Before this change (with gcc 14.2.0-19 from Debian):
$ size fs/btrfs/btrfs.ko
text data bss dec hex filename
2018267 176232 15592 2210091 21b92b fs/btrfs/btrfs.ko
After this change:
$ size fs/btrfs/btrfs.ko
text data bss dec hex filename
2017835 176048 15592 2209475 21b6c3 fs/btrfs/btrfs.ko
Also, replacing the noinline with __cold, yields slighty worse results:
$ size fs/btrfs/btrfs.ko
text data bss dec hex filename
2017889 176048 15592 2209529 21b6f9 fs/btrfs/btrfs.ko
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
The btrfs_abort_transaction() is called at the location where we want to
report the abort. It must be a macro so we get the correct line and
stack trace. This inlines the necessary code and the rest is pushed to
__btrfs_abort_transaction().
There's a possibility to reduce the inlined code if we move the message
to the helper function as well, without loss of information. The
difference is only that the WARN will not print it inside the stack
report but after:
--[ cut here ]--
WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
...
--[ end trace ] --
BTRFS error (device dm-0 state A): Transaction aborted (error -28)
While previously there would be one more line like:
--[ cut here ]--
BTRFS: Transaction aborted (error -28)
WARNING: fs/btrfs/transaction.c:2045 at btrfs_commit_transaction+0xa21/0xd30 [btrfs], CPU#11: bonnie++/3377975
...
--[ end trace ] --
This removes about 20KiB of btrfs.ko on a release config.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
|
Before btrfs switched to the new mount API in 2023, we were setting
SB_NOSEC in btrfs_mount_root(). This flag tells the VFS that the
filesystem may have files which don't have security xattrs, enabling it
to do some optimizations.
Unfortunately this was missed in the transition, meaning that IS_NOSEC
will always return false for a btrfs inode. This means that
btrfs_direct_write() calls will always get the inode lock exclusively,
meaning that DIO writes to the same file will be serialized.
On my machine, this one-line change results in a ~59% improvement in DIO
throughput:
Before patch:
test: (g=0): rw=randwrite, bs=(R) 4096B-4096B, (W) 4096B-4096B, (T) 4096B-4096B, ioengine=io_uring, iodepth=64
...
fio-3.39
Starting 32 processes
test: Laying out IO file (1 file / 1024MiB)
Jobs: 32 (f=32): [w(32)][100.0%][w=764MiB/s][w=195k IOPS][eta 00m:00s]
test: (groupid=0, jobs=32): err= 0: pid=586: Wed Apr 22 13:03:04 2026
write: IOPS=202k, BW=787MiB/s (826MB/s)(46.1GiB/60012msec); 0 zone resets
bw ( KiB/s): min=498714, max=1199892, per=100.00%, avg=806659.03, stdev=4229.94, samples=3808
iops : min=124677, max=299971, avg=201661.82, stdev=1057.49, samples=3808
cpu : usr=0.32%, sys=1.27%, ctx=8329204, majf=0, minf=1163
IO depths : 1=0.0%, 2=0.0%, 4=0.0%, 8=0.0%, 16=0.0%, 32=0.0%, >=64=100.0%
submit : 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.0%
complete : 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.1%, >=64=0.0%
issued rwts: total=0,12094328,0,0 short=0,0,0,0 dropped=0,0,0,0
latency : target=0, window=0, percentile=100.00%, depth=64
Run status group 0 (all jobs):
WRITE: bw=787MiB/s (826MB/s), 787MiB/s-787MiB/s (826MB/s-826MB/s), io=46.1GiB (49.5GB), run=60012-60012msec
After patch:
test: (g=0): rw=randwrite, bs=(R) 4096B-4096B, (W) 4096B-4096B, (T) 4096B-4096B, ioengine=io_uring, iodepth=64
...
fio-3.39
Starting 32 processes
test: Laying out IO file (1 file / 1024MiB)
Jobs: 32 (f=32): [w(32)][100.0%][w=1255MiB/s][w=321k IOPS][eta 00m:00s]
test: (groupid=0, jobs=32): err= 0: pid=572: Wed Apr 22 13:13:46 2026
write: IOPS=320k, BW=1250MiB/s (1311MB/s)(73.3GiB/60003msec); 0 zone resets
bw ( MiB/s): min= 619, max= 2289, per=100.00%, avg=1251.28, stdev= 9.64, samples=3808
iops : min=158538, max=586025, avg=320320.80, stdev=2468.97, samples=3808
cpu : usr=0.35%, sys=11.50%, ctx=1584847, majf=0, minf=1160
IO depths : 1=0.0%, 2=0.0%, 4=0.0%, 8=0.0%, 16=0.0%, 32=0.0%, >=64=100.0%
submit : 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.0%, >=64=0.0%
complete : 0=0.0%, 4=100.0%, 8=0.0%, 16=0.0%, 32=0.0%, 64=0.1%, >=64=0.0%
issued rwts: total=0,19203309,0,0 short=0,0,0,0 dropped=0,0,0,0
latency : target=0, window=0, percentile=100.00%, depth=64
Run status group 0 (all jobs):
WRITE: bw=1250MiB/s (1311MB/s), 1250MiB/s-1250MiB/s (1311MB/s-1311MB/s), io=73.3GiB (78.7GB), run=60003-60003msec
The script to reproduce that:
#!/bin/bash
mkfs.btrfs -f /dev/nvme0n1
mount /dev/nvme0n1 /mnt/test
mkdir /mnt/test/nocow
chattr +C /mnt/test/nocow
fio /root/test.fio
# cat /root/test.fio
[global]
rw=randwrite
ioengine=io_uring
iodepth=64
size=1g
direct=1
startdelay=20
force_async=4
ramp_time=5
runtime=60
group_reporting=1
numjobs=32
time_based
disk_util=0
clat_percentiles=0
disable_lat=1
disable_clat=1
disable_slat=1
filename=/mnt/test/nocow/fiofile
[test]
name=test
bs=4k
stonewall
This was on a VM with 8 cores and 8GB of RAM, with a real NVMe exposed
through PCI passthrough. The figures for XFS and ext4 in comparison are
both about ~3GB/s.
Fixes: ad21f15b0f79 ("btrfs: switch to the new mount API")
Signed-off-by: Mark Harmstone <mark@harmstone.com>
Signed-off-by: David Sterba <dsterba@suse.com>
|
