FreeBSD: zfs_putpages: don't undirty pages until after write completes

zfs_putpages() would put the entire range of pages onto the ZIL, then
return VM_PAGER_OK for each page to the kernel. However, an associated
zil_commit() or txg sync had not happened at this point, so the write
may not actually be on disk.

So, we rework it to use a ZIL commit callback, and do the post-write
work of undirtying the page and signaling completion there. We return
VM_PAGER_PEND to the kernel instead so it knows that we will take care
of it.

We change from a single zfs_log_write() to cover the whole range, to
multiple calls, each for a single page. This is because if
zfs_log_write() receives a write too large to fit into a single ZIL
block, it will split it into multiple writes, but each will receive the
same callback and argument. This will lead to the callback being called
multiple times, but we don't which pages/range the call is for, and so
can't clean them up. This way is not ideal, but is simple and correct,
and so is a good start.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <[email protected]>

Author: robn

include/os/freebsd/spl/sys/vm.h

@@ -35,6 +35,8 @@
 extern const int zfs_vm_pagerret_bad;
 extern const int zfs_vm_pagerret_error;
 extern const int zfs_vm_pagerret_ok;
+extern const int zfs_vm_pagerret_pend;
+extern const int zfs_vm_pagerret_again;
 extern const int zfs_vm_pagerput_sync;
 extern const int zfs_vm_pagerput_inval;
 

module/os/freebsd/spl/spl_vm.c

@@ -43,6 +43,8 @@
 const int zfs_vm_pagerret_bad = VM_PAGER_BAD;
 const int zfs_vm_pagerret_error = VM_PAGER_ERROR;
 const int zfs_vm_pagerret_ok = VM_PAGER_OK;
+const int zfs_vm_pagerret_pend = VM_PAGER_PEND;
+const int zfs_vm_pagerret_again = VM_PAGER_AGAIN;
 const int zfs_vm_pagerput_sync = VM_PAGER_PUT_SYNC;
 const int zfs_vm_pagerput_inval = VM_PAGER_PUT_INVAL;
 

module/os/freebsd/zfs/zfs_vnops_os.c

@@ -4084,6 +4084,15 @@ zfs_freebsd_getpages(struct vop_getpages_args *ap)
 	    ap->a_rahead));
 }
 
+static void
+zfs_putpage_commit_cb(void *arg)
+{
+	vm_page_t pp = arg;
+	vm_page_undirty(pp);
+	vm_page_sunbusy(pp);
+	vm_object_pip_wakeup(pp->object);
+}
+
 static int
 zfs_putpages(struct vnode *vp, vm_page_t *ma, size_t len, int flags,
     int *rtvals)
@@ -4185,10 +4194,12 @@ zfs_putpages(struct vnode *vp, vm_page_t *ma, size_t len, int flags,
 	}
 
 	if (zp->z_blksz < PAGE_SIZE) {
-		for (i = 0; len > 0; off += tocopy, len -= tocopy, i++) {
-			tocopy = len > PAGE_SIZE ? PAGE_SIZE : len;
+		vm_ooffset_t woff = off;
+		size_t wlen = len;
+		for (i = 0; wlen > 0; woff += tocopy, wlen -= tocopy, i++) {
+			tocopy = MIN(PAGE_SIZE, wlen);
 			va = zfs_map_page(ma[i], &sf);
-			dmu_write(zfsvfs->z_os, zp->z_id, off, tocopy, va, tx);
+			dmu_write(zfsvfs->z_os, zp->z_id, woff, tocopy, va, tx);
 			zfs_unmap_page(sf);
 		}
 	} else {
@@ -4209,19 +4220,47 @@ zfs_putpages(struct vnode *vp, vm_page_t *ma, size_t len, int flags,
 		zfs_tstamp_update_setup(zp, CONTENT_MODIFIED, mtime, ctime);
 		err = sa_bulk_update(zp->z_sa_hdl, bulk, count, tx);
 		ASSERT0(err);
+
 		/*
-		 * XXX we should be passing a callback to undirty
-		 * but that would make the locking messier
+		 * Loop over the pages and load them onto the ZIL. Each page
+		 * gets a separate log write so we can get the correct page
+		 * pointer into the callback.
 		 */
-		zfs_log_write(zfsvfs->z_log, tx, TX_WRITE, zp, off,
-		    len, commit, B_FALSE, NULL, NULL);
+		vm_ooffset_t pgoff = off;
+		size_t pgrem = len;
 
 		zfs_vmobject_wlock(object);
 		for (i = 0; i < ncount; i++) {
-			rtvals[i] = zfs_vm_pagerret_ok;
-			vm_page_undirty(ma[i]);
+			if (pgrem == 0) {
+				/*
+				 * This page is entirely beyond the requested
+				 * length, nothing more to do.
+				 */
+				rtvals[i] = zfs_vm_pagerret_again;
+				continue;
+			}
+
+			size_t pglen = MIN(PAGE_SIZE, pgrem);
+
+			zfs_log_write(zfsvfs->z_log, tx, TX_WRITE, zp,
+			    pgoff, pglen, commit, B_FALSE,
+			    zfs_putpage_commit_cb, ma[i]);
+
+			pgoff += pglen;
+			pgrem -= pglen;
+
+			/*
+			 * Inform the kernel that we will take care of page
+			 * cleanup and signalling once it's written out.
+			 */
+			rtvals[i] = zfs_vm_pagerret_pend;
 		}
+
+		ASSERT3U(pgoff, ==, off+len);
+		ASSERT0(pgrem);
+
 		zfs_vmobject_wunlock(object);
+
 		VM_CNT_INC(v_vnodeout);
 		VM_CNT_ADD(v_vnodepgsout, ncount);
 	}