Adding Direct IO Support

Adding O_DIRECT support to ZFS to bypass the ARC for writes/reads.

O_DIRECT support in ZFS will always ensure there is coherency between
buffered and O_DIRECT IO requests. This ensures that all IO requests,
whether buffered or direct, will see the same file contents at all
times. Just as in other FS's , O_DIRECT does not imply O_SYNC. While
data is written directly to VDEV disks, metadata will not be synced
until the associated  TXG is synced.
For both O_DIRECT read and write request the offset and request sizes,
at a minimum, must be PAGE_SIZE aligned. In the event they are not,
then EINVAL is returned unless the direct property is set to always (see
below).

For O_DIRECT writes:
The request also must be block aligned (recordsize) or the write
request will take the normal (buffered) write path. In the event that
request is block aligned and a cached copy of the buffer in the ARC,
then it will be discarded from the ARC forcing all further reads to
retrieve the data from disk.

For O_DIRECT reads:
The only alignment restrictions are PAGE_SIZE alignment. In the event
that the requested data is in buffered (in the ARC) it will just be
copied from the ARC into the user buffer.

For both O_DIRECT writes and reads the O_DIRECT flag will be ignored in
the event that file contents are mmap'ed. In this case, all requests
that are at least PAGE_SIZE aligned will just fall back to the buffered
paths. If the request however is not PAGE_SIZE aligned, EINVAL will
be returned as always regardless if the file's contents are mmap'ed.

Since O_DIRECT writes go through the normal ZIO pipeline, the
following operations are supported just as with normal buffered writes:
Checksum
Compression
Encryption
Erasure Coding
There is one caveat for the data integrity of O_DIRECT writes that is
distinct for each of the OS's supported by ZFS.
FreeBSD - FreeBSD is able to place user pages under write protection so
          any data in the user buffers and written directly down to the
	  VDEV disks is guaranteed to not change. There is no concern
	  with data integrity and O_DIRECT writes.
Linux - Linux is not able to place anonymous user pages under write
        protection. Because of this, if the user decides to manipulate
	the page contents while the write operation is occurring, data
	integrity can not be guaranteed. However, there is a module
	parameter `zfs_vdev_direct_write_verify` that controls the
	if a O_DIRECT writes that can occur to a top-level VDEV before
	a checksum verify is run before the contents of the I/O buffer
        are committed to disk. In the event of a checksum verification
	failure the write will return EIO. The number of O_DIRECT write
	checksum verification errors can be observed by doing
	`zpool status -d`, which will list all verification errors that
	have occurred on a top-level VDEV. Along with `zpool status`, a
	ZED event will be issues as `dio_verify` when a checksum
	verification error occurs.

ZVOLs and dedup is not currently supported with Direct I/O.

A new dataset property `direct` has been added with the following 3
allowable values:
disabled - Accepts O_DIRECT flag, but silently ignores it and treats
	   the request as a buffered IO request.
standard - Follows the alignment restrictions  outlined above for
	   write/read IO requests when the O_DIRECT flag is used.
always   - Treats every write/read IO request as though it passed
           O_DIRECT and will do O_DIRECT if the alignment restrictions
	   are met otherwise will redirect through the ARC. This
	   property will not allow a request to fail.

There is also a module parameter zfs_dio_enabled that can be used to
force all reads and writes through the ARC. By setting this module
parameter to 0, it mimics as if the  direct dataset property is set to
disabled.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Brian Atkinson <[email protected]>
Co-authored-by: Mark Maybee <[email protected]>
Co-authored-by: Matt Macy <[email protected]>
Co-authored-by: Brian Behlendorf <[email protected]>
Closes #10018 

5 files changed, 399 insertions, 15 deletions

diff --git a/module/os/freebsd/spl/spl_uio.c b/module/os/freebsd/spl/spl_uio.c
index 17886cbeb..74cbe36bb 100644
--- a/module/os/freebsd/spl/spl_uio.c
+++ b/module/os/freebsd/spl/spl_uio.c
@@ -44,6 +44,10 @@
 #include <sys/uio_impl.h>
 #include <sys/vnode.h>
 #include <sys/zfs_znode.h>
+#include <sys/byteorder.h>
+#include <sys/lock.h>
+#include <sys/vm.h>
+#include <vm/vm_map.h>
 
 static void
 zfs_freeuio(struct uio *uio)
