mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Boris Burkov <boris@bur.io>
To: Breno Leitao <leitao@debian.org>
Cc: linux-btrfs@vger.kernel.org, David Sterba <dsterba@suse.com>,
	Chris Mason <mason@kernel.org>, Qu Wenruo <wqu@suse.com>,
	Filipe Manana <fdmanana@kernel.org>,
	Atish Patra <atish.patra@linux.dev>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: bug: btrfs: direct IO fallback racing with in-flight direct IO fails fsync
Date: Thu, 5 Nov 2026 09:53:21 -0800	[thread overview]
Message-ID: <20261105175321.GA3915752@zen.localdomain> (raw)
In-Reply-To: <auxv7Tg0xSq5yHmb@gmail.com>

On Thu, Nov 05, 2026 at 04:07:49AM -0800, Breno Leitao wrote:
> Hi,
> 
> Atish reported that arm64 production hosts (64K pages, 4K blocks) that
> keep VM disk images as NOCOW files on btrfs, opened with O_DIRECT, on
> 7.x based kernels, and it still reproduces in linux-next as of
> next-20261104:
> 
>   Page cache invalidation failure on direct I/O.  Possible data corruption due to collision with buffered I/O!
> 
> Besides the message, dio_warn_stale_pagecache() records -EIO in the
> mapping, so the next fsync() on the image fails. The message blames the
> application for mixing buffered and direct IO, but every write here is
> O_DIRECT: the buffered writes come from btrfs itself, when it falls back
> for writes that are not 4K aligned.
> 
> It does not look like a regression, since the fallback never waited for
> in-flight direct IO. 64K pages make it easy to hit, as a folio then holds
> 16 blocks. Since e9b7b9d9e78e5b ("btrfs: use IOMAP_DIO_BOUNCE flag
> instead of falling back to buffered IO"), COW files are exposed too.
> 
> I was able to come up with a reproducer that hits the issue within
> seconds with 64K pages. With XFS on a loop device backed by such a file,
> standing in for the VM, XFS shuts down on "log I/O error -5":
> 
>         https://github.com/leitao/btrfs-dio-fallback
> 
> My robot identified this as the root cause:
> 
> 1. Task A has an async O_DIRECT write in flight to one 4K block of a 64K
>    folio. iomap invalidated the page cache before submitting it, and the
>    inode lock was dropped.
> 2. Task B issues an O_DIRECT write whose offset, length or buffer is not
>    4K aligned. btrfs_direct_write() falls back to btrfs_buffered_write(),
>    which does not wait for in-flight direct IO, and dirties another block
>    of the same folio.
> 3. Task A's write completes. kiocb_invalidate_post_direct_write() finds
>    the folio dirty, so invalidate_inode_pages2_range() fails with -EBUSY
>    and dio_warn_stale_pagecache() records -EIO. The next fsync() fails.
> 
> ext4 fixed the same race in 15cdefd0c052 ("ext4: drain in-flight DIO
> before buffered write fallback"), and ext2, ntfs and exfat are getting
> the same fix. Doing the same in btrfs_buffered_write() for the fallback
> case makes the reproducer clean, and I can send that patch if it looks
> like the right direction.
> 
> This patch is a copy of the fixes above (from other FS) and make the
> reproducer happy.
> 
> commit 16cd17d1e8205f9f86b8da0a9b1ccefdb2665764
> Author: Breno Leitao <leitao@debian.org>
> Date:   Thu Nov 5 02:59:39 2026 -0800
> 
>     btrfs: drain in-flight direct IO before buffered write fallback
>     
>     When btrfs_direct_write() cannot do a write as direct IO, because the
>     offset, the length or the user buffer is not aligned to the sectorsize,
>     or the range has compressed extents or page cache, it falls back to
>     btrfs_buffered_write(). That dirties the page cache without waiting for
>     direct IO still in flight from other tasks:
>     
>     1) Task A submits an async direct write to block 1 of a folio. iomap
>        invalidates the page cache in the range and submits the bio, and the
>        inode lock is dropped.
>     2) Task B issues a 512 byte O_DIRECT write to block 0 of the same folio.
>        btrfs_direct_write() falls back to btrfs_buffered_write(), which
>        dirties the folio.
>     3) Task A's write completes. kiocb_invalidate_post_direct_write() finds
>        the folio dirty, invalidate_inode_pages2_range() fails with -EBUSY,
>        and dio_warn_stale_pagecache() logs:
>     
>        [    2.048568] Page cache invalidation failure on direct I/O.  Possible data corruption due to collision with buffered I/O!
>        [    2.048574] File: /tmp/diorepro.JTCKpJ/block.img PID: 260 Comm: kworker/11:1
>     
>        It also records -EIO in the mapping, so the next fsync() fails.
>     
>     The two writes do not touch the same bytes, they only share a folio.
>     With 4K blocks on 64K pages a folio holds 16 blocks, so this is easy to
>     hit there. Meta's arm64 hosts with 64K pages hit it with VM disk images
>     that are NOCOW files on btrfs, opened with O_DIRECT. In a test, XFS on a
>     loop device with direct IO on such a file gets EIO on a log write and
>     shuts down within seconds.
>     
>     Wait for in-flight direct IO before the fallback dirties the page cache,
>     as ext4 does since commit 15cdefd0c052 ("ext4: drain in-flight DIO
>     before buffered write fallback"). btrfs_buffered_write() holds the inode
>     lock exclusively there, so no new direct IO can start until the write is
>     done. NOWAIT writes never get there, as btrfs_direct_write() returns
>     -EAGAIN for them before falling back.
>     
>     A reproducer that keeps 4K libaio O_DIRECT writes in flight while other
>     threads issue 512 byte O_DIRECT writes to the same folios of a NOCOW
>     file got 1123 fdatasync() EIOs in 10 seconds on a 64K page VM, and none
>     with this patch.
>     
>     Fixes: f85781fb505e ("btrfs: switch to iomap for direct IO")
>     Reported-by: Atish Patra <atish.patra@linux.dev>
>     Signed-off-by: Breno Leitao <leitao@debian.org>
> 
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index f978c6524aa03f..2017a96814d857 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1334,6 +1334,14 @@ ssize_t btrfs_buffered_write(struct kiocb *iocb, struct iov_iter *iter)
>  	if (ret < 0)
>  		return ret;
>  
> +	/*
> +	 * A direct write that fell back to buffered IO must not dirty folios
> +	 * while direct IO is in flight: its completion fails to invalidate
> +	 * them and records -EIO in the mapping.
> +	 */
> +	if (iocb->ki_flags & IOCB_DIRECT)
> +		inode_dio_wait(inode);
> +

The explanation and fix make good sense, thanks!

However, thinking out loud, we do know that if we have started the DIO,
we will have an ordered_extent from:

btrfs_dio_iomap_begin()
  btrfs_get_blocks_direct_write()
    btrfs_create_dio_extent()
      btrfs_alloc_ordered_extent()

so perhaps rather than waiting for *all* dio on the inode we can just do
some variant of btrfs_wait_ordered_range() which only waits on dio OEs.

Anyway, that is an enhancement for later, and not necessary for a fix,
so I would be happy with this fix as-is.

Reviewed-by: Boris Burkov <boris@bur.io>

>  	/*
>  	 * We can only trust the isize with inode lock held, or it can race with
>  	 * other buffered writes and cause incorrect call of

  reply	other threads:[~2026-11-05 17:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-05 12:07 Breno Leitao
2026-11-05 17:53 ` Boris Burkov [this message]
2026-11-05 21:45 ` Qu Wenruo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261105175321.GA3915752@zen.localdomain \
    --to=boris@bur.io \
    --cc=atish.patra@linux.dev \
    --cc=dsterba@suse.com \
    --cc=fdmanana@kernel.org \
    --cc=leitao@debian.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mason@kernel.org \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®