[PATCH 0/5] fs: drain in-flight DIO before buffered writes

[PATCH 0/5] fs: drain in-flight DIO before buffered writes

[Jiale Yao]

An asynchronous direct write can remain in flight after its submitting
thread releases the filesystem's I/O submission lock. If a buffered write
dirties page cache before that direct write completes, post-I/O page-cache
invalidation can find dirty pages, report possible data corruption, and
record -EIO in the mapping error sequence.

Drain in-flight DIO before buffered writes in the generic filemap path,
F2FS, XFS, and zonefs. The filemap change also protects UDF and other
users of __generic_file_write_iter(). Keep IOCB_NOWAIT requests
nonblocking by returning -EAGAIN when a drain would be required.

The patches are separated by common code or filesystem and remain
independently buildable in series order.

Jiale Yao (5):
  filemap: drain in-flight DIO before buffered writes
  f2fs: drain in-flight DIO before buffered writes
  xfs: drain in-flight DIO before buffered writes
  xfs: drain in-flight DIO before zoned buffered writes
  zonefs: drain in-flight DIO before buffered writes

 fs/f2fs/file.c    | 13 +++++++++++--
 fs/xfs/xfs_file.c | 22 ++++++++++++++++++++++
 fs/zonefs/file.c  |  8 ++++++++
 mm/filemap.c      |  9 +++++++++
 4 files changed, 50 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 1/5] filemap: drain in-flight DIO before buffered writes

[Jiale Yao]

An asynchronous direct write can remain in flight after its submitting
thread releases the inode lock. A buffered write can then dirty page
cache before the direct write completes its post-I/O invalidation. The
invalidation finds dirty pages, reports a possible data corruption, and
records -EIO in the mapping error sequence.

Drain in-flight DIO before both regular buffered writes and direct I/O
fallbacks in __generic_file_write_iter(). Preserve IOCB_NOWAIT semantics
by returning -EAGAIN when waiting would be required, while retaining a
positive partial direct write result.

Fixes: 8174202b34c3 ("write_iter variants of {__,}generic_file_aio_write()")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 mm/filemap.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/mm/filemap.c b/mm/filemap.c
index b74bc1e5015c..289b8ca44ada 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -4502,10 +4502,19 @@ ssize_t __generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 		 */
 		if (ret < 0 || !iov_iter_count(from) || IS_DAX(inode))
 			return ret;
+		if ((iocb->ki_flags & IOCB_NOWAIT) &&
+		    atomic_read(&inode->i_dio_count))
+			return ret ?: -EAGAIN;
+		inode_dio_wait(inode);
 		return direct_write_fallback(iocb, from, ret,
 				generic_perform_write(iocb, from));
 	}
 
+	if ((iocb->ki_flags & IOCB_NOWAIT) &&
+	    atomic_read(&inode->i_dio_count))
+		return -EAGAIN;
+	inode_dio_wait(inode);
+
 	return generic_perform_write(iocb, from);
 }
 EXPORT_SYMBOL(__generic_file_write_iter);
-- 
2.34.1

[PATCH 5/5] zonefs: drain in-flight DIO before buffered writes

[Jiale Yao]

Conventional zone files allow both direct and buffered writes. An
asynchronous direct write can remain active after its submitting thread
releases the inode lock. A buffered write can then dirty page cache before
direct I/O completion performs post-write invalidation, causing the
invalidation to fail and record -EIO in the mapping error sequence.

After taking the exclusive inode lock, wait for active DIO before running
buffered write checks. Return -EAGAIN when an IOCB_NOWAIT request would
otherwise block.

Fixes: 4008e2a0b01a ("zonefs: Reorganize code")
Cc: stable@vger.kernel.org
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 fs/zonefs/file.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/zonefs/file.c b/fs/zonefs/file.c
index 5b34849be7a2..8635a795c24c 100644
--- a/fs/zonefs/file.c
+++ b/fs/zonefs/file.c
@@ -576,6 +576,14 @@ static ssize_t zonefs_file_buffered_write(struct kiocb *iocb,
 		inode_lock(inode);
 	}
 
+	if (atomic_read(&inode->i_dio_count)) {
+		if (iocb->ki_flags & IOCB_NOWAIT) {
+			ret = -EAGAIN;
+			goto inode_unlock;
+		}
+		inode_dio_wait(inode);
+	}
+
 	ret = zonefs_write_checks(iocb, from);
 	if (ret <= 0)
 		goto inode_unlock;
