mbox series

[GIT,PULL] Improve iomap async dio performance

Message ID 647e79f4-ddaa-7003-6e00-f31e11535082@kernel.dk (mailing list archive)
State Superseded, archived
Headers show
Series [GIT,PULL] Improve iomap async dio performance | expand

Pull-request

git://git.kernel.dk/linux.git tags/xfs-async-dio

Message

Jens Axboe July 21, 2023, 4:54 p.m. UTC
Hi,

Here's the pull request for improving async dio performance with
iomap. Contains a few generic cleanups as well, but the meat of it
is described in the tagged commit message below.

Please pull for 6.6!


The following changes since commit ccff6d117d8dc8d8d86e8695a75e5f8b01e573bf:

  Merge tag 'perf-tools-fixes-for-v6.5-1-2023-07-18' of git://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools (2023-07-18 14:51:29 -0700)

are available in the Git repository at:

  git://git.kernel.dk/linux.git tags/xfs-async-dio

for you to fetch changes up to 901a0c0e4248aa7e3ab4dce9a8c67c47215e6ccc:

  iomap: use an unsigned type for IOMAP_DIO_* defines (2023-07-21 10:34:42 -0600)

----------------------------------------------------------------
iomap always punts async dio write completions to a workqueue, which has
a cost in terms of efficiency (now you need an unrelated worker to
process it) and latency (now you're bouncing a completion through an
async worker, which is a classic slowdown scenario).

Even for writes that should, in theory, be able to complete inline, if
we race with truncate or need to invalidate pages post completion, we
cannot sanely be in IRQ context as the locking types don't allow for
that.

io_uring handles IRQ completions via task_work, and for writes that
don't need to do extra IO at completion time, we can safely complete
them inline from that. This patchset adds IOCB_DIO_CALLER_COMP, which an
IO issuer can set to inform the completion side that any extra work that
needs doing for that completion can be punted to a safe task context.

The iomap dio completion will happen in hard/soft irq context, and we
need a saner context to process these completions. IOCB_DIO_CALLER_COMP
is added, which can be set in a struct kiocb->ki_flags by the issuer. If
the completion side of the iocb handling understands this flag, it can
choose to set a kiocb->dio_complete() handler and just call ki_complete
from IRQ context. The issuer must then ensure that this callback is
processed from a task. io_uring punts IRQ completions to task_work
already, so it's trivial wire it up to run more of the completion before
posting a CQE. This is good for up to a 37% improvement in
throughput/latency for low queue depth IO, patch 5 has the details.

If we need to do real work at completion time, iomap will clear the
IOMAP_DIO_CALLER_COMP flag.

This work came about when Andres tested low queue depth dio writes for
postgres and compared it to doing sync dio writes, showing that the
async processing slows us down a lot.

----------------------------------------------------------------
Jens Axboe (9):
      iomap: cleanup up iomap_dio_bio_end_io()
      iomap: add IOMAP_DIO_INLINE_COMP
      iomap: treat a write through cache the same as FUA
      iomap: completed polled IO inline
      iomap: only set iocb->private for polled bio
      fs: add IOCB flags related to passing back dio completions
      io_uring/rw: add write support for IOCB_DIO_CALLER_COMP
      iomap: support IOCB_DIO_CALLER_COMP
      iomap: use an unsigned type for IOMAP_DIO_* defines

 fs/iomap/direct-io.c | 166 +++++++++++++++++++++++++++++++++++++++------------
 include/linux/fs.h   |  35 ++++++++++-
 io_uring/rw.c        |  26 +++++++-
 3 files changed, 183 insertions(+), 44 deletions(-)

Comments

Dave Chinner July 21, 2023, 10:08 p.m. UTC | #1
On Fri, Jul 21, 2023 at 10:54:41AM -0600, Jens Axboe wrote:
> Hi,
> 
> Here's the pull request for improving async dio performance with
> iomap. Contains a few generic cleanups as well, but the meat of it
> is described in the tagged commit message below.
> 
> Please pull for 6.6!

Ah, I just reviewed v4 (v5 came out while I was sleeping) and I
think there are still problems with some of the logic...

So it might be worth holding off from pulling this until we work
through that...

-Dave.
Darrick J. Wong July 22, 2023, 1:02 a.m. UTC | #2
On Sat, Jul 22, 2023 at 08:08:51AM +1000, Dave Chinner wrote:
> On Fri, Jul 21, 2023 at 10:54:41AM -0600, Jens Axboe wrote:
> > Hi,
> > 
> > Here's the pull request for improving async dio performance with
> > iomap. Contains a few generic cleanups as well, but the meat of it
> > is described in the tagged commit message below.
> > 
> > Please pull for 6.6!
> 
> Ah, I just reviewed v4 (v5 came out while I was sleeping) and I
> think there are still problems with some of the logic...
> 
> So it might be worth holding off from pulling this until we work
> through that...

No worries, next week I'm starting the iomap merging process with
willy's large folios write (Monday) and then Ritesh's dirty tracking
(Tuesday if nothing blows up), so there's plenty of time for more
questions.

--D

> -Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
Jens Axboe July 22, 2023, 2:53 a.m. UTC | #3
On 7/21/23 4:08?PM, Dave Chinner wrote:
> On Fri, Jul 21, 2023 at 10:54:41AM -0600, Jens Axboe wrote:
>> Hi,
>>
>> Here's the pull request for improving async dio performance with
>> iomap. Contains a few generic cleanups as well, but the meat of it
>> is described in the tagged commit message below.
>>
>> Please pull for 6.6!
> 
> Ah, I just reviewed v4 (v5 came out while I was sleeping) and I
> think there are still problems with some of the logic...
> 
> So it might be worth holding off from pulling this until we work
> through that...

No problem, thanks for taking a look! I'll address your feedback
tomorrow and I can always just send another pull request.