[PATCH RFC] ext4: never fall back to buffered I/O for atomic direct writes

[PATCH RFC] ext4: never fall back to buffered I/O for atomic direct writes

[Tal Zussman]

ext4_dio_write_iter() finishes a short direct write through the page
cache. For an atomic write that path only warns and then continues,
which would tear the write.

This came out of a block device fix [1], where an IOCB_ATOMIC direct
write could complete short when only part of the buffer could be
pinned, with the rest going through the buffered fallback. Christoph
Hellwig noted that ext4 has the same fallback and only warns in it [2],
and John Garry agreed it should reject the write instead [3].

In contrast to the block device, nothing reaches the branch on ext4
today: iomap requires the mapping to cover the whole atomic write, a
buffer that can't be pinned in full fails in iomap before the bio is
submitted, and a failed page cache invalidation returns -EAGAIN rather
than reaching the buffered fallback. Exclude atomic writes from the
fallback outright instead of WARN()-ing inside it, so any future bug
yields an error rather than a torn write with a WARN().

Link: https://lore.kernel.org/linux-block/20260828-blkdev-fixes-v2-0-32f3f40cebed@columbia.edu/ [1]
Link: https://lore.kernel.org/linux-block/20260907071217.GD934@lst.de/ [2]
Link: https://lore.kernel.org/linux-block/a7752d1e-8ec1-44d1-a266-c287c315e8ab@oracle.com/ [3]
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
 fs/ext4/file.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 374b4bc25bd5..fb561c701d29 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -666,17 +666,16 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	else
 		inode_unlock(inode);
 