-- 
2.34.1

Re: [PATCH 1/5] filemap: drain in-flight DIO before buffered writes

[Jan Kara]

On Sun 25-10-26 20:41:10, Jiale Yao wrote:
> An asynchronous direct write can remain in flight after its submitting
> thread releases the inode lock. A buffered write can then dirty page
> cache before the direct write completes its post-I/O invalidation. The
> invalidation finds dirty pages, reports a possible data corruption, and
> records -EIO in the mapping error sequence.

Well, when you do stupid things, you get stupid results. We were always
saying mixing buffered and direct IO in the same file isn't a good idea and
we try to not screw users too badly but give no promises. This falls into
that cathegory... So I don't think there's much to fix here. The only
sensible exception here are simple filesystems which have only simple
support for direct IO and fallback to buffered IO for the rest. But I think
we should handle these on a case-by-case basis and either drop the support
for direct IO altogether or convert them to iomap and properly handle the
waiting in their .write_iter methods (like e.g. ext2 does it).

								Honza

> Drain in-flight DIO before both regular buffered writes and direct I/O
> fallbacks in __generic_file_write_iter(). Preserve IOCB_NOWAIT semantics
> by returning -EAGAIN when waiting would be required, while retaining a
> positive partial direct write result.
> 
> Fixes: 8174202b34c3 ("write_iter variants of {__,}generic_file_aio_write()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  mm/filemap.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index b74bc1e5015c..289b8ca44ada 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -4502,10 +4502,19 @@ ssize_t __generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  		 */
>  		if (ret < 0 || !iov_iter_count(from) || IS_DAX(inode))
>  			return ret;
> +		if ((iocb->ki_flags & IOCB_NOWAIT) &&
> +		    atomic_read(&inode->i_dio_count))
> +			return ret ?: -EAGAIN;
> +		inode_dio_wait(inode);
>  		return direct_write_fallback(iocb, from, ret,
>  				generic_perform_write(iocb, from));
>  	}
>  
> +	if ((iocb->ki_flags & IOCB_NOWAIT) &&
> +	    atomic_read(&inode->i_dio_count))
> +		return -EAGAIN;
> +	inode_dio_wait(inode);
> +
>  	return generic_perform_write(iocb, from);
>  }
>  EXPORT_SYMBOL(__generic_file_write_iter);
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 1/5] filemap: drain in-flight DIO before buffered writes

[Christoph Hellwig]

On Mon, Oct 26, 2026 at 02:31:31PM +0100, Jan Kara wrote:
> > An asynchronous direct write can remain in flight after its submitting
> > thread releases the inode lock. A buffered write can then dirty page
> > cache before the direct write completes its post-I/O invalidation. The
> > invalidation finds dirty pages, reports a possible data corruption, and
> > records -EIO in the mapping error sequence.
> 
> Well, when you do stupid things, you get stupid results. We were always
> saying mixing buffered and direct IO in the same file isn't a good idea and
> we try to not screw users too badly but give no promises. This falls into
> that cathegory... So I don't think there's much to fix here.

We have declared it stupid, but people do it anyway.  See all the nfsd
patches floating around for in-kernel consumers doing this heavy
and fine grained mixing.  So I think we should get this right, and we
should also have test in xfstests to verify the beavior instead of just
shooting into the sky.  (related: I only see patches 1 and 5 in my
fsdevel inbox, something is messed up with the series)

> The only
> sensible exception here are simple filesystems which have only simple
> support for direct IO and fallback to buffered IO for the rest. But I think
> we should handle these on a case-by-case basis and either drop the support
> for direct IO altogether or convert them to iomap and properly handle the
> waiting in their .write_iter methods (like e.g. ext2 does it).

That's a bit too easy.  There actually is a much better option, which
unfortunately is much more invasive:  kill i_dio_wait and move to
non-owner unlocking of i_rwsem from the asynchronous completion handler.
That gets us sane locking behavior without any footguns.  I really
regret adding i_dio_wait in hindsight - yes, the PREEMPT_RT folks
hat locks that aren't bound to tasks, but we ended up with a few of
those anyway and thus have the primitives, and file read/write is a
case where they really fit.