From 0be3c03002e0b3153565b81e415c97140b2e4b16 Mon Sep 17 00:00:00 2001 From: Jorgen Lundman Date: Tue, 13 Nov 2018 09:09:13 +0900 Subject: [PATCH] Avoid kmem_alloc in IO path There appears to be some issues with too-frequent allocations in spl-kmem and this is especially noticable due to allocations we do for each read or write IO operation. We embed the vdev_buf into zio struct itself, and since ldi_vnode has to call buf_alloc() we also allow ldi_iokit to allocate a local holder. (It is inconvenient to try to include C++ headers in zio.h). If this does speed up IO, it is only because we now avoid the real problem which is still something we should also address --- include/sys/ldi_buf.h | 6 ++++++ include/sys/vdev_impl.h | 10 +--------- include/sys/zio.h | 6 ++++++ module/zfs/ldi_iokit.cpp | 24 ++++++++++++++---------- module/zfs/vdev_disk.c | 7 ++++--- 5 files changed, 31 insertions(+), 22 deletions(-) diff --git a/include/sys/ldi_buf.h b/include/sys/ldi_buf.h index 9b69b0610a..b549dd0e1c 100644 --- a/include/sys/ldi_buf.h +++ b/include/sys/ldi_buf.h @@ -32,6 +32,7 @@ extern "C" { #endif /* __cplusplus */ + /* * Buffer context for LDI strategy */ @@ -50,6 +51,11 @@ typedef struct ldi_buf { uint64_t pad; /* Pad to 64 bytes */ } ldi_buf_t; /* XXX Currently 64b */ +typedef struct vdev_buf { + ldi_buf_t vb_buf; /* buffer that describes the io */ + void *vb_io; /* pointer back to the original zio_t */ +} vdev_buf_t; + ldi_buf_t *ldi_getrbuf(int); void ldi_freerbuf(ldi_buf_t *); void ldi_bioinit(ldi_buf_t *); diff --git a/include/sys/vdev_impl.h b/include/sys/vdev_impl.h index 06061ea501..336314f65c 100644 --- a/include/sys/vdev_impl.h +++ b/include/sys/vdev_impl.h @@ -252,7 +252,7 @@ struct vdev { /* pool checkpoint related */ space_map_t *vdev_checkpoint_sm; /* contains reserved blocks */ - + boolean_t vdev_initialize_exit_wanted; vdev_initializing_state_t vdev_initialize_state; kthread_t *vdev_initialize_thread; @@ -500,14 +500,6 @@ extern boolean_t vdev_obsolete_counts_are_precise(vdev_t *vd); */ int vdev_checkpoint_sm_object(vdev_t *vd); -/* - * The vdev_buf_t is used to translate between zio_t and buf_t, and back again. - */ -typedef struct vdev_buf { - ldi_buf_t vb_buf; /* buffer that describes the io */ - zio_t *vb_io; /* pointer back to the original zio_t */ -} vdev_buf_t; - #ifdef __cplusplus } #endif diff --git a/include/sys/zio.h b/include/sys/zio.h index 444b9ee4dd..76695969ea 100644 --- a/include/sys/zio.h +++ b/include/sys/zio.h @@ -36,6 +36,8 @@ #include #include #include +#include + #ifdef __cplusplus extern "C" { @@ -489,6 +491,10 @@ struct zio { kcondvar_t io_cv; int io_allocator; +#if __APPLE__ + vdev_buf_t io_ldi_buf; +#endif + /* FMA state */ zio_cksum_report_t *io_cksum_report; uint64_t io_ena; diff --git a/module/zfs/ldi_iokit.cpp b/module/zfs/ldi_iokit.cpp index e6f83561d0..c724731ee6 100644 --- a/module/zfs/ldi_iokit.cpp +++ b/module/zfs/ldi_iokit.cpp @@ -50,6 +50,8 @@ #include #include +#include + /* * ZFS internal */ @@ -1176,10 +1178,10 @@ media_from_path(const char *path = 0) /* Define an IOKit buffer for buf_strategy_iokit */ typedef struct ldi_iokit_buf { - IOMemoryDescriptor *iomem; - IOStorageCompletion iocompletion; - IOStorageAttributes ioattr; -} ldi_iokit_buf_t; /* XXX Currently 64b */ + IOMemoryDescriptor *iomem; + IOStorageCompletion iocompletion; + IOStorageAttributes ioattr; +} ldi_iokit_buf_t; /* XXX Currently 64b */ /* Completion handler for IOKit strategy */ static void @@ -1188,7 +1190,6 @@ ldi_iokit_io_intr(void *target, void *parameter, { ldi_iokit_buf_t *iobp = (ldi_iokit_buf_t *)target; ldi_buf_t *lbp = (ldi_buf_t *)parameter; - #ifdef DEBUG /* In debug builds, verify buffer pointers */ ASSERT3U(lbp, !=, 0); @@ -1243,7 +1244,8 @@ ldi_iokit_io_intr(void *target, void *parameter, } /* Free IOKit buffer */ - kmem_free(iobp, sizeof (ldi_iokit_buf_t)); + //kmem_free(iobp, sizeof (ldi_iokit_buf_t)); + FREE(iobp, M_TEMP); /* Call original completion function */ if (lbp->b_iodone) { @@ -1291,6 +1293,7 @@ buf_sync_strategy_iokit(ldi_buf_t *lbp, ldi_iokit_buf_t *iobp, * IOStorageAttributes * attributes = 0, * UInt64 * actualByteCount = 0); */ + int buf_strategy_iokit(ldi_buf_t *lbp, struct ldi_handle *lhp) { @@ -1309,8 +1312,9 @@ buf_strategy_iokit(ldi_buf_t *lbp, struct ldi_handle *lhp) #endif /* DEBUG */ /* Allocate an IOKit buffer */ - iobp = (ldi_iokit_buf_t *)kmem_alloc(sizeof (ldi_iokit_buf_t), - KM_SLEEP); + //iobp = (ldi_iokit_buf_t *)MALLOC(sizeof (ldi_iokit_buf_t)); + MALLOC(iobp, ldi_iokit_buf_t *, sizeof (ldi_iokit_buf_t),M_TEMP,M_WAITOK); + if (!iobp) { dprintf("%s couldn't allocate buf_iokit_t\n", __func__); return (ENOMEM); @@ -1346,7 +1350,7 @@ buf_strategy_iokit(ldi_buf_t *lbp, struct ldi_handle *lhp) if (iobp->iomem) { iobp->iomem->release(); } - kmem_free(iobp, sizeof (ldi_iokit_buf_t)); + //kmem_free(iobp, sizeof (ldi_iokit_buf_t)); return (ENOMEM); } @@ -1355,7 +1359,7 @@ buf_strategy_iokit(ldi_buf_t *lbp, struct ldi_handle *lhp) dprintf("%s device not online\n", __func__); iobp->iomem->complete(); iobp->iomem->release(); - kmem_free(iobp, sizeof (ldi_iokit_buf_t)); + //kmem_free(iobp, sizeof (ldi_iokit_buf_t)); return (ENODEV); } diff --git a/module/zfs/vdev_disk.c b/module/zfs/vdev_disk.c index 936cb9c682..2262cef821 100644 --- a/module/zfs/vdev_disk.c +++ b/module/zfs/vdev_disk.c @@ -711,7 +711,7 @@ vdev_disk_io_intr(ldi_buf_t *bp) 0, zio->io_size, zio->io_abd->abd_size); } - kmem_free(vb, sizeof (vdev_buf_t)); + //kmem_free(vb, sizeof (vdev_buf_t)); zio_delay_interrupt(zio); } @@ -838,7 +838,8 @@ vdev_disk_io_start(zio_t *zio) zio->io_target_timestamp = zio_handle_io_delay(zio); - vb = kmem_alloc(sizeof (vdev_buf_t), KM_SLEEP); +// vb = kmem_alloc(sizeof (vdev_buf_t), KM_SLEEP); + vb = &(zio->io_ldi_buf); vb->vb_io = zio; bp = &vb->vb_buf; @@ -892,7 +893,7 @@ vdev_disk_io_start(zio_t *zio) if (error != 0) { dprintf("%s error from ldi_strategy %d\n", __func__, error); zio->io_error = EIO; - kmem_free(vb, sizeof (vdev_buf_t)); + //kmem_free(vb, sizeof (vdev_buf_t)); zio_execute(zio); // zio_interrupt(zio); }