Updating based on PR Feedback(6)

1. Updated typo in man page zfs.4.
2. Fix fat fingered typing errors in zpl_aio_write().
3. Fixed spelling O_DIRECT typo in zpl_direct_IO_impl()
4. Updated dmu_write_uio_dnode() to issue a write_size based multiple
   dn->dn_datablksz chunks at once.
5. Removed empty lines in zfs_write().
6. Returned code back to same indentation in zfs_get_data().
7. Removed duplicate ASSERT statements in dmu_buf_will_clone_or_dio().
8. Fixed spelling typo of cause in comment in
   dmu_buf_will_clone_or_dio().
9. Return 0 in FreeBSD zfs_uio_get_pages() when count != nr_pages.
10. Updated FreeBSD zfs_uio_get_dio_pages_alloc() to unhold pages in
    the event of an error.
11. Linux changed zfs_uio_iov_step() to use SET_ERROR() so it matches
    the FreeBSD implementation.
12. Upated zfs_read() to add back dio_remaining_resid to n in the event
    of an error.
13. Added an ASSERT in zio_ddt_write() making sure no Direct I/O writes
    are issued with deduplication. Also, added a comment with ASSERT to
    state why Direct I/O writes can not use deduplication.
14. Removed _KERNEL include guard around zfs_dio_page_aligned(). The
    proper uio_impl.h or uio.h is included through zfs_context.h.
15. Updated the zfsprop man page to state that Direct I/O is not
   currenntly available for zvols. Also that Direct I/O writes are
   incompatiable with dedup, so dedup will not take place with Direct
   I/O writes.
16. Fixed typo in comment in manipulate_user_buffer.c.

Signed-off-by: Brian Atkinson <[email protected]>

Author: bwatkinson

include/sys/uio_impl.h

@@ -49,14 +49,12 @@ extern void zfs_uio_free_dio_pages(zfs_uio_t *, zfs_uio_rw_t);
 extern int zfs_uio_get_dio_pages_alloc(zfs_uio_t *, zfs_uio_rw_t);
 extern boolean_t zfs_uio_page_aligned(zfs_uio_t *);
 
-#ifdef _KERNEL
 static inline boolean_t
 zfs_dio_page_aligned(void *buf)
 {
 	return ((((uintptr_t)(buf) & (PAGESIZE - 1)) == 0) ?
 	    B_TRUE : B_FALSE);
 }
-#endif
 
 static inline boolean_t
 zfs_dio_offset_aligned(uint64_t offset, uint64_t blksz)

man/man4/zfs.4

@@ -424,7 +424,7 @@ May be increased up to
 .Sy ASHIFT_MAX Po 16 Pc ,
 but this may negatively impact pool space efficiency.
 .
-.It Sy zfs_vdev_direct_write_verify Ns = Ns Sy Linux 1 | FreeBSED 0 Pq uint
+.It Sy zfs_vdev_direct_write_verify Ns = Ns Sy Linux 1 | FreeBSD 0 Pq uint
 If non-zero, then a Direct I/O write's checksum will be verified every
 time the write is issued and before it is commited to the block pointer.
 In the event the checksum is not valid then the I/O operation will return EIO.

man/man7/zfsprops.7

@@ -1076,6 +1076,10 @@ Furthermore, if an application uses
 .Xr mmap 2
 based file access then in order to maintain coherency all direct requests
 are converted to buffered requests while the file is mapped.
+Currently Direct I/O is not supported with zvols.
+If dedup is enabled on a dataset, Direct I/O writes will not check for
+deduplication.
+Deduplication and Direct I/O writes are currently incompatible.
 .It Xo
 .Sy dnodesize Ns = Ns Sy legacy Ns | Ns Sy auto Ns | Ns Sy 1k Ns | Ns
 .Sy 2k Ns | Ns Sy 4k Ns | Ns Sy 8k Ns | Ns Sy 16k

module/os/freebsd/spl/spl_uio.c

@@ -217,7 +217,7 @@ zfs_uio_get_user_pages(unsigned long start, int nr_pages,
 	if (count != nr_pages) {
 		if (count > 0)
 			vm_page_unhold_pages(pages, count);
-		return (count);
+		return (0);
 	}
 
 	ASSERT3S(count, ==, nr_pages);
@@ -295,6 +295,8 @@ zfs_uio_get_dio_pages_alloc(zfs_uio_t *uio, zfs_uio_rw_t rw)
 	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);
 	}

module/os/linux/zfs/zfs_uio.c

