From 900d09b285107e13fa0bdb8378274dff89347181 Mon Sep 17 00:00:00 2001 From: Prakash Surya Date: Tue, 23 Oct 2018 14:14:27 -0700 Subject: OpenZFS 9962 - zil_commit should omit cache thrash As a result of the changes made in 8585, it's possible for an excessive amount of vdev flush commands to be issued under some workloads. Specifically, when the workload consists of mostly async write activity, interspersed with some sync write and/or fsync activity, we can end up issuing more flush commands to the underlying storage than is actually necessary. As a result of these flush commands, the write latency and overall throughput of the pool can be poorly impacted (latency increases, throughput decreases). Currently, any time an lwb completes, the vdev(s) written to as a result of that lwb will be issued a flush command. The intenion is so the data written to that vdev is on stable storage, prior to communicating to any waiting threads that their data is safe on disk. The problem with this scheme, is that sometimes an lwb will not have any threads waiting for it to complete. This can occur when there's async activity that gets "converted" to sync requests, as a result of calling the zil_async_to_sync() function via zil_commit_impl(). When this occurs, the current code may issue many lwbs that don't have waiters associated with them, resulting in many flush commands, potentially to the same vdev(s). For example, given a pool with a single vdev, and a single fsync() call that results in 10 lwbs being written out (e.g. due to other async writes), that will result in 10 flush commands to that single vdev (a flush issued after each lwb write completes). Ideally, we'd only issue a single flush command to that vdev, after all 10 lwb writes completed. Further, and most important as it pertains to this change, since the flush commands are often very impactful to the performance of the pool's underlying storage, unnecessarily issuing these flush commands can poorly impact the performance of the lwb writes themselves. Thus, we need to avoid issuing flush commands when possible, in order to acheive the best possible performance out of the pool's underlying storage. This change attempts to address this problem by changing the ZIL's logic to only issue a vdev flush command when it detects an lwb that has a thread waiting for it to complete. When an lwb does not have threads waiting for it, the responsibility of issuing the flush command to the vdevs involved with that lwb's write is passed on to the "next" lwb. It's only once a write for an lwb with waiters completes, do we issue the vdev flush command(s). As a result, now when we issue the flush(s), we will issue them to the vdevs involved with that specific lwb's write, but potentially also to vdevs involved with "previous" lwb writes (i.e. if the previous lwbs did not have waiters associated with them). Thus, in our prior example with 10 lwbs, it's only once the last lwb completes (which will be the lwb containing the waiter for the thread that called fsync) will we issue the vdev flush command; all of the other lwbs will find they have no waiters, so they'll pass the responsibility of the flush to the "next" lwb (until reaching the last lwb that has the waiter). Porting Notes: * Reconciled conflicts with the fastwrite feature. Authored by: Prakash Surya Reviewed by: Matt Ahrens Reviewed by: Brad Lewis Reviewed by: Patrick Mooney Reviewed by: Jerry Jelinek Approved by: Joshua M. Clulow Ported-by: Signed-off-by: Brian Behlendorf OpenZFS-issue: https://www.illumos.org/issues/9962 OpenZFS-commit: https://github.com/openzfs/openzfs/commit/545190c6 Closes #8188 --- module/zfs/zvol.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'module/zfs/zvol.c') diff --git a/module/zfs/zvol.c b/module/zfs/zvol.c index e6f8451b2..7395dcb8d 100644 --- a/module/zfs/zvol.c +++ b/module/zfs/zvol.c @@ -1030,6 +1030,7 @@ out: #endif } +/* ARGSUSED */ static void zvol_get_done(zgd_t *zgd, int error) { @@ -1038,9 +1039,6 @@ zvol_get_done(zgd_t *zgd, int error) rangelock_exit(zgd->zgd_lr); - if (error == 0 && zgd->zgd_bp) - zil_lwb_add_block(zgd->zgd_lwb, zgd->zgd_bp); - kmem_free(zgd, sizeof (zgd_t)); } -- cgit v1.2.3