fs/ext4/file.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
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>
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
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
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 > >
© 2016 - 2026 Red Hat, Inc.