@@ -599,9 +599,9 @@ zfs_uio_iov_step(struct iovec v, zfs_uio_rw_t rw, zfs_uio_t *uio,
 	    P2ALIGN_TYPED(addr, PAGE_SIZE, unsigned long), n, rw == UIO_READ,
 	    &uio->uio_dio.pages[uio->uio_dio.npages]);
 	if (res < 0) {
-		return (-res);
+		return (SET_ERROR(-res));
 	} else if (len != (res * PAGE_SIZE)) {
-		return (EFAULT);
+		return (SET_ERROR(EFAULT));
 	}
 
 	ASSERT3S(len, ==, res * PAGE_SIZE);
@@ -632,7 +632,7 @@ zfs_uio_get_dio_pages_iov(zfs_uio_t *uio, zfs_uio_rw_t rw)
 		int error = zfs_uio_iov_step(iov, rw, uio, &numpages);
 
 		if (error)
-			return (SET_ERROR(error));
+			return (error);
 
 		uio->uio_dio.npages += numpages;
 		len -= iov.iov_len;

module/os/linux/zfs/zpl_file.c

@@ -454,7 +454,7 @@ zpl_aio_write(struct kiocb *kiocb, const struct iovec *iov,
 	if (ret)
 		return (ret);
 
-	ret = geeric_write_checks(filep, &pos, &count, S_ISBLK(ip->i_mode));
+	ret = generic_write_checks(filp, &pos, &count, S_ISBLK(ip->i_mode));
 	if (ret)
 		return (ret);
 
@@ -488,7 +488,7 @@ static ssize_t
 zpl_direct_IO_impl(void)
 {
 	/*
-	 * All O_DIRCT requests should be handled by
+	 * All O_DIRECT requests should be handled by
 	 * zpl_{iter/aio}_{write/read}(). There is no way kernel generic code
 	 * should call the direct_IO address_space_operations function. We set
 	 * this code path to be fatal if it is executed.

module/zfs/dbuf.c

@@ -2794,14 +2794,11 @@ dmu_buf_untransform_direct(dmu_buf_impl_t *db, spa_t *spa)
 void
 dmu_buf_will_clone_or_dio(dmu_buf_t *db_fake, dmu_tx_t *tx)
 {
-	dmu_buf_impl_t *db = (dmu_buf_impl_t *)db_fake;
-	ASSERT0(db->db_level);
-	ASSERT(db->db_blkid != DMU_BONUS_BLKID);
-	ASSERT(db->db.db_object != DMU_META_DNODE_OBJECT);
-
 	/*
 	 * Block clones and Direct I/O writes always happen in open-context.
 	 */
+	dmu_buf_impl_t *db = (dmu_buf_impl_t *)db_fake;
+	ASSERT0(db->db_level);
 	ASSERT(!dmu_tx_is_syncing(tx));
 	ASSERT0(db->db_level);
 	ASSERT(db->db_blkid != DMU_BONUS_BLKID);
@@ -2820,7 +2817,7 @@ dmu_buf_will_clone_or_dio(dmu_buf_t *db_fake, dmu_tx_t *tx)
 	 * to go ahead free up the space accounting through dbuf_undirty() ->
 	 * dbuf_unoverride() -> zio_free(). Space accountiung for determining
 	 * if a write can occur in zfs_write() happens through dmu_tx_assign().
-	 * This can cuase an issue with Direct I/O writes in the case of
+	 * This can cause an issue with Direct I/O writes in the case of
 	 * overwriting the same block, because all DVA allocations are being
 	 * done in open-context. Constantly allowing Direct I/O overwrites to
 	 * the same block can exhaust the pools available space leading to

module/zfs/dmu.c

@@ -1512,10 +1512,11 @@ dmu_write_uio_dnode(dnode_t *dn, zfs_uio_t *uio, uint64_t size, dmu_tx_t *tx)
 		} else if (write_size > dn->dn_datablksz &&
 		    zfs_dio_offset_aligned(zfs_uio_offset(uio),
 		    dn->dn_datablksz)) {
-			err = dmu_write_uio_direct(dn, uio, dn->dn_datablksz,
-			    tx);
+			write_size =
+			    dn->dn_datablksz * (write_size / dn->dn_datablksz);
+			err = dmu_write_uio_direct(dn, uio, write_size, tx);
 			if (err == 0) {
-				size -= dn->dn_datablksz;
+				size -= write_size;
 				goto top;
 			} else {
 				return (err);

module/zfs/zfs_ioctl.c

@@ -160,7 +160,6 @@
 #include <sys/types.h>
 #include <sys/param.h>
 #include <sys/errno.h>
-#include <sys/uio_impl.h>
 #include <sys/file.h>
 #include <sys/kmem.h>
 #include <sys/cmn_err.h>

module/zfs/zfs_vnops.c

@@ -35,7 +35,6 @@
 #include <sys/time.h>
 #include <sys/sysmacros.h>
 #include <sys/vfs.h>
-#include <sys/uio_impl.h>
 #include <sys/file.h>
 #include <sys/stat.h>
 #include <sys/kmem.h>
@@ -445,7 +444,6 @@ zfs_read(struct znode *zp, zfs_uio_t *uio, int ioflag, cred_t *cr)
 		n -= nbytes;
 	}
 
-	int64_t nread = start_resid;
 	if (error == 0 && (uio->uio_extflg & UIO_DIRECT) &&
 	    dio_remaining_resid != 0) {
 		/*
@@ -464,9 +462,11 @@ zfs_read(struct znode *zp, zfs_uio_t *uio, int ioflag, cred_t *cr)
 		uio->uio_extflg |= UIO_DIRECT;
 
 		if (error != 0)
-			n -= dio_remaining_resid;
+			n += dio_remaining_resid;
+	} else if (error && (uio->uio_extflg & UIO_DIRECT)) {
+		n += dio_remaining_resid;
 	}
-	nread -= n;
+	int64_t nread = start_resid - n;
 
 	dataset_kstats_update_read_kstats(&zfsvfs->z_kstat, nread);
 out:
@@ -634,7 +634,6 @@ zfs_write(znode_t *zp, zfs_uio_t *uio, int ioflag, cred_t *cr)
 		return (SET_ERROR(EFAULT));
 	}
 
-
 	/*
 	 * If in append mode, set the io offset pointer to eof.
 	 */
@@ -670,7 +669,6 @@ zfs_write(znode_t *zp, zfs_uio_t *uio, int ioflag, cred_t *cr)
 		lr = zfs_rangelock_enter(&zp->z_rangelock, woff, n, RL_WRITER);
 	}
 
-
 	if (zn_rlimit_fsize_uio(zp, uio)) {
 		zfs_rangelock_exit(lr);
 		zfs_exit(zfsvfs, FTAG);
@@ -1139,11 +1137,10 @@ zfs_get_data(void *arg, uint64_t gen, lr_write_t *lr, char *buf,
 		for (;;) {
 			uint64_t blkoff;
 			size = zp->z_blksz;
-			blkoff = ISP2(size) ? P2PHASE(offset, size) :
-			    offset;
+			blkoff = ISP2(size) ? P2PHASE(offset, size) : offset;
 			offset -= blkoff;
-			zgd->zgd_lr = zfs_rangelock_enter(
-			    &zp->z_rangelock, offset, size, RL_READER);
+			zgd->zgd_lr = zfs_rangelock_enter(&zp->z_rangelock,
+			    offset, size, RL_READER);
 			if (zp->z_blksz == size)
 				break;
 			offset += blkoff;

module/zfs/zio.c

@@ -3591,6 +3591,13 @@ zio_ddt_write(zio_t *zio)
 	ASSERT(BP_GET_CHECKSUM(bp) == zp->zp_checksum);
 	ASSERT(BP_IS_HOLE(bp) || zio->io_bp_override);
 	ASSERT(!(zio->io_bp_override && (zio->io_flags & ZIO_FLAG_RAW)));
+	/*
+	 * Deduplication will not take place for Direct I/O writes. The
+	 * ddt_tree will be emptied in syncing context. Direct I/O writes take
+	 * place in the open-context. Direct I/O write can not attempt to
+	 * modify the ddt_tree while issuing out a write.
+	 */
+	ASSERT3B(zio->io_prop.zp_direct_write, ==, B_FALSE);
 
 	ddt_enter(ddt);
 	dde = ddt_lookup(ddt, bp);

tests/zfs-tests/cmd/manipulate_user_buffer.c

@@ -243,7 +243,7 @@ main(int argc, char *argv[])
 	}
 
 	/*
-	 * Writing using O_DIRECT while manipulating the buffer conntents until
+	 * Writing using O_DIRECT while manipulating the buffer contents until
 	 * the entire file is written.
 	 */
 	if ((rc = pthread_create(&manipul_thr, NULL, manipulate_buf_thread,

tests/zfs-tests/tests/functional/direct/dio_dedup.ksh

@@ -31,7 +31,8 @@
 
 #
 # DESCRIPTION:
-# 	Verify deduplication works using Direct I/O.
+# 	Verify deduplication works. Deduplication is disabled when issuing
+# 	Direct I/O writes.
 #
 # STRATEGY:
 #	1. Enable dedup