linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Jan Kara <jack@suse.cz>, Keith Busch <kbusch@kernel.org>,
	Dave Chinner <david@fromorbit.com>,
	Carlos Maiolino <cem@kernel.org>,
	Christian Brauner <brauner@kernel.org>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-kernel@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-raid@vger.kernel.org,
	linux-block@vger.kernel.org
Subject: Re: fall back from direct to buffered I/O when stable writes are required
Date: Thu, 13 Nov 2025 18:39:07 +0100	[thread overview]
Message-ID: <aRYXuwtSQUz6buBs@redhat.com> (raw)
In-Reply-To: <20251103122111.GA17600@lst.de>

Am 03.11.2025 um 13:21 hat Christoph Hellwig geschrieben:
> On Mon, Nov 03, 2025 at 12:14:06PM +0100, Jan Kara wrote:
> > I also think the performance cost of the unconditional bounce buffering is
> > so heavy that it's just a polite way of pushing the app to do proper IO
> > buffer synchronization itself (assuming it cares about IO performance but
> > given it bothered with direct IO it presumably does). 
> >
> > So the question is how to get out of this mess with the least disruption
> > possible which IMO also means providing easy way for well-behaved apps to
> > avoid the overhead.
> 
> Remember the cases where this matters is checksumming and parity, where
> we touch all the cache lines anyway and consume the DRAM bandwidth,
> although bounce buffering upgrades this from pure reads to also writes.
> So the overhead is heavy, but if we handle it the right way, that is
> doing the checksum/parity calculation while the cache line is still hot
> it should not be prohibitive.  And getting this right in the direct
> I/O code means that the low-level code could stop bounce buffering
> for buffered I/O, providing a major speedup there.
> 
> I've been thinking a bit more on how to better get the copy close to the
> checksumming at least for PI, and to avoid the extra copies for RAID5
> buffered I/O. M maybe a better way is to mark a bio as trusted/untrusted
> so that the checksumming/raid code can bounce buffer it, and I start to
> like that idea.

This feels like the right idea to me. It's also what I thought of after
reading your problem description.

The problem is not that RAID5 uses bounce buffers. That's the correct
and safe thing to do when you don't know that the buffer can't change.
I'd argue changing that would be a RAID5 bug, and the corruption you
showed earlier in the thread is not a sign of a buggy filesystem or
application [1], but that you told the device to operate incorrectly.

What is the problem is that it still uses bounce buffers when you do
know that the buffer can't change. Then it's just wasteful and doesn't
contribute to correctness.

Passing down a flag to the device so that it can decide whether the
bounce buffer is needed seems like the obvious solution for that.

> A complication is that PI could relax that requirement if we support
> PI passthrough from userspace (currently only for block device, but I
> plan to add file system support), where the device checks it, but we
> can't do that for parity RAID.

Not sure I understand the problem here. If it's passed through from
userspace, isn't its validity the problem of userspace, too? I'd expect
that you only need a bounce buffer in the kernel if the kernel itself
does something like a checksum calculation?

Kevin

[1] For a QEMU developer like me, not blaming the application may sound
    like an excuse, but we're really only in the same position as the
    kernel here for anything that comes from the guest. Whenever we rely
    on stable buffers, we already have to use bounce buffers, too.


  parent reply	other threads:[~2025-11-13 17:39 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-29  7:15 fall back from direct to buffered I/O when stable writes are required Christoph Hellwig
2025-10-29  7:15 ` [PATCH 1/4] fs: replace FOP_DIO_PARALLEL_WRITE with a fmode bits Christoph Hellwig
2025-10-29 16:01   ` Darrick J. Wong
2025-11-04  7:00   ` Nirjhar Roy (IBM)
2025-11-05 14:04     ` Christoph Hellwig
2025-11-11  9:44   ` Christian Brauner
2025-10-29  7:15 ` [PATCH 2/4] fs: return writeback errors for IOCB_DONTCACHE in generic_write_sync Christoph Hellwig
2025-10-29 16:01   ` Darrick J. Wong
2025-10-29 16:37     ` Christoph Hellwig
2025-10-29 18:12       ` Darrick J. Wong
2025-10-30  5:59         ` Christoph Hellwig
2025-11-04 12:04       ` Nirjhar Roy (IBM)
2025-11-04 15:53         ` Christoph Hellwig
2025-10-29  7:15 ` [PATCH 3/4] xfs: use IOCB_DONTCACHE when falling back to buffered writes Christoph Hellwig
2025-10-29 15:57   ` Darrick J. Wong
2025-11-04 12:33   ` Nirjhar Roy (IBM)
2025-11-04 15:52     ` Christoph Hellwig
2025-10-29  7:15 ` [PATCH 4/4] xfs: fallback to buffered I/O for direct I/O when stable writes are required Christoph Hellwig
2025-10-29 15:53   ` Darrick J. Wong
2025-10-29 16:35     ` Christoph Hellwig
2025-10-29 21:23       ` Qu Wenruo
2025-10-30  5:58         ` Christoph Hellwig
2025-10-30  6:37           ` Qu Wenruo
2025-10-30  6:49             ` Christoph Hellwig
2025-10-30  6:53               ` Qu Wenruo
2025-10-30  6:55                 ` Christoph Hellwig
2025-10-30  7:14                   ` Qu Wenruo
2025-10-30  7:17                     ` Christoph Hellwig
2025-11-10 13:38   ` Nirjhar Roy (IBM)
2025-11-10 13:59     ` Christoph Hellwig
2025-11-12  7:13       ` Nirjhar Roy (IBM)
2025-10-29 15:58 ` fall back from direct to buffered " Bart Van Assche
2025-10-29 16:14   ` Darrick J. Wong
2025-10-29 16:33   ` Christoph Hellwig
2025-10-30 11:20 ` Dave Chinner
2025-10-30 12:00   ` Geoff Back
2025-10-30 12:54     ` Jan Kara
2025-10-30 14:35     ` Christoph Hellwig
2025-10-30 22:02     ` Dave Chinner
2025-10-30 14:33   ` Christoph Hellwig
2025-10-30 23:18     ` Dave Chinner
2025-10-31 13:00       ` Christoph Hellwig
2025-10-31 15:57         ` Keith Busch
2025-10-31 16:47           ` Christoph Hellwig
2025-11-03 11:14             ` Jan Kara
2025-11-03 12:21               ` Christoph Hellwig
2025-11-03 22:47                 ` Keith Busch
2025-11-04 23:38                 ` Darrick J. Wong
2025-11-05 14:11                   ` Christoph Hellwig
2025-11-05 21:44                     ` Darrick J. Wong
2025-11-06  9:50                       ` Johannes Thumshirn
2025-11-06 12:49                         ` hch
2025-11-12 14:18                           ` Ming Lei
2025-11-12 14:38                             ` hch
2025-11-13 17:39                 ` Kevin Wolf [this message]
2025-11-14  5:39                   ` Christoph Hellwig
2025-11-14  9:29                     ` Kevin Wolf
2025-11-14 12:01                       ` Christoph Hellwig
2025-11-14 12:31                         ` Kevin Wolf
2025-11-14 15:36                           ` Christoph Hellwig
2025-11-14 16:55                             ` Kevin Wolf

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=aRYXuwtSQUz6buBs@redhat.com \
    --to=kwolf@redhat.com \
    --cc=brauner@kernel.org \
    --cc=cem@kernel.org \
    --cc=david@fromorbit.com \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=kbusch@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=martin.petersen@oracle.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;
as well as URLs for NNTP newsgroup(s).