-	if (ret >= 0 && iov_iter_count(from)) {
+	/*
+	 * Buffered I/O provides no torn-write protection, so never finish an
+	 * atomic write through the page cache. iomap should not return a short
+	 * atomic write anyway.
+	 */
+	if (ret >= 0 && iov_iter_count(from) &&
+	    !(iocb->ki_flags & IOCB_ATOMIC)) {
 		ssize_t err;
 		loff_t endbyte;
 
-		/*
-		 * There is no support for atomic writes on buffered-io yet,
-		 * we should never fallback to buffered-io for DIO atomic
-		 * writes.
-		 */
-		WARN_ON_ONCE(iocb->ki_flags & IOCB_ATOMIC);
-
 		offset = iocb->ki_pos;
 		err = ext4_buffered_write_iter(iocb, from);
 		if (err < 0)

---
base-commit: df2908090cda368b01ff43709f51890076c56157
change-id: 20260909-ext4-atomic-no-fallback-b1550a7a3fcf

Best regards,
--  
Tal Zussman <tz2294@columbia.edu>

Re: [PATCH RFC] ext4: never fall back to buffered I/O for atomic direct writes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The patch fails to return an error for short atomic writes, silently returning a torn write instead.

Pre-existing issues:
- [High] Silent fallback to non-atomic buffered I/O early in the function when `ext4_should_use_dio()` returns false.
--

commit 4f1077482a95547df7986c873d9a1c2cff493fea
Author: Tal Zussman <tz2294@columbia.edu>
ext4: never fall back to buffered I/O for atomic direct writes

This commit modifies the direct I/O path in ext4 to explicitly prevent falling
back to buffered I/O for atomic writes. It intends to return an error for short
atomic writes instead of completing them through the page cache and issuing
a warning.

Link: https://lore.kernel.org/linux-block/20260828-blkdev-fixes-v2-0-32f3f40cebed@columbia.edu/ [1]
Link: https://lore.kernel.org/linux-block/20260907071217.GD934@lst.de/ [2]
Link: https://lore.kernel.org/linux-block/a7752d1e-8ec1-44d1-a266-c287c315e8ab@oracle.com/ [3]

> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 374b4bc25bd5d..fb561c701d291 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -666,17 +666,16 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  	else
>  		inode_unlock(inode);
>  
> -	if (ret >= 0 && iov_iter_count(from)) {
> +	/*
> +	 * Buffered I/O provides no torn-write protection, so never finish an
> +	 * atomic write through the page cache. iomap should not return a short
> +	 * atomic write anyway.
> +	 */
> +	if (ret >= 0 && iov_iter_count(from) &&
> +	    !(iocb->ki_flags & IOCB_ATOMIC)) {

[Severity: High]
Does this unintentionally return a short torn write instead of an error?

The commit message states the patch "yields an error rather than a torn
write with a WARN()". However, if iomap_dio_rw() completes a short atomic
write, where ret is greater than 0 but less than the requested count, we
skip this fallback block entirely because !(iocb->ki_flags & IOCB_ATOMIC)
evaluates to false.

Since we skip the block, the function proceeds to the end and returns the
positive short byte count in ret to the VFS. This exposes a torn write to
userspace without returning an error. Should we explicitly return an error
like -EIO or -EFAULT when a short atomic write occurs?

>  		ssize_t err;
>  		loff_t endbyte;
>  
> -		/*
> -		 * There is no support for atomic writes on buffered-io yet,
> -		 * we should never fallback to buffered-io for DIO atomic
> -		 * writes.
> -		 */
> -		WARN_ON_ONCE(iocb->ki_flags & IOCB_ATOMIC);
> -
>  		offset = iocb->ki_pos;
>  		err = ext4_buffered_write_iter(iocb, from);
>  		if (err < 0)

[Severity: High]
This is a pre-existing issue, but it looks like we might silently downgrade
atomic writes to non-atomic buffered writes if the file doesn't support DIO.

Earlier in ext4_dio_write_iter(), there is an initial fallback:

    /* Fallback to buffered I/O if the inode does not support direct I/O. */
    if (!ext4_should_use_dio(iocb, from)) {
        if (ilock_shared)
            inode_unlock_shared(inode);
        else
            inode_unlock(inode);
        return ext4_buffered_write_iter(iocb, from);
    }

If a user requests an atomic write on an ext4 file that does not support
DIO (such as a file with data journaling enabled), ext4_should_use_dio()
will return false. This causes an early fallback to
ext4_buffered_write_iter().

Because ext4_buffered_write_iter() does not check for IOCB_ATOMIC, it
proceeds with a standard buffered write via generic_perform_write().
Does this silently break the atomicity guarantees expected by userspace
for such files?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-ext4-atomic-no-fallback-v1-1-5a6b9539c002@columbia.edu?part=1

Re: [PATCH RFC] ext4: never fall back to buffered I/O for atomic direct writes

[Jan Kara]

On Wed 09-09-26 16:56:17, Tal Zussman wrote:
> ext4_dio_write_iter() finishes a short direct write through the page
> cache. For an atomic write that path only warns and then continues,
> which would tear the write.
> 
> This came out of a block device fix [1], where an IOCB_ATOMIC direct
> write could complete short when only part of the buffer could be
> pinned, with the rest going through the buffered fallback. Christoph
> Hellwig noted that ext4 has the same fallback and only warns in it [2],
> and John Garry agreed it should reject the write instead [3].
> 
> In contrast to the block device, nothing reaches the branch on ext4
> today: iomap requires the mapping to cover the whole atomic write, a
> buffer that can't be pinned in full fails in iomap before the bio is
> submitted, and a failed page cache invalidation returns -EAGAIN rather
> than reaching the buffered fallback. Exclude atomic writes from the
> fallback outright instead of WARN()-ing inside it, so any future bug
> yields an error rather than a torn write with a WARN().
> 
> Link: https://lore.kernel.org/linux-block/20260828-blkdev-fixes-v2-0-32f3f40cebed@columbia.edu/ [1]
> Link: https://lore.kernel.org/linux-block/20260907071217.GD934@lst.de/ [2]
> Link: https://lore.kernel.org/linux-block/a7752d1e-8ec1-44d1-a266-c287c315e8ab@oracle.com/ [3]
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>

I agree your patch makes things better but I have a question: are short
writes (even 0) allowed for atomic writes? I don't think so as far as my
understanding of atomic writes goes (and based on what I could find in the
code). But after your change we'd just return short write (including ret ==
0) now which doesn't look ideal and also doesn't match your "so any future
bug yields an error rather than a torn write with a WARN()" description. So
as a futureproofing I'd maybe suggest just:

	if (WARN_ON_ONCE(iocb->ki_flags & IOCB_ATOMIC && ret >= 0 &&
			 iov_iter_count(from)))
		ret = -EIO; /* or maybe -ENOPROTOOPT??? */

before the fallback to buffered IO as a more robust solution.

								Honza

> ---
>  fs/ext4/file.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 374b4bc25bd5..fb561c701d29 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -666,17 +666,16 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  	else
>  		inode_unlock(inode);
>  
> -	if (ret >= 0 && iov_iter_count(from)) {
> +	/*
> +	 * Buffered I/O provides no torn-write protection, so never finish an
> +	 * atomic write through the page cache. iomap should not return a short
> +	 * atomic write anyway.
> +	 */
> +	if (ret >= 0 && iov_iter_count(from) &&
> +	    !(iocb->ki_flags & IOCB_ATOMIC)) {
>  		ssize_t err;
>  		loff_t endbyte;
>  
> -		/*
> -		 * There is no support for atomic writes on buffered-io yet,
> -		 * we should never fallback to buffered-io for DIO atomic
> -		 * writes.
> -		 */
> -		WARN_ON_ONCE(iocb->ki_flags & IOCB_ATOMIC);
> -
>  		offset = iocb->ki_pos;
>  		err = ext4_buffered_write_iter(iocb, from);
>  		if (err < 0)
> 
> ---
> base-commit: df2908090cda368b01ff43709f51890076c56157
> change-id: 20260909-ext4-atomic-no-fallback-b1550a7a3fcf
> 
> Best regards,
> --  
> Tal Zussman <tz2294@columbia.edu>
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH RFC] ext4: never fall back to buffered I/O for atomic direct writes

[Tal Zussman]

On 9/10/26 6:58 AM, Jan Kara wrote:
> I agree your patch makes things better but I have a question: are short
> writes (even 0) allowed for atomic writes? I don't think so as far as my
> understanding of atomic writes goes (and based on what I could find in the
> code). But after your change we'd just return short write (including ret ==
> 0) now which doesn't look ideal and also doesn't match your "so any future
> bug yields an error rather than a torn write with a WARN()" description. So
> as a futureproofing I'd maybe suggest just:
> 
> 	if (WARN_ON_ONCE(iocb->ki_flags & IOCB_ATOMIC && ret >= 0 &&
> 			 iov_iter_count(from)))
> 		ret = -EIO; /* or maybe -ENOPROTOOPT??? */
> 
> before the fallback to buffered IO as a more robust solution.
> 

Short writes shouldn't be allowed, so this is definitely better in terms of
future-proofing. I'll do this with EIO but with the if (WARN_ON_ONCE())
inside the existing ret and iov_iter_count() check to avoid duplicating the
checks.

It looks like Sashiko also found a pre-existing issue where an atomic request
can be done non-atomically [1]. I'll see if I can reproduce it and add a fix
as a second patch in v2.

Thanks,
Tal

[1] https://sashiko.dev/#/patchset/20260909-ext4-atomic-no-fallback-v1-1-5a6b9539c002%40columbia.edu?part=1

Re: [PATCH RFC] ext4: never fall back to buffered I/O for atomic direct writes

[Ojaswin Mujoo]

On Sun, Sep 13, 2026 at 07:18:37PM -0400, Tal Zussman wrote:
> On 9/10/26 6:58 AM, Jan Kara wrote:
> > I agree your patch makes things better but I have a question: are short
> > writes (even 0) allowed for atomic writes? I don't think so as far as my
> > understanding of atomic writes goes (and based on what I could find in the
> > code). But after your change we'd just return short write (including ret ==
> > 0) now which doesn't look ideal and also doesn't match your "so any future
> > bug yields an error rather than a torn write with a WARN()" description. So
> > as a futureproofing I'd maybe suggest just:
> > 
> > 	if (WARN_ON_ONCE(iocb->ki_flags & IOCB_ATOMIC && ret >= 0 &&
> > 			 iov_iter_count(from)))
> > 		ret = -EIO; /* or maybe -ENOPROTOOPT??? */
> > 
> > before the fallback to buffered IO as a more robust solution.
> > 
> 
> Short writes shouldn't be allowed, so this is definitely better in terms of
> future-proofing. I'll do this with EIO but with the if (WARN_ON_ONCE())
> inside the existing ret and iov_iter_count() check to avoid duplicating the
> checks.

Hi Tal, Jan,

AFAICR the WARN_ON was mostly precautionary as we ensure in the dio path
that a during submission we ensure a write will not tear else we return
an error. Sure we can go with Jan's suggestion but  I think either ways,
if we hit that WARN_ON at that point we have already torn the write so
it is actually a BUG in the sense that we have not honored the
RWF_ATOMIC contract, but hopefully that will not happen.

> 
> It looks like Sashiko also found a pre-existing issue where an atomic request
> can be done non-atomically [1]. I'll see if I can reproduce it and add a fix
> as a second patch in v2.

Looking at Sashiko's comment, it does seem like a valid issue. A simple
test would be to create an inode with inline data after mounting with -o
inline_data which should force ext4_should_use_dio() to return false.

Return,
ojaswin

> 
> Thanks,
> Tal


> 
> [1] https://sashiko.dev/#/patchset/20260909-ext4-atomic-no-fallback-v1-1-5a6b9539c002%40columbia.edu?part=1
> 
>