@@ -115,3 +119,200 @@ zfs_uio_fault_move(void *p, size_t n, zfs_uio_rw_t dir, zfs_uio_t *uio)
 	ASSERT3U(zfs_uio_rw(uio), ==, dir);
 	return (vn_io_fault_uiomove(p, n, GET_UIO_STRUCT(uio)));
 }
+
+/*
+ * Check if the uio is page-aligned in memory.
+ */
+boolean_t
+zfs_uio_page_aligned(zfs_uio_t *uio)
+{
+	const struct iovec *iov = GET_UIO_STRUCT(uio)->uio_iov;
+
+	for (int i = zfs_uio_iovcnt(uio); i > 0; iov++, i--) {
+		uintptr_t addr = (uintptr_t)iov->iov_base;
+		size_t size = iov->iov_len;
+		if ((addr & (PAGE_SIZE - 1)) || (size & (PAGE_SIZE - 1))) {
+				return (B_FALSE);
+		}
+	}
+
+	return (B_TRUE);
+}
+
+static void
+zfs_uio_set_pages_to_stable(zfs_uio_t *uio)
+{
+	ASSERT3P(uio->uio_dio.pages, !=, NULL);
+	ASSERT3S(uio->uio_dio.npages, >, 0);
+
+	for (int i = 0; i < uio->uio_dio.npages; i++) {
+		vm_page_t page = uio->uio_dio.pages[i];
+		ASSERT3P(page, !=, NULL);
+
+		MPASS(page == PHYS_TO_VM_PAGE(VM_PAGE_TO_PHYS(page)));
+		vm_page_busy_acquire(page, VM_ALLOC_SBUSY);
+		pmap_remove_write(page);
+	}
+}
+
+static void
+zfs_uio_release_stable_pages(zfs_uio_t *uio)
+{
+	ASSERT3P(uio->uio_dio.pages, !=, NULL);
+	for (int i = 0; i < uio->uio_dio.npages; i++) {
+		vm_page_t page = uio->uio_dio.pages[i];
+
+		ASSERT3P(page, !=, NULL);
+		vm_page_sunbusy(page);
+	}
+}
+
+/*
+ * If the operation is marked as read, then we are stating the pages will be
+ * written to and must be given write access.
+ */
+static int
+zfs_uio_hold_pages(unsigned long start, size_t len, int nr_pages,
+    zfs_uio_rw_t rw, vm_page_t *pages)
+{
+	vm_map_t map;
+	vm_prot_t prot;
+	int count;
+
+	map = &curthread->td_proc->p_vmspace->vm_map;
+	ASSERT3S(len, >, 0);
+
+	prot = rw == UIO_READ ? (VM_PROT_READ | VM_PROT_WRITE) : VM_PROT_READ;
+	count = vm_fault_quick_hold_pages(map, start, len, prot, pages,
+	    nr_pages);
+
+	return (count);
+}
+
+void
+zfs_uio_free_dio_pages(zfs_uio_t *uio, zfs_uio_rw_t rw)
+{
+	ASSERT(uio->uio_extflg & UIO_DIRECT);
+	ASSERT3P(uio->uio_dio.pages, !=, NULL);
+	ASSERT(zfs_uio_rw(uio) == rw);
+
+	if (rw == UIO_WRITE)
+		zfs_uio_release_stable_pages(uio);
+
+	vm_page_unhold_pages(&uio->uio_dio.pages[0],
+	    uio->uio_dio.npages);
+
+	kmem_free(uio->uio_dio.pages,
+	    uio->uio_dio.npages * sizeof (vm_page_t));
+}
+
+static int
+zfs_uio_get_user_pages(unsigned long start, int nr_pages,
+    size_t len, zfs_uio_rw_t rw, vm_page_t *pages)
+{
+	int count;
+
+	count = zfs_uio_hold_pages(start, len, nr_pages, rw, pages);
+
+	if (count != nr_pages) {
+		if (count > 0)
+			vm_page_unhold_pages(pages, count);
+		return (0);
+	}
+
+	ASSERT3S(count, ==, nr_pages);
+
+	return (count);
+}
+
+static int
+zfs_uio_iov_step(struct iovec v, zfs_uio_t *uio, int *numpages)
+{
+	unsigned long addr = (unsigned long)(v.iov_base);
+	size_t len = v.iov_len;
+	int n = DIV_ROUND_UP(len, PAGE_SIZE);
+
+	int res = zfs_uio_get_user_pages(
+	    P2ALIGN_TYPED(addr, PAGE_SIZE, unsigned long), n, len,
+	    zfs_uio_rw(uio), &uio->uio_dio.pages[uio->uio_dio.npages]);
+
+	if (res != n)
+		return (SET_ERROR(EFAULT));
+
+	ASSERT3U(len, ==, res * PAGE_SIZE);
+	*numpages = res;
+	return (0);
+}
+
+static int
+zfs_uio_get_dio_pages_impl(zfs_uio_t *uio)
+{
+	const struct iovec *iovp = GET_UIO_STRUCT(uio)->uio_iov;
+	size_t len = zfs_uio_resid(uio);
+
+	for (int i = 0; i < zfs_uio_iovcnt(uio); i++) {
+		struct iovec iov;
+		int numpages = 0;
+
+		if (iovp->iov_len == 0) {
+			iovp++;
+			continue;
+		}
+		iov.iov_len = MIN(len, iovp->iov_len);
+		iov.iov_base = iovp->iov_base;
+		int error = zfs_uio_iov_step(iov, uio, &numpages);
+
+		if (error)
+			return (error);
+
+		uio->uio_dio.npages += numpages;
+		len -= iov.iov_len;
+		iovp++;
+	}
+
+	ASSERT0(len);
+
+	return (0);
+}
+
+/*
+ * This function holds user pages into the kernel. In the event that the user
+ * pages are not successfully held an error value is returned.
+ *
+ * On success, 0 is returned.
+ */
+int
+zfs_uio_get_dio_pages_alloc(zfs_uio_t *uio, zfs_uio_rw_t rw)
+{
+	int error = 0;
+	int npages = DIV_ROUND_UP(zfs_uio_resid(uio), PAGE_SIZE);
+	size_t size = npages * sizeof (vm_page_t);
+
+	ASSERT(zfs_uio_rw(uio) == rw);
+
+	uio->uio_dio.pages = kmem_alloc(size, KM_SLEEP);
+
+	error = zfs_uio_get_dio_pages_impl(uio);
+
+	if (error) {
+		vm_page_unhold_pages(&uio->uio_dio.pages[0],
+		    uio->uio_dio.npages);
+		kmem_free(uio->uio_dio.pages, size);
+		return (error);
+	}
+
+	ASSERT3S(uio->uio_dio.npages, >, 0);
+
+	/*
+	 * Since we will be writing the user pages we must make sure that
+	 * they are stable. That way the contents of the pages can not change
+	 * while we are doing: compression, checksumming, encryption, parity
+	 * calculations or deduplication.
+	 */
+	if (zfs_uio_rw(uio) == UIO_WRITE)
+		zfs_uio_set_pages_to_stable(uio);
+
+	uio->uio_extflg |= UIO_DIRECT;
+
+	return (0);
+}
diff --git a/module/os/freebsd/zfs/abd_os.c b/module/os/freebsd/zfs/abd_os.c
index f24ea3dc7..f20dc5d8c 100644
--- a/module/os/freebsd/zfs/abd_os.c
+++ b/module/os/freebsd/zfs/abd_os.c
@@ -32,6 +32,7 @@
 #include <sys/zio.h>
 #include <sys/zfs_context.h>
 #include <sys/zfs_znode.h>
