* [PATCH v5 23/25] xfs: introduce health state for corrupted fsverity metadata
From: Andrey Albershteyn @ 2026-03-19 17:02 UTC (permalink / raw)
To: linux-xfs, fsverity, linux-fsdevel, ebiggers
Cc: Andrey Albershteyn, hch, linux-ext4, linux-f2fs-devel,
linux-btrfs, djwong
In-Reply-To: <20260319170231.1455553-1-aalbersh@kernel.org>
Report corrupted fsverity descriptor through health system.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_fs.h | 1 +
fs/xfs/libxfs/xfs_health.h | 4 +++-
fs/xfs/xfs_fsverity.c | 13 ++++++++++---
fs/xfs/xfs_health.c | 1 +
4 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index ebf17a0b0722..cece31ecee81 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -422,6 +422,7 @@ struct xfs_bulkstat {
#define XFS_BS_SICK_SYMLINK (1 << 6) /* symbolic link remote target */
#define XFS_BS_SICK_PARENT (1 << 7) /* parent pointers */
#define XFS_BS_SICK_DIRTREE (1 << 8) /* directory tree structure */
+#define XFS_BS_SICK_FSVERITY (1 << 9) /* fsverity metadata */
/*
* Project quota id helpers (previously projid was 16bit only
diff --git a/fs/xfs/libxfs/xfs_health.h b/fs/xfs/libxfs/xfs_health.h
index 1d45cf5789e8..932b447190da 100644
--- a/fs/xfs/libxfs/xfs_health.h
+++ b/fs/xfs/libxfs/xfs_health.h
@@ -104,6 +104,7 @@ struct xfs_rtgroup;
/* Don't propagate sick status to ag health summary during inactivation */
#define XFS_SICK_INO_FORGET (1 << 12)
#define XFS_SICK_INO_DIRTREE (1 << 13) /* directory tree structure */
+#define XFS_SICK_INO_FSVERITY (1 << 14) /* fsverity metadata */
/* Primary evidence of health problems in a given group. */
#define XFS_SICK_FS_PRIMARY (XFS_SICK_FS_COUNTERS | \
@@ -140,7 +141,8 @@ struct xfs_rtgroup;
XFS_SICK_INO_XATTR | \
XFS_SICK_INO_SYMLINK | \
XFS_SICK_INO_PARENT | \
- XFS_SICK_INO_DIRTREE)
+ XFS_SICK_INO_DIRTREE | \
+ XFS_SICK_INO_FSVERITY)
#define XFS_SICK_INO_ZAPPED (XFS_SICK_INO_BMBTD_ZAPPED | \
XFS_SICK_INO_BMBTA_ZAPPED | \
diff --git a/fs/xfs/xfs_fsverity.c b/fs/xfs/xfs_fsverity.c
index b193009a1bdb..ecc66ee8bac5 100644
--- a/fs/xfs/xfs_fsverity.c
+++ b/fs/xfs/xfs_fsverity.c
@@ -84,16 +84,23 @@ xfs_fsverity_get_descriptor(
return error;
desc_size = be32_to_cpu(d_desc_size);
- if (XFS_IS_CORRUPT(mp, desc_size > FS_VERITY_MAX_DESCRIPTOR_SIZE))
+ if (XFS_IS_CORRUPT(mp, desc_size > FS_VERITY_MAX_DESCRIPTOR_SIZE)) {
+ xfs_inode_mark_sick(XFS_I(inode), XFS_SICK_INO_FSVERITY);
return -ERANGE;
- if (XFS_IS_CORRUPT(mp, desc_size > desc_size_pos))
+ }
+
+ if (XFS_IS_CORRUPT(mp, desc_size > desc_size_pos)) {
+ xfs_inode_mark_sick(XFS_I(inode), XFS_SICK_INO_FSVERITY);
return -ERANGE;
+ }
if (!buf_size)
return desc_size;
- if (XFS_IS_CORRUPT(mp, desc_size > buf_size))
+ if (XFS_IS_CORRUPT(mp, desc_size > buf_size)) {
+ xfs_inode_mark_sick(XFS_I(inode), XFS_SICK_INO_FSVERITY);
return -ERANGE;
+ }
desc_pos = round_down(desc_size_pos - desc_size, blocksize);
error = fsverity_pagecache_read(inode, buf, desc_size, desc_pos);
diff --git a/fs/xfs/xfs_health.c b/fs/xfs/xfs_health.c
index 239b843e83d4..be66760fb120 100644
--- a/fs/xfs/xfs_health.c
+++ b/fs/xfs/xfs_health.c
@@ -625,6 +625,7 @@ static const struct ioctl_sick_map ino_map[] = {
{ XFS_SICK_INO_DIR_ZAPPED, XFS_BS_SICK_DIR },
{ XFS_SICK_INO_SYMLINK_ZAPPED, XFS_BS_SICK_SYMLINK },
{ XFS_SICK_INO_DIRTREE, XFS_BS_SICK_DIRTREE },
+ { XFS_SICK_INO_FSVERITY, XFS_BS_SICK_FSVERITY },
};
/* Fill out bulkstat health info. */
--
2.51.2
^ permalink raw reply related
* [PATCH v5 24/25] xfs: add fsverity traces
From: Andrey Albershteyn @ 2026-03-19 17:02 UTC (permalink / raw)
To: linux-xfs, fsverity, linux-fsdevel, ebiggers
Cc: Andrey Albershteyn, hch, linux-ext4, linux-f2fs-devel,
linux-btrfs, djwong
In-Reply-To: <20260319170231.1455553-1-aalbersh@kernel.org>
Even though fsverity has traces, debugging issues with varying block
sizes could be a bit less transparent without read/write traces.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/xfs/xfs_fsverity.c | 6 ++++++
fs/xfs/xfs_trace.h | 46 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)
diff --git a/fs/xfs/xfs_fsverity.c b/fs/xfs/xfs_fsverity.c
index ecc66ee8bac5..6db8113da52f 100644
--- a/fs/xfs/xfs_fsverity.c
+++ b/fs/xfs/xfs_fsverity.c
@@ -64,6 +64,8 @@ xfs_fsverity_get_descriptor(
uint32_t blocksize = i_blocksize(VFS_I(ip));
xfs_fileoff_t last_block_offset;
+ trace_xfs_fsverity_get_descriptor(ip);
+
ASSERT(inode->i_flags & S_VERITY);
error = xfs_bmap_last_extent(NULL, ip, XFS_DATA_FORK, &rec, &is_empty);
if (error)
@@ -377,6 +379,7 @@ xfs_fsverity_read_merkle(
pgoff_t index)
{
index += xfs_fsverity_metadata_offset(XFS_I(inode)) >> PAGE_SHIFT;
+ trace_xfs_fsverity_read_merkle(XFS_I(inode), index, PAGE_SIZE);
return generic_read_merkle_tree_page(inode, index);
}
@@ -391,6 +394,7 @@ xfs_fsverity_readahead_merkle_tree(
unsigned long nr_pages)
{
index += xfs_fsverity_metadata_offset(XFS_I(inode)) >> PAGE_SHIFT;
+ trace_xfs_fsverity_read_merkle(XFS_I(inode), index, PAGE_SIZE);
generic_readahead_merkle_tree(inode, index, nr_pages);
}
@@ -414,6 +418,8 @@ xfs_fsverity_write_merkle(
const char *p;
unsigned int i;
+ trace_xfs_fsverity_write_merkle(XFS_I(inode), position, size);
+
if (position + size > inode->i_sb->s_maxbytes)
return -EFBIG;
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 813e5a9f57eb..d37f3096c03e 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -6419,6 +6419,52 @@ TRACE_EVENT(xfs_verify_media_error,
__entry->error)
);
+TRACE_EVENT(xfs_fsverity_get_descriptor,
+ TP_PROTO(struct xfs_inode *ip),
+ TP_ARGS(ip),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(xfs_ino_t, ino)
+ ),
+ TP_fast_assign(
+ __entry->dev = VFS_I(ip)->i_sb->s_dev;
+ __entry->ino = ip->i_ino;
+ ),
+ TP_printk("dev %d:%d ino 0x%llx",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->ino)
+);
+
+DECLARE_EVENT_CLASS(xfs_fsverity_class,
+ TP_PROTO(struct xfs_inode *ip, u64 pos, size_t length),
+ TP_ARGS(ip, pos, length),
+ TP_STRUCT__entry(
+ __field(dev_t, dev)
+ __field(xfs_ino_t, ino)
+ __field(u64, pos)
+ __field(size_t, length)
+ ),
+ TP_fast_assign(
+ __entry->dev = VFS_I(ip)->i_sb->s_dev;
+ __entry->ino = ip->i_ino;
+ __entry->pos = pos;
+ __entry->length = length;
+ ),
+ TP_printk("dev %d:%d ino 0x%llx pos 0x%llx length 0x%zx",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->ino,
+ __entry->pos,
+ __entry->length)
+)
+
+#define DEFINE_FSVERITY_EVENT(name) \
+DEFINE_EVENT(xfs_fsverity_class, name, \
+ TP_PROTO(struct xfs_inode *ip, u64 pos, size_t length), \
+ TP_ARGS(ip, pos, length))
+DEFINE_FSVERITY_EVENT(xfs_fsverity_read_merkle);
+DEFINE_FSVERITY_EVENT(xfs_fsverity_write_merkle);
+DEFINE_FSVERITY_EVENT(xfs_fsverity_file_corrupt);
+
#endif /* _TRACE_XFS_H */
#undef TRACE_INCLUDE_PATH
--
2.51.2
^ permalink raw reply related
* [PATCH v5 25/25] xfs: enable ro-compat fs-verity flag
From: Andrey Albershteyn @ 2026-03-19 17:02 UTC (permalink / raw)
To: linux-xfs, fsverity, linux-fsdevel, ebiggers
Cc: Andrey Albershteyn, hch, linux-ext4, linux-f2fs-devel,
linux-btrfs, djwong
In-Reply-To: <20260319170231.1455553-1-aalbersh@kernel.org>
Finalize fs-verity integration in XFS by making kernel fs-verity
aware with ro-compat flag.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
[djwong: add spaces]
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_format.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 4dff29659e40..0ce46c234b9c 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -378,8 +378,9 @@ xfs_sb_has_compat_feature(
#define XFS_SB_FEAT_RO_COMPAT_ALL \
(XFS_SB_FEAT_RO_COMPAT_FINOBT | \
XFS_SB_FEAT_RO_COMPAT_RMAPBT | \
- XFS_SB_FEAT_RO_COMPAT_REFLINK| \
- XFS_SB_FEAT_RO_COMPAT_INOBTCNT)
+ XFS_SB_FEAT_RO_COMPAT_REFLINK | \
+ XFS_SB_FEAT_RO_COMPAT_INOBTCNT | \
+ XFS_SB_FEAT_RO_COMPAT_VERITY)
#define XFS_SB_FEAT_RO_COMPAT_UNKNOWN ~XFS_SB_FEAT_RO_COMPAT_ALL
static inline bool
xfs_sb_has_ro_compat_feature(
--
2.51.2
^ permalink raw reply related
* Re: [PATCH v5 01/25] fsverity: report validation errors through fserror to fsnotify
From: Darrick J. Wong @ 2026-03-19 17:15 UTC (permalink / raw)
To: Andrey Albershteyn
Cc: linux-xfs, fsverity, linux-fsdevel, ebiggers, hch, linux-ext4,
linux-f2fs-devel, linux-btrfs
In-Reply-To: <20260319170231.1455553-2-aalbersh@kernel.org>
On Thu, Mar 19, 2026 at 06:01:48PM +0100, Andrey Albershteyn wrote:
> Reported verification errors to fsnotify through recently added fserror
> interface.
>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
Looks good now,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/verity/verify.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/verity/verify.c b/fs/verity/verify.c
> index 4004a1d42875..4ea494da694f 100644
> --- a/fs/verity/verify.c
> +++ b/fs/verity/verify.c
> @@ -9,6 +9,7 @@
>
> #include <linux/bio.h>
> #include <linux/export.h>
> +#include <linux/fserror.h>
>
> #define FS_VERITY_MAX_PENDING_BLOCKS 2
>
> @@ -203,6 +204,8 @@ static bool verify_data_block(struct fsverity_info *vi,
> * to verify that any data blocks fully past EOF are all zeroes.
> */
> if (memchr_inv(dblock->data, 0, params->block_size)) {
> + fserror_report_data_lost(inode, data_pos,
> + params->block_size, GFP_NOFS);
> fsverity_err(inode,
> "FILE CORRUPTED! Data past EOF is not zeroed");
> return false;
> @@ -312,6 +315,7 @@ static bool verify_data_block(struct fsverity_info *vi,
> data_pos, level - 1, params->hash_alg->name, hsize, want_hash,
> params->hash_alg->name, hsize,
> level == 0 ? dblock->real_hash : real_hash);
> + fserror_report_data_lost(inode, data_pos, params->block_size, GFP_NOFS);
> error:
> for (; level > 0; level--) {
> kunmap_local(hblocks[level - 1].addr);
> --
> 2.51.2
>
>
^ permalink raw reply
* Re: [PATCH 2/3] ext4: add ext4_fc_eligible()
From: NeilBrown @ 2026-03-19 23:31 UTC (permalink / raw)
To: Jan Kara
Cc: Theodore Ts'o, Andreas Dilger, Jan Kara, linux-ext4,
linux-fsdevel
In-Reply-To: <gh7e7uykczpyhfxrh6y54c2jlcs4pzcud65aqsjoj4am6cajzp@wq26crdxv2yd>
On Thu, 19 Mar 2026, Jan Kara wrote:
> On Wed 18-03-26 09:39:50, NeilBrown wrote:
> > From: NeilBrown <neil@brown.name>
> >
> > Testing EXT4_MF_FC_INELIGIBLE is almost always combined with testing
> > ext4_fc_disabled(). The code can be simplified by combining these two
> > in a new ext4_fc_eligible().
> >
> > In ext4_fc_track_inode() this moves the ext4_fc_disabled() test after
> > ext4_fc_mark_ineligible(), but as that is a non-op when
> > ext4_fc_disabled() is true, this is no no consequence.
> >
> > Signed-off-by: NeilBrown <neil@brown.name>
>
> One nit below, otherwise feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> > @@ -557,16 +548,13 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
> > if (S_ISDIR(inode->i_mode))
> > return;
> >
> > - if (ext4_fc_disabled(inode->i_sb))
> > - return;
> > -
> > if (ext4_should_journal_data(inode)) {
> > ext4_fc_mark_ineligible(inode->i_sb,
> > EXT4_FC_REASON_INODE_JOURNAL_DATA, handle);
> > return;
> > }
> >
> > - if (ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE))
> > + if (!ext4_fc_eligible(inode->i_sb))
> > return;
>
> Here I think the !ext4_fc_eligible() check could be actually above the
> ext4_should_journal_data() check - if the fs is not eligible for
> fastcommit, there's no point in marking it ineligible again...
Both you and Andreas have questioned that choice - so I should explain
my reasoning.
ext4_fc_mark_ineligible() is NOT a no-op when the sb is already marked
ineligible. The code updates sb->s_fc_ineligible_tid to the largest tid
which was ineligible for fc. Then it only clears the "ineligible" flag
after that highest numbered transaction has committed. If we skip
ext4_fc_mark_ineligible() because EXT4_MF_FC_INELIGIBLE is already set,
then that flag could be cleared too early.
Thanks,
NeilBrown
^ permalink raw reply
* (no subject)
From: NeilBrown @ 2026-03-20 0:03 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Jan Kara, Harshad Shirwadkar
Cc: linux-ext4, linux-fsdevel
This new version
- includes some reviewed-bys
- adds an explanation to second patch of why ext4_fc_eligible() was
placed where it was in one case
- modifies the last patch (but keeps the Reviews - hope that is OK)
to keep the changes to ->i_nlink in __ext4_link
https://sashiko.dev/#/patchset/20260317224638.3809014-1-neilb%40ownmail.net
points out that removing it for the replay path might not be correct,
and I cannot now see any reason to move those changes (ext4_inc_count()
and drop_nlink())
From previous email:
My particular interest in changing ext4 is to remove the use of
d_alloc(). I will want to deprecate d_alloc() as it doesn't fit the new
model well.
The use of d_alloc() in ext4 is incidental to the actual task at hand.
The code really wants to pass around a parent directory and a name, and
only uses the dentry because that seems convenient. As these patches
show the code actually becomes simpler when we avoid the dentry.
NeilBrown
[PATCH v2 1/3] ext4: split __ext4_add_entry() out of ext4_add_entry()
[PATCH v2 2/3] ext4: add ext4_fc_eligible()
[PATCH v2 3/3] ext4: move dcache manipulation out of __ext4_link()
^ permalink raw reply
* [PATCH v2 1/3] ext4: split __ext4_add_entry() out of ext4_add_entry()
From: NeilBrown @ 2026-03-20 0:03 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Jan Kara, Harshad Shirwadkar
Cc: linux-ext4, linux-fsdevel
In-Reply-To: <20260320000838.3797494-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
__ext4_add_entry() is not given a dentry - just inodes and name.
This will help the next patch which simplifies __ex4_link().
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/ext4/namei.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index c4b5e252af0e..768036a109d7 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2353,10 +2353,10 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
* may not sleep between calling this and putting something into
* the entry, as someone else might have used it while you slept.
*/
-static int ext4_add_entry(handle_t *handle, struct dentry *dentry,
+static int __ext4_add_entry(handle_t *handle, struct inode *dir,
+ const struct qstr *d_name,
struct inode *inode)
{
- struct inode *dir = d_inode(dentry->d_parent);
struct buffer_head *bh = NULL;
struct ext4_dir_entry_2 *de;
struct super_block *sb;
@@ -2373,13 +2373,10 @@ static int ext4_add_entry(handle_t *handle, struct dentry *dentry,
sb = dir->i_sb;
blocksize = sb->s_blocksize;
- if (fscrypt_is_nokey_name(dentry))
- return -ENOKEY;
-
- if (!generic_ci_validate_strict_name(dir, &dentry->d_name))
+ if (!generic_ci_validate_strict_name(dir, d_name))
return -EINVAL;
- retval = ext4_fname_setup_filename(dir, &dentry->d_name, 0, &fname);
+ retval = ext4_fname_setup_filename(dir, d_name, 0, &fname);
if (retval)
return retval;
@@ -2460,6 +2457,16 @@ static int ext4_add_entry(handle_t *handle, struct dentry *dentry,
return retval;
}
+static int ext4_add_entry(handle_t *handle, struct dentry *dentry,
+ struct inode *inode)
+{
+ struct inode *dir = d_inode(dentry->d_parent);
+
+ if (fscrypt_is_nokey_name(dentry))
+ return -ENOKEY;
+ return __ext4_add_entry(handle, dir, &dentry->d_name, inode);
+}
+
/*
* Returns 0 for success, or a negative error value
*/
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH v2 2/3] ext4: add ext4_fc_eligible()
From: NeilBrown @ 2026-03-20 0:03 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Jan Kara, Harshad Shirwadkar
Cc: linux-ext4, linux-fsdevel
In-Reply-To: <20260320000838.3797494-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
Testing EXT4_MF_FC_INELIGIBLE is almost always combined with testing
ext4_fc_disabled(). The code can be simplified by combining these two
in a new ext4_fc_eligible().
In ext4_fc_track_inode() this moves the ext4_fc_disabled() test after
ext4_fc_mark_ineligible(), but as that is a non-op when
ext4_fc_disabled() is true, this is no no consequence.
Note that it is important to still call ext4_fc_mark_ineligible() in
ext4_fc_track_inode() even when ext4_fc_eligible() would return true.
ext4_fc_mark_ineligible() does not ONLY set the "INELIGIBLE" flag but
also updates ->s_fc_ineligible_tid to make sure that the flag remains
set until all ineligible transactions have been committed.
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/ext4/fast_commit.c | 43 ++++++++++++++-----------------------------
1 file changed, 14 insertions(+), 29 deletions(-)
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index f575751f1cae..3ee302b73f45 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -224,6 +224,12 @@ static bool ext4_fc_disabled(struct super_block *sb)
(EXT4_SB(sb)->s_mount_state & EXT4_FC_REPLAY));
}
+static bool ext4_fc_eligible(struct super_block *sb)
+{
+ return !ext4_fc_disabled(sb) &&
+ !(ext4_test_mount_flag(sb, EXT4_MF_FC_INELIGIBLE));
+}
+
/*
* Remove inode from fast commit list. If the inode is being committed
* we wait until inode commit is done.
@@ -473,13 +479,8 @@ void ext4_fc_track_unlink(handle_t *handle, struct dentry *dentry)
{
struct inode *inode = d_inode(dentry);
- if (ext4_fc_disabled(inode->i_sb))
- return;
-
- if (ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE))
- return;
-
- __ext4_fc_track_unlink(handle, inode, dentry);
+ if (ext4_fc_eligible(inode->i_sb))
+ __ext4_fc_track_unlink(handle, inode, dentry);
}
void __ext4_fc_track_link(handle_t *handle,
@@ -500,13 +501,8 @@ void ext4_fc_track_link(handle_t *handle, struct dentry *dentry)
{
struct inode *inode = d_inode(dentry);
- if (ext4_fc_disabled(inode->i_sb))
- return;
-
- if (ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE))
- return;
-
- __ext4_fc_track_link(handle, inode, dentry);
+ if (ext4_fc_eligible(inode->i_sb))
+ __ext4_fc_track_link(handle, inode, dentry);
}
void __ext4_fc_track_create(handle_t *handle, struct inode *inode,
@@ -527,13 +523,8 @@ void ext4_fc_track_create(handle_t *handle, struct dentry *dentry)
{
struct inode *inode = d_inode(dentry);
- if (ext4_fc_disabled(inode->i_sb))
- return;
-
- if (ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE))
- return;
-
- __ext4_fc_track_create(handle, inode, dentry);
+ if (ext4_fc_eligible(inode->i_sb))
+ __ext4_fc_track_create(handle, inode, dentry);
}
/* __track_fn for inode tracking */
@@ -557,16 +548,13 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
if (S_ISDIR(inode->i_mode))
return;
- if (ext4_fc_disabled(inode->i_sb))
- return;
-
if (ext4_should_journal_data(inode)) {
ext4_fc_mark_ineligible(inode->i_sb,
EXT4_FC_REASON_INODE_JOURNAL_DATA, handle);
return;
}
- if (ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE))
+ if (!ext4_fc_eligible(inode->i_sb))
return;
/*
@@ -644,10 +632,7 @@ void ext4_fc_track_range(handle_t *handle, struct inode *inode, ext4_lblk_t star
if (S_ISDIR(inode->i_mode))
return;
- if (ext4_fc_disabled(inode->i_sb))
- return;
-
- if (ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE))
+ if (!ext4_fc_eligible(inode->i_sb))
return;
if (ext4_has_inline_data(inode)) {
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* [PATCH v2 3/3] ext4: move dcache manipulation out of __ext4_link()
From: NeilBrown @ 2026-03-20 0:03 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Jan Kara, Harshad Shirwadkar
Cc: linux-ext4, linux-fsdevel
In-Reply-To: <20260320000838.3797494-1-neilb@ownmail.net>
From: NeilBrown <neil@brown.name>
__ext4_link() has two callers.
- ext4_link() calls it during normal handling of the link() system
call or similar
- ext4_fc_replay_link_internal() calls it when replaying the journal
at mount time.
The former needs changes to dcache - instantiating the dentry to the
inode on success. The latter doesn't need or want any dcache
manipulation.
So move the manipulation out of __ext4_link() and do it in ext4_link()
only.
This requires:
- passing the qname from the dentry explicitly to __ext4_link.
The parent dir is already passed. The dentry is still passed
in the ext4_link() case purely for use by ext4_fc_track_link().
- passing the inode separately to ext4_fc_track_link() as the
dentry will not be instantiated yet.
- using __ext4_add_entry() in ext4_link, which doesn't need a dentry.
- moving ihold(), d_instantiate(), drop_nlink() and iput() calls out
of __ext4_link() into ext4_link().
Note that ext4_inc_count() and drop_nlink() remain in __ext4_link()
as both callers need them and they are not related to the dentry.
This substantially simplifies ext4_fc_replay_link_internal(), and
removes a use of d_alloc() which, it is planned, will be removed.
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: NeilBrown <neil@brown.name>
---
fs/ext4/ext4.h | 5 +++--
fs/ext4/fast_commit.c | 32 ++++----------------------------
fs/ext4/namei.c | 19 +++++++++++--------
3 files changed, 18 insertions(+), 38 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 293f698b7042..e757e9cf9728 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2974,7 +2974,8 @@ void __ext4_fc_track_unlink(handle_t *handle, struct inode *inode,
void __ext4_fc_track_link(handle_t *handle, struct inode *inode,
struct dentry *dentry);
void ext4_fc_track_unlink(handle_t *handle, struct dentry *dentry);
-void ext4_fc_track_link(handle_t *handle, struct dentry *dentry);
+void ext4_fc_track_link(handle_t *handle, struct inode *inode,
+ struct dentry *dentry);
void __ext4_fc_track_create(handle_t *handle, struct inode *inode,
struct dentry *dentry);
void ext4_fc_track_create(handle_t *handle, struct dentry *dentry);
@@ -3719,7 +3720,7 @@ extern int ext4_handle_dirty_dirblock(handle_t *handle, struct inode *inode,
extern int __ext4_unlink(struct inode *dir, const struct qstr *d_name,
struct inode *inode, struct dentry *dentry);
extern int __ext4_link(struct inode *dir, struct inode *inode,
- struct dentry *dentry);
+ const struct qstr *d_name, struct dentry *dentry);
#define S_SHIFT 12
static const unsigned char ext4_type_by_mode[(S_IFMT >> S_SHIFT) + 1] = {
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 3ee302b73f45..175dda11f377 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -497,10 +497,9 @@ void __ext4_fc_track_link(handle_t *handle,
trace_ext4_fc_track_link(handle, inode, dentry, ret);
}
-void ext4_fc_track_link(handle_t *handle, struct dentry *dentry)
+void ext4_fc_track_link(handle_t *handle, struct inode *inode,
+ struct dentry *dentry)
{
- struct inode *inode = d_inode(dentry);
-
if (ext4_fc_eligible(inode->i_sb))
__ext4_fc_track_link(handle, inode, dentry);
}
@@ -1431,7 +1430,6 @@ static int ext4_fc_replay_link_internal(struct super_block *sb,
struct inode *inode)
{
struct inode *dir = NULL;
- struct dentry *dentry_dir = NULL, *dentry_inode = NULL;
struct qstr qstr_dname = QSTR_INIT(darg->dname, darg->dname_len);
int ret = 0;
@@ -1442,21 +1440,7 @@ static int ext4_fc_replay_link_internal(struct super_block *sb,
goto out;
}
- dentry_dir = d_obtain_alias(dir);
- if (IS_ERR(dentry_dir)) {
- ext4_debug("Failed to obtain dentry");
- dentry_dir = NULL;
- goto out;
- }
-
- dentry_inode = d_alloc(dentry_dir, &qstr_dname);
- if (!dentry_inode) {
- ext4_debug("Inode dentry not created.");
- ret = -ENOMEM;
- goto out;
- }
-
- ret = __ext4_link(dir, inode, dentry_inode);
+ ret = __ext4_link(dir, inode, &qstr_dname, NULL);
/*
* It's possible that link already existed since data blocks
* for the dir in question got persisted before we crashed OR
@@ -1470,16 +1454,8 @@ static int ext4_fc_replay_link_internal(struct super_block *sb,
ret = 0;
out:
- if (dentry_dir) {
- d_drop(dentry_dir);
- dput(dentry_dir);
- } else if (dir) {
+ if (dir)
iput(dir);
- }
- if (dentry_inode) {
- d_drop(dentry_inode);
- dput(dentry_inode);
- }
return ret;
}
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 768036a109d7..20ec5674e64b 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -3452,7 +3452,8 @@ static int ext4_symlink(struct mnt_idmap *idmap, struct inode *dir,
return err;
}
-int __ext4_link(struct inode *dir, struct inode *inode, struct dentry *dentry)
+int __ext4_link(struct inode *dir, struct inode *inode,
+ const struct qstr *d_name, struct dentry *dentry)
{
handle_t *handle;
int err, retries = 0;
@@ -3468,9 +3469,8 @@ int __ext4_link(struct inode *dir, struct inode *inode, struct dentry *dentry)
inode_set_ctime_current(inode);
ext4_inc_count(inode);
- ihold(inode);
- err = ext4_add_entry(handle, dentry, inode);
+ err = __ext4_add_entry(handle, dir, d_name, inode);
if (!err) {
err = ext4_mark_inode_dirty(handle, inode);
/* this can happen only for tmpfile being
@@ -3478,11 +3478,10 @@ int __ext4_link(struct inode *dir, struct inode *inode, struct dentry *dentry)
*/
if (inode->i_nlink == 1)
ext4_orphan_del(handle, inode);
- d_instantiate(dentry, inode);
- ext4_fc_track_link(handle, dentry);
+ if (dentry)
+ ext4_fc_track_link(handle, inode, dentry);
} else {
drop_nlink(inode);
- iput(inode);
}
ext4_journal_stop(handle);
if (err == -ENOSPC && ext4_should_retry_alloc(dir->i_sb, &retries))
@@ -3511,9 +3510,13 @@ static int ext4_link(struct dentry *old_dentry,
err = dquot_initialize(dir);
if (err)
return err;
- return __ext4_link(dir, inode, dentry);
+ err = __ext4_link(dir, inode, &dentry->d_name, dentry);
+ if (!err) {
+ ihold(inode);
+ d_instantiate(dentry, inode);
+ }
+ return err;
}
-
/*
* Try to find buffer head where contains the parent block.
* It should be the inode block if it is inlined or the 1st block
--
2.50.0.107.gf914562f5916.dirty
^ permalink raw reply related
* Re: [PATCH v4 3/5] ext4: fix the error handling process in extents_kunit_init).
From: Ritesh Harjani @ 2026-03-20 1:56 UTC (permalink / raw)
To: Ye Bin, tytso, adilger.kernel, linux-ext4; +Cc: jack, Ojaswin Mujoo
In-Reply-To: <20260319125434.333117-4-yebin@huaweicloud.com>
Ye Bin <yebin@huaweicloud.com> writes:
> From: Ye Bin <yebin10@huawei.com>
>
> The error processing in extents_kunit_init() is improper, causing
> resource leakage.
> Reconstruct the error handling process to prevent potential resource
> leaks
>
> Fixes: cb1e0c1d1fad ("ext4: kunit tests for extent splitting and conversion")
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> ---
> fs/ext4/extents-test.c | 43 ++++++++++++++++++++++++++++++------------
> 1 file changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/fs/ext4/extents-test.c b/fs/ext4/extents-test.c
> index 3d4663d99eb1..4ce3f81f6409 100644
> --- a/fs/ext4/extents-test.c
> +++ b/fs/ext4/extents-test.c
> @@ -225,33 +225,37 @@ static int extents_kunit_init(struct kunit *test)
> (struct kunit_ext_test_param *)(test->param_value);
> int err;
>
> - sb = sget(&ext_fs_type, NULL, ext_set, 0, NULL);
> - if (IS_ERR(sb))
> - return PTR_ERR(sb);
> -
> - sb->s_blocksize = 4096;
> - sb->s_blocksize_bits = 12;
> -
> sbi = kzalloc_obj(struct ext4_sb_info);
> if (sbi == NULL)
> return -ENOMEM;
>
> + sb = sget(&ext_fs_type, NULL, ext_set, 0, NULL);
> + if (IS_ERR(sb)) {
> + kfree(sbi);
> + return PTR_ERR(sb);
> + }
> +
> sbi->s_sb = sb;
> sb->s_fs_info = sbi;
>
> + sb->s_blocksize = 4096;
> + sb->s_blocksize_bits = 12;
> +
> if (!param || !param->disable_zeroout)
> sbi->s_extent_max_zeroout_kb = 32;
>
> /* setup the mock inode */
> k_ctx.k_ei = kzalloc_obj(struct ext4_inode_info);
> - if (k_ctx.k_ei == NULL)
> - return -ENOMEM;
> + if (k_ctx.k_ei == NULL) {
> + err = -ENOMEM;
> + goto out_deactivate;
> + }
> ei = k_ctx.k_ei;
> inode = &ei->vfs_inode;
>
> err = ext4_es_register_shrinker(sbi);
> if (err)
> - return err;
> + goto out_deactivate;
Even though the patch looks ok, but still wanted to check if ...
Do you think we can move ext4_es_register_shrinker() before setting up
the mock inode and on error we simply return err?
That way, there won't be any ambiguity in the error handling for calling
ext4_es_unregister_shrinker()?
>
> ext4_es_init_tree(&ei->i_es_tree);
> rwlock_init(&ei->i_es_lock);
> @@ -267,8 +271,10 @@ static int extents_kunit_init(struct kunit *test)
> inode->i_sb = sb;
>
> k_ctx.k_data = kzalloc(EXT_DATA_LEN * 4096, GFP_KERNEL);
> - if (k_ctx.k_data == NULL)
> - return -ENOMEM;
> + if (k_ctx.k_data == NULL) {
> + err = -ENOMEM;
> + goto out_deactivate;
> + }
>
> /*
> * set the data area to a junk value
> @@ -313,6 +319,19 @@ static int extents_kunit_init(struct kunit *test)
> up_write(&sb->s_umount);
>
> return 0;
> +
> +out_deactivate:
> + kfree(k_ctx.k_ei);
> + k_ctx.k_ei = NULL;
> +
> + kfree(k_ctx.k_data);
> + k_ctx.k_data = NULL;
> +
> + ext4_es_unregister_shrinker(sbi);
-ritesh
^ permalink raw reply
* Re: [PATCH] ext4: Fix the might_sleep() warnings in kvfree()
From: Baokun Li @ 2026-03-20 2:47 UTC (permalink / raw)
To: Zqiang; +Cc: tytso, adilger.kernel, linux-ext4, linux-kernel, libaokun
In-Reply-To: <20260319094545.19291-1-qiang.zhang@linux.dev>
On 3/19/26 5:45 PM, Zqiang wrote:
> Use the kvfree() in the RCU read critical section can trigger
> the following warnings:
>
> EXT4-fs (vdb): unmounting filesystem cd983e5b-3c83-4f5a-a136-17b00eb9d018.
>
> WARNING: suspicious RCU usage
>
> ./include/linux/rcupdate.h:409 Illegal context switch in RCU read-side critical section!
>
> other info that might help us debug this:
>
> rcu_scheduler_active = 2, debug_locks = 1
>
> Call Trace:
> <TASK>
> dump_stack_lvl+0xbb/0xd0
> dump_stack+0x14/0x20
> lockdep_rcu_suspicious+0x15a/0x1b0
> __might_resched+0x375/0x4d0
> ? put_object.part.0+0x2c/0x50
> __might_sleep+0x108/0x160
> vfree+0x58/0x910
> ? ext4_group_desc_free+0x27/0x270
> kvfree+0x23/0x40
> ext4_group_desc_free+0x111/0x270
> ext4_put_super+0x3c8/0xd40
> generic_shutdown_super+0x14c/0x4a0
> ? __pfx_shrinker_free+0x10/0x10
> kill_block_super+0x40/0x90
> ext4_kill_sb+0x6d/0xb0
> deactivate_locked_super+0xb4/0x180
> deactivate_super+0x7e/0xa0
> cleanup_mnt+0x296/0x3e0
> __cleanup_mnt+0x16/0x20
> task_work_run+0x157/0x250
> ? __pfx_task_work_run+0x10/0x10
> ? exit_to_user_mode_loop+0x6a/0x550
> exit_to_user_mode_loop+0x102/0x550
> do_syscall_64+0x44a/0x500
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> </TASK>
>
> BUG: sleeping function called from invalid context at mm/vmalloc.c:3441
> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 556, name: umount
> preempt_count: 1, expected: 0
> CPU: 3 UID: 0 PID: 556 Comm: umount
> Call Trace:
> <TASK>
> dump_stack_lvl+0xbb/0xd0
> dump_stack+0x14/0x20
> __might_resched+0x275/0x4d0
> ? put_object.part.0+0x2c/0x50
> __might_sleep+0x108/0x160
> vfree+0x58/0x910
> ? ext4_group_desc_free+0x27/0x270
> kvfree+0x23/0x40
> ext4_group_desc_free+0x111/0x270
> ext4_put_super+0x3c8/0xd40
> generic_shutdown_super+0x14c/0x4a0
> ? __pfx_shrinker_free+0x10/0x10
> kill_block_super+0x40/0x90
> ext4_kill_sb+0x6d/0xb0
> deactivate_locked_super+0xb4/0x180
> deactivate_super+0x7e/0xa0
> cleanup_mnt+0x296/0x3e0
> __cleanup_mnt+0x16/0x20
> task_work_run+0x157/0x250
> ? __pfx_task_work_run+0x10/0x10
> ? exit_to_user_mode_loop+0x6a/0x550
> exit_to_user_mode_loop+0x102/0x550
> do_syscall_64+0x44a/0x500
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> The above scenarios occur in initialization failures and teardown
> paths, there are no parallel operations on the resources released
> by kvfree(), this commit therefore remove rcu_read_lock/unlock() and
> use rcu_access_pointer() instead of rcu_dereference() operations.
>
> Fixes: 7c990728b99e ("ext4: fix potential race between s_flex_groups online resizing and access")
> Fixes: df3da4ea5a0f ("ext4: fix potential race between s_group_info online resizing and access")
> Signed-off-by: Zqiang <qiang.zhang@linux.dev>
Looks good, feel free to add:
Reviewed-by: Baokun Li <libaokun@linux.alibaba.com>
> ---
> fs/ext4/mballoc.c | 10 +++-------
> fs/ext4/super.c | 8 ++------
> 2 files changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 20e9fdaf4301..e96513cc6151 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -3580,9 +3580,7 @@ static int ext4_mb_init_backend(struct super_block *sb)
> rcu_read_unlock();
> iput(sbi->s_buddy_cache);
> err_freesgi:
> - rcu_read_lock();
> - kvfree(rcu_dereference(sbi->s_group_info));
> - rcu_read_unlock();
> + kvfree(rcu_access_pointer(sbi->s_group_info));
> return -ENOMEM;
> }
>
> @@ -3897,7 +3895,8 @@ void ext4_mb_release(struct super_block *sb)
> WARN_ON_ONCE(!list_empty(&sbi->s_discard_list));
> }
>
> - if (sbi->s_group_info) {
> + group_info = rcu_access_pointer(sbi->s_group_info);
> + if (group_info) {
> for (i = 0; i < ngroups; i++) {
> cond_resched();
> grinfo = ext4_get_group_info(sb, i);
> @@ -3915,12 +3914,9 @@ void ext4_mb_release(struct super_block *sb)
> num_meta_group_infos = (ngroups +
> EXT4_DESC_PER_BLOCK(sb) - 1) >>
> EXT4_DESC_PER_BLOCK_BITS(sb);
> - rcu_read_lock();
> - group_info = rcu_dereference(sbi->s_group_info);
> for (i = 0; i < num_meta_group_infos; i++)
> kfree(group_info[i]);
> kvfree(group_info);
> - rcu_read_unlock();
> }
> ext4_mb_avg_fragment_size_destroy(sbi);
> ext4_mb_largest_free_orders_destroy(sbi);
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 43f680c750ae..0b2fa7bd787f 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1254,12 +1254,10 @@ static void ext4_group_desc_free(struct ext4_sb_info *sbi)
> struct buffer_head **group_desc;
> int i;
>
> - rcu_read_lock();
> - group_desc = rcu_dereference(sbi->s_group_desc);
> + group_desc = rcu_access_pointer(sbi->s_group_desc);
> for (i = 0; i < sbi->s_gdb_count; i++)
> brelse(group_desc[i]);
> kvfree(group_desc);
> - rcu_read_unlock();
> }
>
> static void ext4_flex_groups_free(struct ext4_sb_info *sbi)
> @@ -1267,14 +1265,12 @@ static void ext4_flex_groups_free(struct ext4_sb_info *sbi)
> struct flex_groups **flex_groups;
> int i;
>
> - rcu_read_lock();
> - flex_groups = rcu_dereference(sbi->s_flex_groups);
> + flex_groups = rcu_access_pointer(sbi->s_flex_groups);
> if (flex_groups) {
> for (i = 0; i < sbi->s_flex_groups_allocated; i++)
> kvfree(flex_groups[i]);
> kvfree(flex_groups);
> }
> - rcu_read_unlock();
> }
>
> static void ext4_put_super(struct super_block *sb)
^ permalink raw reply
* Re: [PATCH v3] ext4: fix use-after-free in update_super_work when racing with umount
From: Ritesh Harjani @ 2026-03-20 2:53 UTC (permalink / raw)
To: Jiayuan Chen, linux-ext4, jack
Cc: Jiayuan Chen, Jiayuan Chen, Theodore Ts'o, Andreas Dilger,
Ritesh Harjani, Ye Bin, linux-kernel
In-Reply-To: <20260319120336.157873-1-jiayuan.chen@linux.dev>
Jiayuan Chen <jiayuan.chen@linux.dev> writes:
> From: Jiayuan Chen <jiayuan.chen@shopee.com>
>
> Commit b98535d09179 ("ext4: fix bug_on in start_this_handle during umount
> filesystem") moved ext4_unregister_sysfs() before flushing s_sb_upd_work
> to prevent new error work from being queued via /proc/fs/ext4/xx/mb_groups
> reads during unmount. However, this introduced a use-after-free because
> update_super_work calls ext4_notify_error_sysfs() -> sysfs_notify() which
> accesses the kobject's kernfs_node after it has been freed by kobject_del()
> in ext4_unregister_sysfs():
>
> update_super_work ext4_put_super
> ----------------- --------------
> ext4_unregister_sysfs(sb)
> kobject_del(&sbi->s_kobj)
> __kobject_del()
> sysfs_remove_dir()
> kobj->sd = NULL
> sysfs_put(sd)
> kernfs_put() // RCU free
> ext4_notify_error_sysfs(sbi)
> sysfs_notify(&sbi->s_kobj)
> kn = kobj->sd // stale pointer
> kernfs_get(kn) // UAF on freed kernfs_node
> ext4_journal_destroy()
> flush_work(&sbi->s_sb_upd_work)
Yes, the race between kobject_del() and sysfs_notify() -> kernfs_get()
is real.
and sysfs_remove_dir() explicitely says:
* In general, kobject owner is responsible for ensuring removal
* doesn't race with other operations and sysfs doesn't provide any
* protection
Hence this patch which adds a new mutex lock to prevent the above race,
looks good to me. So please feel free to add:
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
^ permalink raw reply
* Re: [PATCH 2/3] ext4: add ext4_fc_eligible()
From: Andreas Dilger @ 2026-03-20 5:12 UTC (permalink / raw)
To: NeilBrown; +Cc: Jan Kara, Theodore Ts'o, linux-ext4, linux-fsdevel
In-Reply-To: <177396306176.3934327.9167241352093307006@noble.neil.brown.name>
> On Mar 19, 2026, at 17:31, NeilBrown <neilb@ownmail.net> wrote:
>
> On Thu, 19 Mar 2026, Jan Kara wrote:
>> On Wed 18-03-26 09:39:50, NeilBrown wrote:
>>> From: NeilBrown <neil@brown.name>
>>>
>>> Testing EXT4_MF_FC_INELIGIBLE is almost always combined with testing
>>> ext4_fc_disabled(). The code can be simplified by combining these two
>>> in a new ext4_fc_eligible().
>>>
>>> In ext4_fc_track_inode() this moves the ext4_fc_disabled() test after
>>> ext4_fc_mark_ineligible(), but as that is a non-op when
>>> ext4_fc_disabled() is true, this is no no consequence.
>>>
>>> Signed-off-by: NeilBrown <neil@brown.name>
>>
>> One nit below, otherwise feel free to add:
>>
>> Reviewed-by: Jan Kara <jack@suse.cz>
>>
>>> @@ -557,16 +548,13 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
>>> if (S_ISDIR(inode->i_mode))
>>> return;
>>>
>>> - if (ext4_fc_disabled(inode->i_sb))
>>> - return;
>>> -
>>> if (ext4_should_journal_data(inode)) {
>>> ext4_fc_mark_ineligible(inode->i_sb,
>>> EXT4_FC_REASON_INODE_JOURNAL_DATA, handle);
>>> return;
>>> }
>>>
>>> - if (ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE))
>>> + if (!ext4_fc_eligible(inode->i_sb))
>>> return;
>>
>> Here I think the !ext4_fc_eligible() check could be actually above the
>> ext4_should_journal_data() check - if the fs is not eligible for
>> fastcommit, there's no point in marking it ineligible again...
>
> Both you and Andreas have questioned that choice - so I should explain
> my reasoning.
>
> ext4_fc_mark_ineligible() is NOT a no-op when the sb is already marked
> ineligible. The code updates sb->s_fc_ineligible_tid to the largest tid
> which was ineligible for fc. Then it only clears the "ineligible" flag
> after that highest numbered transaction has committed. If we skip
> ext4_fc_mark_ineligible() because EXT4_MF_FC_INELIGIBLE is already set,
> then that flag could be cleared too early.
You are right, I didn't understand the ongoing effect of FC_INELIGIBLE.
As you write, checking ext4_fc_eligible() afterward is equivalent, since
ext4_fc_mark_ineligible() is itself checking ext4_fc_disabled() internally,
so the added overhead is the ext4_should_journal_data() call, which looks
pretty complex but is just a bunch of bit field checks (though potentially
adds a lot of branches depending on the frequency of the branches).
Cheers, Andreas
^ permalink raw reply
* Re: [PATCH] ext4: xattr: fix out-of-bounds access in ext4_xattr_set_entry
From: ZhengYuan Huang @ 2026-03-20 7:43 UTC (permalink / raw)
To: Theodore Tso
Cc: adilger.kernel, tahsin, linux-ext4, linux-kernel, baijiaju1990,
r33s3n6, zzzccc427, stable
In-Reply-To: <20260319135826.GA91368@macsyma-wired.lan>
On Thu, Mar 19, 2026 at 9:59 PM Theodore Tso <tytso@mit.edu> wrote:
> We don't consider bugs which involve modfying the mounted filesystem
> as valid from a security perspective. In particular, I don't want to
> add checks to hotpaths to try to protect against these sorts of
> failures, because they simply shouldn't be allowed --- and/or if the
> attacker has write access to the block device while the file system is
> mounted, you've basically lost already.
Thank you for the detailed explanation. I understand that runtime
modifications to a mounted block device are considered out of scope,
and adding checks for such cases in hot paths would be too costly.
Our original understanding was that a filesystem should handle on-disk
inconsistencies gracefully, so we used this approach to simulate
silent disk corruption or I/O errors at runtime and test filesystem
robustness. From your reply above, it seems that this understanding
may not be correct.
> That being said, we are more likely to accept patches to address
> static file system corruption, but the checks need to be done when the
> metadata in question is first loaded, and outside of a hot path. But
> trying to defend against dynamic modifications of the block device is
> really a fools errand, without completely trashing the performance of
> the file system.
There seem to be three layers of defense: fsck, mount-time checks, and
runtime checks. Would it be more accurate to understand the boundary
this way: once the filesystem metadata has passed mount-time
validation (even if it would not necessarily pass fsck), the
filesystem is still expected to handle later errors gracefully rather
than crash?
More specifically, for inconsistencies that arise at runtime, is the
general expectation that they are outside the filesystem's
responsibility and should instead be handled by other layers (for
example, lower-level storage redundancy / RAID)? Or is there still
room for defensive checks in the filesystem, as long as they are done
outside hot paths?
Thanks again for your time and clarification.
Best regards,
ZhengYuan Huang
^ permalink raw reply
* [PATCH] ext4: Fix deadlock on inode reallocation
From: Jan Kara @ 2026-03-20 9:04 UTC (permalink / raw)
To: Ted Tso; +Cc: linux-ext4, yi1.lai, Mateusz Guzik, Jan Kara, stable
Currently there is a race in ext4 when reallocating freed inode
resulting in a deadlock:
Task1 Task2
ext4_evict_inode()
handle = ext4_journal_start();
...
if (IS_SYNC(inode))
handle->h_sync = 1;
ext4_free_inode()
ext4_new_inode()
handle = ext4_journal_start()
finds the bit in inode bitmap
already clear
insert_inode_locked()
waits for inode to be
removed from the hash.
ext4_journal_stop(handle)
jbd2_journal_stop(handle)
jbd2_log_wait_commit(journal, tid);
- deadlocks waiting for transaction handle Task2 holds
Fix the problem by removing inode from the hash already in
ext4_clear_inode() by which time all IO for the inode is done so reuse
is already fine but we are still before possibly blocking on transaction
commit.
Reported-by: "Lai, Yi" <yi1.lai@linux.intel.com>
Link: https://lore.kernel.org/all/abNvb2PcrKj1FBeC@ly-workstation
Fixes: 88ec797c4680 ("fs: make insert_inode_locked() wait for inode destruction")
CC: stable@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/ext4/super.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
Ted, this is a regression recently introduced by VFS changes in
insert_inode_locked() but I think it's best fixed in ext4. If you agree, it
would be nice to merge this so that it makes it to 7.0 release. Thanks!
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 43f680c750ae..b8122d24c083 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1527,6 +1527,27 @@ void ext4_clear_inode(struct inode *inode)
invalidate_inode_buffers(inode);
clear_inode(inode);
ext4_discard_preallocations(inode);
+ /*
+ * We must remove the inode from the hash before ext4_free_inode()
+ * clears the bit in inode bitmap as otherwise another process reusing
+ * the inode will block in insert_inode_hash() waiting for inode
+ * eviction to complete while holding transaction handle open, but
+ * ext4_evict_inode() still running for that inode could block waiting
+ * for transaction commit if the inode is marked as IS_SYNC => deadlock.
+ *
+ * Removing the inode from the hash here is safe. There are two cases
+ * to consider:
+ * 1) The inode still has references to it (i_nlink > 0). In that case
+ * we are keeping the inode and once we remove the inode from the hash,
+ * iget() can create the new inode structure for the same inode number
+ * and we are fine with that as all IO on behalf of the inode is
+ * finished.
+ * 2) We are deleting the inode (i_nlink == 0). In that case inode
+ * number cannot be reused until ext4_free_inode() clears the bit in
+ * the inode bitmap, at which point all IO is done and reuse is fine
+ * again.
+ */
+ remove_inode_hash(inode);
ext4_es_remove_extent(inode, 0, EXT_MAX_BLOCKS);
dquot_drop(inode);
if (EXT4_I(inode)->jinode) {
--
2.51.0
^ permalink raw reply related
* Re: [PATCH 2/3] ext4: add ext4_fc_eligible()
From: Jan Kara @ 2026-03-20 10:24 UTC (permalink / raw)
To: NeilBrown
Cc: Jan Kara, Theodore Ts'o, Andreas Dilger, linux-ext4,
linux-fsdevel
In-Reply-To: <177396306176.3934327.9167241352093307006@noble.neil.brown.name>
On Fri 20-03-26 10:31:01, NeilBrown wrote:
> On Thu, 19 Mar 2026, Jan Kara wrote:
> > On Wed 18-03-26 09:39:50, NeilBrown wrote:
> > > From: NeilBrown <neil@brown.name>
> > >
> > > Testing EXT4_MF_FC_INELIGIBLE is almost always combined with testing
> > > ext4_fc_disabled(). The code can be simplified by combining these two
> > > in a new ext4_fc_eligible().
> > >
> > > In ext4_fc_track_inode() this moves the ext4_fc_disabled() test after
> > > ext4_fc_mark_ineligible(), but as that is a non-op when
> > > ext4_fc_disabled() is true, this is no no consequence.
> > >
> > > Signed-off-by: NeilBrown <neil@brown.name>
> >
> > One nit below, otherwise feel free to add:
> >
> > Reviewed-by: Jan Kara <jack@suse.cz>
> >
> > > @@ -557,16 +548,13 @@ void ext4_fc_track_inode(handle_t *handle, struct inode *inode)
> > > if (S_ISDIR(inode->i_mode))
> > > return;
> > >
> > > - if (ext4_fc_disabled(inode->i_sb))
> > > - return;
> > > -
> > > if (ext4_should_journal_data(inode)) {
> > > ext4_fc_mark_ineligible(inode->i_sb,
> > > EXT4_FC_REASON_INODE_JOURNAL_DATA, handle);
> > > return;
> > > }
> > >
> > > - if (ext4_test_mount_flag(inode->i_sb, EXT4_MF_FC_INELIGIBLE))
> > > + if (!ext4_fc_eligible(inode->i_sb))
> > > return;
> >
> > Here I think the !ext4_fc_eligible() check could be actually above the
> > ext4_should_journal_data() check - if the fs is not eligible for
> > fastcommit, there's no point in marking it ineligible again...
>
> Both you and Andreas have questioned that choice - so I should explain
> my reasoning.
>
> ext4_fc_mark_ineligible() is NOT a no-op when the sb is already marked
> ineligible. The code updates sb->s_fc_ineligible_tid to the largest tid
> which was ineligible for fc. Then it only clears the "ineligible" flag
> after that highest numbered transaction has committed. If we skip
> ext4_fc_mark_ineligible() because EXT4_MF_FC_INELIGIBLE is already set,
> then that flag could be cleared too early.
Oops, you're right. I forgot about this subtlety. Thanks for explanation!
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* [syzbot ci] Re: ext4: fix use-after-free in update_super_work when racing with umount
From: syzbot ci @ 2026-03-20 10:27 UTC (permalink / raw)
To: adilger.kernel, jack, jiayuan.chen, jiayuan.chen, linux-ext4,
linux-kernel, riteshh, tytso, yebin10
Cc: syzbot, syzkaller-bugs
In-Reply-To: <20260319073651.79209-1-jiayuan.chen@linux.dev>
syzbot ci has tested the following series
[v2] ext4: fix use-after-free in update_super_work when racing with umount
https://lore.kernel.org/all/20260319073651.79209-1-jiayuan.chen@linux.dev
* [PATCH v2] ext4: fix use-after-free in update_super_work when racing with umount
and found the following issue:
WARNING in ext4_notify_error_sysfs
Full report is available here:
https://ci.syzbot.org/series/3b258c63-ea54-492a-ab47-c09e62612658
***
WARNING in ext4_notify_error_sysfs
tree: torvalds
URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux
base: 0e4f8f1a3d081e834be5fd0a62bdb2554fadd307
arch: amd64
compiler: Debian clang version 21.1.8 (++20251221033036+2078da43e25a-1~exp1~20251221153213.50), Debian LLD 21.1.8
config: https://ci.syzbot.org/builds/8149e3c8-4cb9-4fa0-9bf5-3d6deda6899d/config
C repro: https://ci.syzbot.org/findings/6f6eafc6-a231-4909-9025-539fa10ffcfd/c_repro
syz repro: https://ci.syzbot.org/findings/6f6eafc6-a231-4909-9025-539fa10ffcfd/syz_repro
------------[ cut here ]------------
DEBUG_LOCKS_WARN_ON(lock->magic != lock)
WARNING: kernel/locking/mutex.c:593 at __mutex_lock_common kernel/locking/mutex.c:593 [inline], CPU#1: kworker/1:4/5883
WARNING: kernel/locking/mutex.c:593 at __mutex_lock+0x10a4/0x1300 kernel/locking/mutex.c:776, CPU#1: kworker/1:4/5883
Modules linked in:
CPU: 1 UID: 0 PID: 5883 Comm: kworker/1:4 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Workqueue: events update_super_work
RIP: 0010:__mutex_lock_common kernel/locking/mutex.c:593 [inline]
RIP: 0010:__mutex_lock+0x10ab/0x1300 kernel/locking/mutex.c:776
Code: 11 90 48 c1 e8 03 42 0f b6 04 28 84 c0 0f 85 33 02 00 00 83 3d d9 e1 61 04 00 75 13 48 8d 3d 1c f7 64 04 48 c7 c6 c0 e0 cc 8b <67> 48 0f b9 3a 90 e9 ac f0 ff ff 90 0f 0b 90 e9 73 f4 ff ff 90 0f
RSP: 0018:ffffc90004dcf900 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 1ffff920009b9f38 RCX: ffff888172d3d7c0
RDX: 0000000000000000 RSI: ffffffff8bcce0c0 RDI: ffffffff9014ec10
RBP: ffffc90004dcfaa8 R08: ffffffff9011d6c3 R09: 1ffffffff2023ad8
R10: dffffc0000000000 R11: fffffbfff2023ad9 R12: ffff888172218368
R13: dffffc0000000000 R14: 0000000000000000 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff8882a945d000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000562af42c7d40 CR3: 0000000111346000 CR4: 00000000000006f0
Call Trace:
<TASK>
ext4_notify_error_sysfs+0x23/0xa0 fs/ext4/sysfs.c:600
process_one_work kernel/workqueue.c:3276 [inline]
process_scheduled_works+0xb6e/0x18c0 kernel/workqueue.c:3359
worker_thread+0xa53/0xfc0 kernel/workqueue.c:3440
kthread+0x388/0x470 kernel/kthread.c:436
ret_from_fork+0x51e/0xb90 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
</TASK>
----------------
Code disassembly (best guess):
0: 11 90 48 c1 e8 03 adc %edx,0x3e8c148(%rax)
6: 42 0f b6 04 28 movzbl (%rax,%r13,1),%eax
b: 84 c0 test %al,%al
d: 0f 85 33 02 00 00 jne 0x246
13: 83 3d d9 e1 61 04 00 cmpl $0x0,0x461e1d9(%rip) # 0x461e1f3
1a: 75 13 jne 0x2f
1c: 48 8d 3d 1c f7 64 04 lea 0x464f71c(%rip),%rdi # 0x464f73f
23: 48 c7 c6 c0 e0 cc 8b mov $0xffffffff8bcce0c0,%rsi
* 2a: 67 48 0f b9 3a ud1 (%edx),%rdi <-- trapping instruction
2f: 90 nop
30: e9 ac f0 ff ff jmp 0xfffff0e1
35: 90 nop
36: 0f 0b ud2
38: 90 nop
39: e9 73 f4 ff ff jmp 0xfffff4b1
3e: 90 nop
3f: 0f .byte 0xf
***
If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
Tested-by: syzbot@syzkaller.appspotmail.com
---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.
^ permalink raw reply
* Re: [PATCH] ext4: xattr: fix out-of-bounds access in ext4_xattr_set_entry
From: Theodore Tso @ 2026-03-20 12:32 UTC (permalink / raw)
To: ZhengYuan Huang
Cc: adilger.kernel, tahsin, linux-ext4, linux-kernel, baijiaju1990,
r33s3n6, zzzccc427, stable
In-Reply-To: <CAOmEq9VAW_a7RsSPquy0_eJOLP4aHOWvwTtzmeLUPXpy85xJvw@mail.gmail.com>
On Fri, Mar 20, 2026 at 03:43:21PM +0800, ZhengYuan Huang wrote:
>
> There seem to be three layers of defense: fsck, mount-time checks, and
> runtime checks.
Within runtime checks, there are those checks that are done the first
time metadata is loaded from disk --- for example, see the checks in
__ext4_iget() and the functions it calls, such as check_igot_inode().
And then there are checks that are done in hotpaths, since at least in
theory, a stupid system administrator which makes a block device be
world-writeable and so a malicious or accidental actor could modify
the copy of the metadata in the buffer cache. Those are the ones
sorts of runtime checks we sould try to avoid.
Mount-time checks tend to be those that validate superblock and block
group descriptor contents. They can't validate all of the inodes
because that would take a lot longer.
> Would it be more accurate to understand the boundary
> this way: once the filesystem metadata has passed mount-time
> validation (even if it would not necessarily pass fsck), the
> filesystem is still expected to handle later errors gracefully rather
> than crash?
It is a nice to have that a file system, should handle errors
gracefully rather than crash. However, if the inconsistency would
have been caught and corrected by fsck, I don't consider it a
CVE-worthy security bug, but rather a quality-of-implementation bug.
This is important, because there are risks associated with rolling out
a new kernel to hundreds of thousands of machines, or using live
patching to fix high severity security bugs. If the issue could have
been caught by fsck, and a competently administered system *does* run
fsck at boot time (such as at $WORK), the cost benefit ratio of
treating such bugs as security bugs doesn't make sense.
> More specifically, for inconsistencies that arise at runtime, is the
> general expectation that they are outside the filesystem's
> responsibility and should instead be handled by other layers (for
> example, lower-level storage redundancy / RAID)? Or is there still
> room for defensive checks in the filesystem, as long as they are done
> outside hot paths?
This would be on a case by case basis. If the check is *super* cheap,
and it's done outside of a hotpath --- say, when a file is first
opened. And if doesn't cause long-term maintenance issues, it is
comething that could be considered. But in terms of the priority of
dealing with such patches, it is not something that would be
considered high priority. Perhaps just a step above spelling or
grammer fixes in comments. :-)
Consider that for a enterprise hard drives, the bit error rate is 1 in
10**15. And the chances that such as a bit error would be cause a
metadata inconsistency that would lead to a crash has to be factored
in. If we had infinite resources, it might be something that would be
considered higher priority, but in the real world, when the
opportunity cost of having software engineers working on other
improvements, it's not necessarily going to be a compelling business
case when I go to my management asking for more headcount. And if you
are an academic, perhaps the impact of such work might also be called
into question.
Cheers,
- Ted
^ permalink raw reply
* [PATCH 02/41] gfs2: Don't zero i_private_data
From: Jan Kara @ 2026-03-20 13:40 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-block, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise, Jan Kara, Andreas Gruenbacher, gfs2
In-Reply-To: <20260320131728.6449-1-jack@suse.cz>
Remove the explicit zeroing of mapping->i_private_data since this
field is no longer used.
CC: Andreas Gruenbacher <agruenba@redhat.com>
CC: gfs2@lists.linux.dev
Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/gfs2/glock.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 2acbabccc8ad..b8a144d3a73b 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -1149,7 +1149,6 @@ int gfs2_glock_get(struct gfs2_sbd *sdp, u64 number,
mapping->flags = 0;
gfp_mask = mapping_gfp_mask(sdp->sd_inode->i_mapping);
mapping_set_gfp_mask(mapping, gfp_mask);
- mapping->i_private_data = NULL;
mapping->writeback_index = 0;
}
--
2.51.0
^ permalink raw reply related
* [PATCH v2 0/41] fs: Move metadata bh tracking from address_space
From: Jan Kara @ 2026-03-20 13:40 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-block, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise, Jan Kara
Hello,
here is a next revision of the patchset cleaning up buffer head metadata
tracking and use of address_space's private_list and private_lock. The patches
have survived some testing with fstests and ltp however I didn't test AFFS and
KVM guest_memfd changes so a help with testing those would be very welcome.
Thanks.
Changes since v1:
* Fixed hugetlbfs handling of root directory
* Reworked mapping_metadata_bhs handling functions to get the tracking
structure as an argument so we now don't need iops method to fetch the struct
from the inode
* Reordered patches into more sensible order
* Added patch to merge two mostly duplicate generic fsync implementations
* Added Reviewed-by tags
* Couple more minor changes that were requested during review
Original cover letter:
this patch series cleans up the mess that has accumulated over the years in
metadata buffer_head tracking for inodes, moves the tracking into dedicated
structure in filesystem-private part of the inode (so that we don't use
private_list, private_data, and private_lock in struct address_space), and also
moves couple other users of private_data and private_list so these are removed
from struct address_space saving 3 longs in struct inode for 99% of inodes. I
would like to get rid of private_lock in struct address_space as well however
the locking changes for buffer_heads are non-trivial there and the patch series
is long enough as is. So let's leave that for another time.
block/bdev.c | 1
fs/affs/affs.h | 2
fs/affs/dir.c | 1
fs/affs/file.c | 1
fs/affs/inode.c | 2
fs/affs/super.c | 6
fs/affs/symlink.c | 1
fs/aio.c | 78 +++++++-
fs/bfs/bfs.h | 2
fs/bfs/dir.c | 1
fs/bfs/file.c | 4
fs/bfs/inode.c | 9 +
fs/buffer.c | 387 +++++++++++++++++---------------------------
fs/ext2/ext2.h | 2
fs/ext2/file.c | 1
fs/ext2/inode.c | 3
fs/ext2/namei.c | 2
fs/ext2/super.c | 6
fs/ext2/symlink.c | 2
fs/ext4/ext4.h | 4
fs/ext4/file.c | 1
fs/ext4/inode.c | 9 -
fs/ext4/namei.c | 2
fs/ext4/super.c | 9 -
fs/ext4/symlink.c | 3
fs/fat/fat.h | 2
fs/fat/file.c | 1
fs/fat/inode.c | 16 +
fs/fat/namei_msdos.c | 1
fs/fat/namei_vfat.c | 1
fs/gfs2/glock.c | 1
fs/hugetlbfs/inode.c | 10 -
fs/inode.c | 24 +-
fs/minix/file.c | 1
fs/minix/inode.c | 10 +
fs/minix/minix.h | 2
fs/minix/namei.c | 1
fs/ntfs3/file.c | 3
fs/ocfs2/dlmglue.c | 1
fs/ocfs2/namei.c | 3
fs/udf/file.c | 1
fs/udf/inode.c | 2
fs/udf/namei.c | 1
fs/udf/super.c | 6
fs/udf/symlink.c | 1
fs/udf/udf_i.h | 1
fs/udf/udfdecl.h | 1
include/linux/buffer_head.h | 6
include/linux/fs.h | 11 -
include/linux/hugetlb.h | 1
mm/hugetlb.c | 10 -
virt/kvm/guest_memfd.c | 12 -
52 files changed, 360 insertions(+), 309 deletions(-)
Honza
Previous versions:
Link: http://lore.kernel.org/r/20260303101717.27224-1-jack@suse.cz # v1
^ permalink raw reply
* [PATCH 01/41] ext4: Use inode_has_buffers()
From: Jan Kara @ 2026-03-20 13:40 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-block, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise, Jan Kara
In-Reply-To: <20260320131728.6449-1-jack@suse.cz>
Instead of checking i_private_list directly use appropriate wrapper
inode_has_buffers(). Also delete stale comment.
Acked-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/buffer.c | 1 +
fs/ext4/inode.c | 5 +----
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index 22b43642ba57..1bc0f22f3cc2 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -524,6 +524,7 @@ int inode_has_buffers(struct inode *inode)
{
return !list_empty(&inode->i_data.i_private_list);
}
+EXPORT_SYMBOL_GPL(inode_has_buffers);
/*
* osync is designed to support O_SYNC io. It waits synchronously for
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 396dc3a5d16b..d18d94acddcc 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1420,9 +1420,6 @@ static int write_end_fn(handle_t *handle, struct inode *inode,
/*
* We need to pick up the new inode size which generic_commit_write gave us
* `iocb` can be NULL - eg, when called from page_symlink().
- *
- * ext4 never places buffers on inode->i_mapping->i_private_list. metadata
- * buffers are managed internally.
*/
static int ext4_write_end(const struct kiocb *iocb,
struct address_space *mapping,
@@ -3437,7 +3434,7 @@ static bool ext4_inode_datasync_dirty(struct inode *inode)
}
/* Any metadata buffers to write? */
- if (!list_empty(&inode->i_mapping->i_private_list))
+ if (inode_has_buffers(inode))
return true;
return inode_state_read_once(inode) & I_DIRTY_DATASYNC;
}
--
2.51.0
^ permalink raw reply related
* [PATCH 04/41] ocfs2: Drop pointless sync_mapping_buffers() calls
From: Jan Kara @ 2026-03-20 13:40 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-block, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise, Jan Kara, Joel Becker, Joseph Qi, ocfs2-devel
In-Reply-To: <20260320131728.6449-1-jack@suse.cz>
ocfs2 never calls mark_buffer_dirty_inode() and thus its metadata
buffers list is always empty. Drop the pointless sync_mapping_buffers()
calls.
CC: Joel Becker <jlbec@evilplan.org>
CC: Joseph Qi <joseph.qi@linux.alibaba.com>
CC: ocfs2-devel@lists.linux.dev
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/ocfs2/dlmglue.c | 1 -
fs/ocfs2/namei.c | 3 ---
2 files changed, 4 deletions(-)
diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index bd2ddb7d841d..7283bb2c5a31 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -3971,7 +3971,6 @@ static int ocfs2_data_convert_worker(struct ocfs2_lock_res *lockres,
mlog(ML_ERROR, "Could not sync inode %llu for downconvert!",
(unsigned long long)OCFS2_I(inode)->ip_blkno);
}
- sync_mapping_buffers(mapping);
if (blocking == DLM_LOCK_EX) {
truncate_inode_pages(mapping, 0);
} else {
diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 268b79339a51..1277666c77cd 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -1683,9 +1683,6 @@ static int ocfs2_rename(struct mnt_idmap *idmap,
if (rename_lock)
ocfs2_rename_unlock(osb);
- if (new_inode)
- sync_mapping_buffers(old_inode->i_mapping);
-
iput(new_inode);
ocfs2_free_dir_lookup_result(&target_lookup_res);
--
2.51.0
^ permalink raw reply related
* [PATCH 03/41] ntfs3: Drop pointless sync_mapping_buffers() and invalidate_inode_buffers() calls
From: Jan Kara @ 2026-03-20 13:40 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-block, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise, Jan Kara, Konstantin Komarov, ntfs3
In-Reply-To: <20260320131728.6449-1-jack@suse.cz>
ntfs3 never calls mark_buffer_dirty_inode() and thus its metadata
buffers list is always empty. Drop the pointless sync_mapping_buffers()
and invalidate_inode_buffers() calls.
CC: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
CC: ntfs3@lists.linux.dev
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/ntfs3/file.c | 3 ---
fs/ntfs3/inode.c | 1 -
2 files changed, 4 deletions(-)
diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 7eecf1e01f74..570c92fa7ee7 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -387,9 +387,6 @@ static int ntfs_extend(struct inode *inode, loff_t pos, size_t count,
int err2;
err = filemap_fdatawrite_range(mapping, pos, end - 1);
- err2 = sync_mapping_buffers(mapping);
- if (!err)
- err = err2;
err2 = write_inode_now(inode, 1);
if (!err)
err = err2;
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
index 6e65066ebcc1..5d8f04dedcc8 100644
--- a/fs/ntfs3/inode.c
+++ b/fs/ntfs3/inode.c
@@ -1860,7 +1860,6 @@ void ntfs_evict_inode(struct inode *inode)
{
truncate_inode_pages_final(&inode->i_data);
- invalidate_inode_buffers(inode);
clear_inode(inode);
ni_clear(ntfs_i(inode));
--
2.51.0
^ permalink raw reply related
* [PATCH 08/41] udf: Switch to generic_buffers_fsync()
From: Jan Kara @ 2026-03-20 13:41 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-block, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise, Jan Kara
In-Reply-To: <20260320131728.6449-1-jack@suse.cz>
UDF uses metadata bh list attached to inode. Switch it to
generic_buffers_fsync() instead of generic_file_fsync().
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/dir.c | 2 +-
fs/udf/file.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/udf/dir.c b/fs/udf/dir.c
index 5bf75638f352..a1705aedac46 100644
--- a/fs/udf/dir.c
+++ b/fs/udf/dir.c
@@ -157,6 +157,6 @@ const struct file_operations udf_dir_operations = {
.read = generic_read_dir,
.iterate_shared = udf_readdir,
.unlocked_ioctl = udf_ioctl,
- .fsync = generic_file_fsync,
+ .fsync = generic_buffers_fsync,
.setlease = generic_setlease,
};
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 32ae7cfd72c5..627b07320d06 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -205,7 +205,7 @@ const struct file_operations udf_file_operations = {
.mmap = udf_file_mmap,
.write_iter = udf_file_write_iter,
.release = udf_release_file,
- .fsync = generic_file_fsync,
+ .fsync = generic_buffers_fsync,
.splice_read = filemap_splice_read,
.splice_write = iter_file_splice_write,
.llseek = generic_file_llseek,
--
2.51.0
^ permalink raw reply related
* [PATCH 06/41] ufs: Drop pointless invalidate_mapping_buffers() call
From: Jan Kara @ 2026-03-20 13:41 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-block, Christian Brauner, Al Viro, linux-ext4, Ted Tso,
Tigran A. Aivazian, David Sterba, OGAWA Hirofumi, Muchun Song,
Oscar Salvador, David Hildenbrand, linux-mm, linux-aio,
Benjamin LaHaise, Jan Kara
In-Reply-To: <20260320131728.6449-1-jack@suse.cz>
UFS doesn't call mark_buffer_dirty_inode() and thus
invalidate_mapping_buffers() never has anything to drop. Remove the
pointless call.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/ufs/inode.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/ufs/inode.c b/fs/ufs/inode.c
index e2b0a35de2a7..77617a31d517 100644
--- a/fs/ufs/inode.c
+++ b/fs/ufs/inode.c
@@ -853,7 +853,6 @@ void ufs_evict_inode(struct inode * inode)
ufs_update_inode(inode, inode_needs_sync(inode));
}
- invalidate_inode_buffers(inode);
clear_inode(inode);
if (want_delete)
--
2.51.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox