[PATCH 0/13] fs: Various I_DIRTY_* state handling fixes

[PATCH 0/13] fs: Various I_DIRTY_* state handling fixes

[Jan Kara]

Hello,

this series is a result of my audit of I_DIRTY_* state handling fixes. Over the
years some pointless code or code with smaller or larger bugs has accumulated
in filesystems around handling of inode dirty state. In particular testing
for I_DIRTY_PAGES is very subtle (and racy without holding i_lock), some places
forget that I_DIRTY_TIME also means inode has dirty metadata etc. So this
patch set removes direct checks of dirtiness state from filesystems and either
change them to directly testing PAGECACHE_TAG_DIRTY or to a helper function
which performs appropriate dirtiness check reliably.

								Honza

[PATCH 01/13] bdev: Drop pointless loop from blkdev_flush_mapping()

[Jan Kara]

bdev_write_inode() is really odd loop calling write_inode_now(). The
bdev doesn't have any pages by now and if there are any dirty bits in
the inode state, they are stale. Just call write_inode_now() once to
clear them.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 block/bdev.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index f20f91b3176e..9fa72df4f47a 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -60,24 +60,6 @@ struct block_device *file_bdev(struct file *bdev_file)
 }
 EXPORT_SYMBOL(file_bdev);
 
-static void bdev_write_inode(struct block_device *bdev)
-{
-	struct inode *inode = BD_INODE(bdev);
-	int ret;
-
-	spin_lock(&inode->i_lock);
-	while (inode_state_read(inode) & I_DIRTY) {
-		spin_unlock(&inode->i_lock);
-		ret = write_inode_now(inode, true);
-		if (ret)
-			pr_warn_ratelimited(
-	"VFS: Dirty inode writeback failed for block device %pg (err=%d).\n",
-				bdev, ret);
-		spin_lock(&inode->i_lock);
-	}
-	spin_unlock(&inode->i_lock);
-}
-
 /* Kill _all_ buffers and pagecache , dirty or not.. */
 static void kill_bdev(struct block_device *bdev)
 {
@@ -755,7 +737,12 @@ static void blkdev_flush_mapping(struct block_device *bdev)
 	WARN_ON_ONCE(bdev->bd_holders);
 	sync_blockdev(bdev);
 	kill_bdev(bdev);
-	bdev_write_inode(bdev);
+	/*
+	 * All data is written out and evicted but inode can have some stale
+	 * I_DIRTY bits set. Call write_inode_now() to get rid of them so that
+	 * the inode can be evicted.
+	 */
+	write_inode_now(BD_INODE(bdev), 1);
 }
 
 static void blkdev_put_whole(struct block_device *bdev)
-- 
2.53.0

[PATCH 02/13] xfs: Drop pointless I_DIRTY_PAGES check in xfs_can_free_cowblocks()

[Jan Kara]

PAGECACHE_TAG_DIRTY is the authoritative source of information about any
folio being dirty in the mapping. I_DIRTY_PAGES is just an approximation
used by writeback code that can get stale under certain situations. So
just rely on the authoritative check.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/xfs/xfs_reflink.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
index 683c1841e640..1310cc9fe829 100644
--- a/fs/xfs/xfs_reflink.h
+++ b/fs/xfs/xfs_reflink.h
@@ -17,8 +17,7 @@ xfs_can_free_cowblocks(struct xfs_inode *ip)
 {
 	struct inode *inode = VFS_I(ip);
 
-	if ((inode_state_read_once(inode) & I_DIRTY_PAGES) ||
-	    mapping_tagged(inode->i_mapping, PAGECACHE_TAG_DIRTY) ||
+	if (mapping_tagged(inode->i_mapping, PAGECACHE_TAG_DIRTY) ||
 	    mapping_tagged(inode->i_mapping, PAGECACHE_TAG_WRITEBACK) ||
 	    atomic_read(&inode->i_dio_count))
 		return false;
-- 
2.53.0

[PATCH 03/13] orangefs: Check PAGECACHE_TAG_DIRTY instead of I_DIRTY_PAGES inode state

[Jan Kara]

PAGECACHE_TAG_DIRTY is the authoritative source of dirty folios present
in the mapping. Use that instead of looking at I_DIRTY_PAGES inode state
which can get stale (e.g. when folios are written directly with
filemap_fdatawrite()).

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/orangefs/orangefs-utils.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/orangefs/orangefs-utils.c b/fs/orangefs/orangefs-utils.c
index 9ab1119ebd28..4c4036f470cd 100644
--- a/fs/orangefs/orangefs-utils.c
+++ b/fs/orangefs/orangefs-utils.c
@@ -247,7 +247,8 @@ int orangefs_inode_getattr(struct inode *inode, int flags)
 	spin_lock(&inode->i_lock);
 	/* Must have all the attributes in the mask and be within cache time. */
 	if ((!flags && time_before(jiffies, orangefs_inode->getattr_time)) ||
-	    orangefs_inode->attr_valid || inode_state_read(inode) & I_DIRTY_PAGES) {
+	    orangefs_inode->attr_valid ||
+	    mapping_tagged(inode->i_mapping, PAGECACHE_TAG_DIRTY)) {
 		if (orangefs_inode->attr_valid) {
 			spin_unlock(&inode->i_lock);
 			write_inode_now(inode, 1);
@@ -281,13 +282,14 @@ int orangefs_inode_getattr(struct inode *inode, int flags)
 	spin_lock(&inode->i_lock);
 	/* Must have all the attributes in the mask and be within cache time. */
 	if ((!flags && time_before(jiffies, orangefs_inode->getattr_time)) ||
-	    orangefs_inode->attr_valid || inode_state_read(inode) & I_DIRTY_PAGES) {
+	    orangefs_inode->attr_valid ||
+	    mapping_tagged(inode->i_mapping, PAGECACHE_TAG_DIRTY)) {
 		if (orangefs_inode->attr_valid) {
 			spin_unlock(&inode->i_lock);
 			write_inode_now(inode, 1);
 			goto again2;
 		}
-		if (inode_state_read(inode) & I_DIRTY_PAGES) {
+		if (mapping_tagged(inode->i_mapping, PAGECACHE_TAG_DIRTY)) {
 			ret = 0;
 			goto out_unlock;
 		}
-- 
2.53.0

[PATCH 04/13] fs: Provide inode_is_dirty() helper

[Jan Kara]

Provide inode_is_dirty() helper for use by filesystems (mostly fsync
methods).

Signed-off-by: Jan Kara <jack@suse.cz>
---
 include/linux/fs.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 99caef43ff3c..025f5d890689 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2217,6 +2217,17 @@ static inline void mark_inode_dirty_sync(struct inode *inode)
 	__mark_inode_dirty(inode, I_DIRTY_SYNC);
 }
 
+/*
+ * Check whether inode itself needs to be written out either at all or for
+ * the purposes of datasync if @datasync is set.
+ */
+static inline bool inode_is_dirty(struct inode *inode, bool datasync)
+{
+	int mask = datasync ? I_DIRTY_DATASYNC : (I_DIRTY_INODE | I_DIRTY_TIME);
+
+	return inode_state_read_once(inode) & mask;
+}
+
 static inline void set_inode_metadata_writeback(struct inode *inode)
 {
 	spin_lock(&inode->i_lock);
-- 
2.53.0

[PATCH 05/13] jfs: Use inode_is_dirty() in jfs_fsync()

[Jan Kara]

Use inode_is_dirty() helper in jfs_fsync() instead of explicit i_state
check. This slightly changes behavior in that we no longer trigger inode
commit if the inode is clean except it still has I_DIRTY_PAGES set after
data writeout but that is actually a desirable behavior because commit
is pointless if only folios are dirty.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/jfs/file.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/jfs/file.c b/fs/jfs/file.c
index 2f5bb79c0591..90001d3d90fc 100644
--- a/fs/jfs/file.c
+++ b/fs/jfs/file.c
@@ -27,8 +27,7 @@ int jfs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
 		return rc;
 
 	inode_lock(inode);
-	if (!(inode_state_read_once(inode) & I_DIRTY_ALL) ||
-	    (datasync && !(inode_state_read_once(inode) & I_DIRTY_DATASYNC))) {
+	if (!inode_is_dirty(inode, datasync)) {
 		/* Make sure committed changes hit the disk */
 		jfs_flush_journal(JFS_SBI(inode->i_sb)->log, 1);
 		inode_unlock(inode);
-- 
2.53.0

[PATCH 06/13] gfs2: Avoid racy dirtiness check in gfs2_fsync()

[Jan Kara]

Check for I_DIRTY_PAGES state in gfs2_fsync() is racy because
__writeback_single_inode() unconditionally clears the flag and
reinstates it only later based on PAGECACHE_TAG_DIRTY tag. If
gfs2_fsync() unlocked read of i_state hits in between, it will not see
I_DIRTY_PAGES although there may be dirty folios in the mapping. Check
for PAGECACHE_TAG_DIRTY instead and use inode_is_dirty() helper for
inode metadata dirtiness check.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/gfs2/file.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 1efd0679badd..9e52f1832673 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -749,7 +749,6 @@ static int gfs2_fsync(struct file *file, loff_t start, loff_t end,
 {
 	struct address_space *mapping = file->f_mapping;
 	struct inode *inode = mapping->host;
-	int sync_state = inode_state_read_once(inode) & I_DIRTY;
 	struct gfs2_glock *gl = gfs2_inode_glock(inode);
 	struct gfs2_inode *ip = GFS2_I(inode);
 	int ret = 0, ret1 = 0;
@@ -760,12 +759,9 @@ static int gfs2_fsync(struct file *file, loff_t start, loff_t end,
 			return ret1;
 	}
 
-	if (!gfs2_is_jdata(ip))
-		sync_state &= ~I_DIRTY_PAGES;
-	if (datasync)
-		sync_state &= ~I_DIRTY_SYNC;
-
-	if (sync_state) {
+	if (inode_is_dirty(inode, datasync) ||
+	    (gfs2_is_jdata(ip) && mapping_tagged(inode->i_mapping,
+						 PAGECACHE_TAG_DIRTY))) {
 		ret = sync_inode_metadata(inode, 1);
 		if (ret)
 			return ret;
-- 
2.53.0

[PATCH 07/13] fuse: Use inode_is_dirty() check in fuse_evict_inode()

[Jan Kara]

Use inode_is_dirty() helper instead of explicit i_state check. This will
result in additionally warning if we have unwritten timestamp updates
when lazytime is enabled but that is actually desirable.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/fuse/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 1341b681d0ed..ef88ba7bf682 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -143,7 +143,7 @@ static void fuse_evict_inode(struct inode *inode)
 	struct fuse_inode *fi = get_fuse_inode(inode);
 
 	/* Will write inode on close/munmap and in all other dirtiers */
-	WARN_ON(inode_state_read_once(inode) & I_DIRTY_INODE);
+	WARN_ON(inode_is_dirty(inode, false));
 
 	if (IS_DAX(inode))
 		dax_break_layout_final(inode);
-- 
2.53.0

[PATCH 08/13] fs: Provide inode_or_folios_dirty() helper

[Jan Kara]

Provide a helper function checking whether inode itself or some of its
folios are dirty. We'll use it in filesystems instead of direct i_state
checks. Note that this check returns true also if only timestamps are
dirty when lazytime is enabled. This is what users really want.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 include/linux/fs.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 025f5d890689..f602deacf953 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2228,6 +2228,13 @@ static inline bool inode_is_dirty(struct inode *inode, bool datasync)
 	return inode_state_read_once(inode) & mask;
 }
 
+/* Check whether anything in the inode itself or its folios need writing */
+static inline bool inode_or_folios_dirty(struct inode *inode)
+{
+	return inode_state_read_once(inode) & I_DIRTY_ALL ||
+		mapping_tagged(inode->i_mapping, PAGECACHE_TAG_DIRTY);
+}
+
 static inline void set_inode_metadata_writeback(struct inode *inode)
 {
 	spin_lock(&inode->i_lock);
-- 
2.53.0

[PATCH 09/13] gfs2: Fix dirtiness checks in gfs2_write_inode() and evict_linked_inode()

[Jan Kara]

The checks are bogus because they check against inode->i_flags instead
of inode->i_state. Use the inode_or_folios_dirty() helper which actually
checks what was intended.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/gfs2/super.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index 04bb4cf787d4..9b723dd591a5 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -460,7 +460,7 @@ static int gfs2_write_inode(struct inode *inode, struct writeback_control *wbc)
 		mark_inode_dirty_sync(inode);
 	else {
 		spin_lock(&inode->i_lock);
-		if (!(inode->i_flags & I_DIRTY))
+		if (!inode_or_folios_dirty(inode))
 			gfs2_ordered_del_inode(ip);
 		spin_unlock(&inode->i_lock);
 	}
@@ -1418,7 +1418,8 @@ static int evict_linked_inode(struct inode *inode, struct gfs2_holder *gh)
 	struct address_space *metamapping = gfs2_glock2aspace(gl);
 	int ret;
 
-	if (!(test_bit(GLF_DIRTY, &gl->gl_flags) || inode->i_flags & I_DIRTY))
+	if (!(test_bit(GLF_DIRTY, &gl->gl_flags) ||
+	      inode_or_folios_dirty(inode)))
 		goto clean;
 
 	/* The inode glock must be held exclusively and be instantiated. */
-- 
2.53.0

[PATCH 10/13] afs: Use inode_or_folios_dirty() in afs_evict_inode()

[Jan Kara]

Use the helper instead of direct inode I_DIRTY check to abstract details
of inode dirtiness handling. This also properly triggers when only
timestamps are dirty and lazytime is enabled (although I'm not sure if
it really matters for this particular case in AFS).

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/afs/inode.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 0733abff77a5..69982e7d7020 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -667,10 +667,8 @@ void afs_evict_inode(struct inode *inode)
 
 	ASSERTCMP(inode->i_ino, ==, vnode->fid.vnode);
 
-	if ((S_ISDIR(inode->i_mode) ||
-	     S_ISLNK(inode->i_mode)) &&
-	    (inode_state_read_once(inode) & I_DIRTY) &&
-	    !sbi->dyn_root) {
+	if ((S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode)) &&
+	    inode_or_folios_dirty(inode) && !sbi->dyn_root) {
 		struct writeback_control wbc = {
 			.sync_mode = WB_SYNC_ALL,
 			.for_sync = true,
-- 
2.53.0

[PATCH 11/13] nfsd: Use inode_or_folios_dirty() for dirtiness check in wait_for_concurrent_writes()

[Jan Kara]

The check for dirtiness in wait_for_concurrent_writes() is racy because
I_DIRTY_PAGES flag can be temporarily cleared by
__writeback_single_inode(). Use inode_or_folios_dirty() helper to avoid
that. This also makes sure inode timestamps properly persisted when
lazytime is enabled.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/nfsd/vfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 56c49b5bc5bb..a314ca39272f 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1249,7 +1249,7 @@ static int wait_for_concurrent_writes(struct file *file)
 		dprintk("nfsd: write resume %d\n", task_pid_nr(current));
 	}
 
-	if (inode_state_read_once(inode) & I_DIRTY) {
+	if (inode_or_folios_dirty(inode)) {
 		dprintk("nfsd: write sync %d\n", task_pid_nr(current));
 		err = vfs_fsync(file, 0);
 	}
-- 
2.53.0

[PATCH 12/13] fscrypt: Use inode_or_folios_dirty() instead of explicit state check

[Jan Kara]

Use inode_or_folios_dirty() helper instead of explicit inode state
check. No functional change.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/crypto/keysetup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index a77b889d7578..1cd40663b251 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -839,7 +839,7 @@ int fscrypt_drop_inode(struct inode *inode)
 	 * userspace is still using the files, inodes can be dirtied between
 	 * then and now.  We mustn't lose any writes, so skip dirty inodes here.
 	 */
-	if (inode_state_read(inode) & I_DIRTY_ALL)
+	if (inode_or_folios_dirty(inode))
 		return 0;
 
 	/*
-- 
2.53.0

[PATCH 13/13] fs: Use inode_or_folios_dirty() in __inode_lru_list_add()

[Jan Kara]

Use inode_or_folios_dirty() helper instead of explicit i_state check in
__inode_lru_list_add(). No functional change.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/inode.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/inode.c b/fs/inode.c
index f63535ee3629..73afa57fe385 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -552,7 +552,9 @@ static void __inode_lru_list_add(struct inode *inode, bool rotate)
 {
 	lockdep_assert_held(&inode->i_lock);
 
-	if (inode_state_read(inode) & (I_DIRTY_ALL | I_SYNC | I_FREEING | I_WILL_FREE))
+	if (inode_state_read(inode) & (I_SYNC | I_FREEING | I_WILL_FREE))
+		return;
+	if (inode_or_folios_dirty(inode))
 		return;
 	if (icount_read(inode))
 		return;
-- 
2.53.0

Re: [PATCH 11/13] nfsd: Use inode_or_folios_dirty() for dirtiness check in wait_for_concurrent_writes()

[Jeff Layton]

On Fri, 2026-11-13 at 09:47 +0100, Jan Kara wrote:
> The check for dirtiness in wait_for_concurrent_writes() is racy because
> I_DIRTY_PAGES flag can be temporarily cleared by
> __writeback_single_inode(). Use inode_or_folios_dirty() helper to avoid
> that. This also makes sure inode timestamps properly persisted when
> lazytime is enabled.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/nfsd/vfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 56c49b5bc5bb..a314ca39272f 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1249,7 +1249,7 @@ static int wait_for_concurrent_writes(struct file *file)
>  		dprintk("nfsd: write resume %d\n", task_pid_nr(current));
>  	}
>  
> -	if (inode_state_read_once(inode) & I_DIRTY) {
> +	if (inode_or_folios_dirty(inode)) {
>  		dprintk("nfsd: write sync %d\n", task_pid_nr(current));
>  		err = vfs_fsync(file, 0);
>  	}

Reviewed-by: Jeff Layton <jlayton@kernel.org>