+#include <sys/vm.h>
 
 typedef struct abd_stats {
 	kstat_named_t abdstat_struct_size;
@@ -135,7 +136,9 @@ abd_size_alloc_linear(size_t size)
 void
 abd_update_scatter_stats(abd_t *abd, abd_stats_op_t op)
 {
-	uint_t n = abd_scatter_chunkcnt(abd);
+	uint_t n;
+
+	n = abd_scatter_chunkcnt(abd);
 	ASSERT(op == ABDSTAT_INCR || op == ABDSTAT_DECR);
 	int waste = (n << PAGE_SHIFT) - abd->abd_size;
 	if (op == ABDSTAT_INCR) {
@@ -198,10 +201,16 @@ abd_free_chunks(abd_t *abd)
 {
 	uint_t i, n;
 
-	n = abd_scatter_chunkcnt(abd);
-	for (i = 0; i < n; i++) {
-		kmem_cache_free(abd_chunk_cache,
-		    ABD_SCATTER(abd).abd_chunks[i]);
+	/*
+	 * Scatter ABDs may be constructed by abd_alloc_from_pages() from
+	 * an array of pages. In which case they should not be freed.
+	 */
+	if (!abd_is_from_pages(abd)) {
+		n = abd_scatter_chunkcnt(abd);
+		for (i = 0; i < n; i++) {
+			kmem_cache_free(abd_chunk_cache,
+			    ABD_SCATTER(abd).abd_chunks[i]);
+		}
 	}
 }
 
@@ -342,11 +351,8 @@ abd_fini(void)
 void
 abd_free_linear_page(abd_t *abd)
 {
-	/*
-	 * FreeBSD does not have scatter linear pages
-	 * so there is an error.
-	 */
-	VERIFY(0);
+	ASSERT3P(abd->abd_u.abd_linear.sf, !=, NULL);
+	zfs_unmap_page(abd->abd_u.abd_linear.sf);
 }
 
 /*
@@ -365,6 +371,26 @@ abd_alloc_for_io(size_t size, boolean_t is_metadata)
 	return (abd_alloc_linear(size, is_metadata));
 }
 
+static abd_t *
+abd_get_offset_from_pages(abd_t *abd, abd_t *sabd, size_t chunkcnt,
+    size_t new_offset)
+{
+	ASSERT(abd_is_from_pages(sabd));
+
+	/*
+	 * Set the child child chunks to point at the parent chunks as
+	 * the chunks are just pages and we don't want to copy them.
+	 */
+	size_t parent_offset = new_offset / PAGE_SIZE;
+	ASSERT3U(parent_offset, <, abd_scatter_chunkcnt(sabd));
+	for (int i = 0; i < chunkcnt; i++)
+		ABD_SCATTER(abd).abd_chunks[i] =
+		    ABD_SCATTER(sabd).abd_chunks[parent_offset + i];
+
+	abd->abd_flags |= ABD_FLAG_FROM_PAGES;
+	return (abd);
+}
+
 abd_t *
 abd_get_offset_scatter(abd_t *abd, abd_t *sabd, size_t off,
     size_t size)
@@ -399,6 +425,11 @@ abd_get_offset_scatter(abd_t *abd, abd_t *sabd, size_t off,
 
 	ABD_SCATTER(abd).abd_offset = new_offset & PAGE_MASK;
 
+	if (abd_is_from_pages(sabd)) {
+		return (abd_get_offset_from_pages(abd, sabd, chunkcnt,
+		    new_offset));
+	}
+
 	/* Copy the scatterlist starting at the correct offset */
 	(void) memcpy(&ABD_SCATTER(abd).abd_chunks,
 	    &ABD_SCATTER(sabd).abd_chunks[new_offset >> PAGE_SHIFT],
@@ -408,6 +439,44 @@ abd_get_offset_scatter(abd_t *abd, abd_t *sabd, size_t off,
 }
 
 /*
+ * Allocate a scatter ABD structure from user pages.
+ */
+abd_t *
+abd_alloc_from_pages(vm_page_t *pages, unsigned long offset, uint64_t size)
+{
+	VERIFY3U(size, <=, DMU_MAX_ACCESS);
+	ASSERT3U(offset, <, PAGE_SIZE);
+	ASSERT3P(pages, !=, NULL);
+
+	abd_t *abd = abd_alloc_struct(size);
+	abd->abd_flags |= ABD_FLAG_OWNER | ABD_FLAG_FROM_PAGES;
+	abd->abd_size = size;
+
+	if ((offset + size) <= PAGE_SIZE) {
+		/*
+		 * There is only a single page worth of data, so we will just
+		 * use  a linear ABD. We have to make sure to take into account
+		 * the offset though. In all other cases our offset will be 0
+		 * as we are always PAGE_SIZE aligned.
+		 */
+		abd->abd_flags |= ABD_FLAG_LINEAR | ABD_FLAG_LINEAR_PAGE;
+		ABD_LINEAR_BUF(abd) = (char *)zfs_map_page(pages[0],
+		    &abd->abd_u.abd_linear.sf) + offset;
+	} else {
+		ABD_SCATTER(abd).abd_offset = offset;
+		ASSERT0(ABD_SCATTER(abd).abd_offset);
+
+		/*
+		 * Setting the ABD's abd_chunks to point to the user pages.
+		 */
+		for (int i = 0; i < abd_chunkcnt_for_bytes(size); i++)
+			ABD_SCATTER(abd).abd_chunks[i] = pages[i];
+	}
+
+	return (abd);
+}
+
+/*
  * Initialize the abd_iter.
  */
 void
@@ -468,6 +537,16 @@ abd_iter_map(struct abd_iter *aiter)
 	if (abd_is_linear(abd)) {
 		aiter->iter_mapsize = abd->abd_size - offset;
 		paddr = ABD_LINEAR_BUF(abd);
+	} else if (abd_is_from_pages(abd)) {
+		aiter->sf = NULL;
+		offset += ABD_SCATTER(abd).abd_offset;
+		size_t index = offset / PAGE_SIZE;
+		offset &= PAGE_MASK;
+		aiter->iter_mapsize = MIN(PAGE_SIZE - offset,
+		    abd->abd_size - aiter->iter_pos);
+		paddr = zfs_map_page(
+		    ABD_SCATTER(aiter->iter_abd).abd_chunks[index],
+		    &aiter->sf);
 	} else {
 		offset += ABD_SCATTER(abd).abd_offset;
 		paddr = ABD_SCATTER(abd).abd_chunks[offset >> PAGE_SHIFT];
@@ -490,6 +569,12 @@ abd_iter_unmap(struct abd_iter *aiter)
 		ASSERT3U(aiter->iter_mapsize, >, 0);
 	}
 
+	if (abd_is_from_pages(aiter->iter_abd) &&
+	    !abd_is_linear_page(aiter->iter_abd)) {
+		ASSERT3P(aiter->sf, !=, NULL);
+		zfs_unmap_page(aiter->sf);
+	}
+
 	aiter->iter_mapaddr = NULL;
 	aiter->iter_mapsize = 0;
 }
@@ -499,3 +584,67 @@ abd_cache_reap_now(void)
 {
 	kmem_cache_reap_soon(abd_chunk_cache);
 }
+
+/*
+ * Borrow a raw buffer from an ABD without copying the contents of the ABD
+ * into the buffer. If the ABD is scattered, this will alloate a raw buffer
+ * whose contents are undefined. To copy over the existing data in the ABD, use
+ * abd_borrow_buf_copy() instead.
+ */
+void *
+abd_borrow_buf(abd_t *abd, size_t n)
+{
+	void *buf;
+	abd_verify(abd);
+	ASSERT3U(abd->abd_size, >=, 0);
+	if (abd_is_linear(abd)) {
+		buf = abd_to_buf(abd);
+	} else {
+		buf = zio_buf_alloc(n);
+	}
+#ifdef ZFS_DEBUG
+	(void) zfs_refcount_add_many(&abd->abd_children, n, buf);
+#endif
+	return (buf);
+}
+
+void *
+abd_borrow_buf_copy(abd_t *abd, size_t n)
+{
+	void *buf = abd_borrow_buf(abd, n);
+	if (!abd_is_linear(abd)) {
+		abd_copy_to_buf(buf, abd, n);
+	}
+	return (buf);
+}
+
+/*
+ * Return a borrowed raw buffer to an ABD. If the ABD is scattered, this will
+ * no change the contents of the ABD and will ASSERT that you didn't modify
+ * the buffer since it was borrowed. If you want any changes you made to buf to
+ * be copied back to abd, use abd_return_buf_copy() instead.
+ */
+void
+abd_return_buf(abd_t *abd, void *buf, size_t n)
+{
+	abd_verify(abd);
+	ASSERT3U(abd->abd_size, >=, n);
+#ifdef ZFS_DEBUG
+	(void) zfs_refcount_remove_many(&abd->abd_children, n, buf);
+#endif
+	if (abd_is_linear(abd)) {
+		ASSERT3P(buf, ==, abd_to_buf(abd));
+	} else {
+		ASSERT0(abd_cmp_buf(abd, buf, n));
+		zio_buf_free(buf, n);
+	}
+}
+
+void
+abd_return_buf_copy(abd_t *abd, void *buf, size_t n)
+{
+	if (!abd_is_linear(abd)) {
+		abd_copy_from_buf(abd, buf, n);
+	}
+	abd_return_buf(abd, buf, n);
+}
diff --git a/module/os/freebsd/zfs/zfs_racct.c b/module/os/freebsd/zfs/zfs_racct.c
index 883255bc1..2989a9af9 100644
--- a/module/os/freebsd/zfs/zfs_racct.c
+++ b/module/os/freebsd/zfs/zfs_racct.c
@@ -27,7 +27,7 @@
 #include <sys/racct.h>
 
 void
-zfs_racct_read(uint64_t size, uint64_t iops)
+zfs_racct_read(spa_t *spa, uint64_t size, uint64_t iops, uint32_t flags)
 {
 	curthread->td_ru.ru_inblock += iops;
 #ifdef RACCT
@@ -40,10 +40,12 @@ zfs_racct_read(uint64_t size, uint64_t iops)
 #else
 	(void) size;
 #endif /* RACCT */
+
+	spa_iostats_read_add(spa, size, iops, flags);
 }
 
 void
-zfs_racct_write(uint64_t size, uint64_t iops)
+zfs_racct_write(spa_t *spa, uint64_t size, uint64_t iops, uint32_t flags)
 {
 	curthread->td_ru.ru_oublock += iops;
 #ifdef RACCT
@@ -56,4 +58,6 @@ zfs_racct_write(uint64_t size, uint64_t iops)
 #else
 	(void) size;
 #endif /* RACCT */
+
+	spa_iostats_write_add(spa, size, iops, flags);
 }
diff --git a/module/os/freebsd/zfs/zfs_vnops_os.c b/module/os/freebsd/zfs/zfs_vnops_os.c
index 01b964f98..5dbca10a3 100644
--- a/module/os/freebsd/zfs/zfs_vnops_os.c
+++ b/module/os/freebsd/zfs/zfs_vnops_os.c
@@ -4131,7 +4131,7 @@ zfs_putpages(struct vnode *vp, vm_page_t *ma, size_t len, int flags,
 		 * but that would make the locking messier
 		 */
 		zfs_log_write(zfsvfs->z_log, tx, TX_WRITE, zp, off,
-		    len, commit, NULL, NULL);
+		    len, commit, B_FALSE, NULL, NULL);
 
 		zfs_vmobject_wlock(object);
 		for (i = 0; i < ncount; i++) {
@@ -4266,6 +4266,8 @@ ioflags(int ioflags)
 		flags |= O_APPEND;
 	if (ioflags & IO_NDELAY)
 		flags |= O_NONBLOCK;
+	if (ioflags & IO_DIRECT)
+		flags |= O_DIRECT;
 	if (ioflags & IO_SYNC)
 		flags |= O_SYNC;
 
@@ -4285,9 +4287,36 @@ static int
 zfs_freebsd_read(struct vop_read_args *ap)
 {
 	zfs_uio_t uio;
+	int error = 0;
 	zfs_uio_init(&uio, ap->a_uio);
-	return (zfs_read(VTOZ(ap->a_vp), &uio, ioflags(ap->a_ioflag),
-	    ap->a_cred));
+	error = zfs_read(VTOZ(ap->a_vp), &uio, ioflags(ap->a_ioflag),
+	    ap->a_cred);
+	/*
+	 * XXX We occasionally get an EFAULT for Direct I/O reads on
+	 * FreeBSD 13. This still needs to be resolved. The EFAULT comes
+	 * from:
+	 * zfs_uio_get__dio_pages_alloc() ->
+	 * zfs_uio_get_dio_pages_impl() ->
+	 * zfs_uio_iov_step() ->
+	 * zfs_uio_get_user_pages().
+	 * We return EFAULT from zfs_uio_iov_step(). When a Direct I/O
+	 * read fails to map in the user pages (returning EFAULT) the
+	 * Direct I/O request is broken up into two separate IO requests
+	 * and issued separately using Direct I/O.
+	 */
+#ifdef ZFS_DEBUG
+	if (error == EFAULT && uio.uio_extflg & UIO_DIRECT) {
+#if 0
+		printf("%s(%d): Direct I/O read returning EFAULT "
+		    "uio = %p, zfs_uio_offset(uio) = %lu "
+		    "zfs_uio_resid(uio) = %lu\n",
+		    __FUNCTION__, __LINE__, &uio, zfs_uio_offset(&uio),
+		    zfs_uio_resid(&uio));
+#endif
+	}
+
+#endif
+	return (error);
 }
 
 #ifndef _SYS_SYSPROTO_H_
diff --git a/module/os/freebsd/zfs/zvol_os.c b/module/os/freebsd/zfs/zvol_os.c
index ddb20b031..c3be4730d 100644
--- a/module/os/freebsd/zfs/zvol_os.c
+++ b/module/os/freebsd/zfs/zvol_os.c
@@ -922,6 +922,7 @@ zvol_cdev_write(struct cdev *dev, struct uio *uio_s, int ioflag)
 	if (commit)
 		zil_commit(zv->zv_zilog, ZVOL_OBJ);
 	rw_exit(&zv->zv_suspend_lock);
+
 	return (error);
 }