From ba76bb30a697d84c18de47d1ffd578b04b51df56 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Fri, 16 Jul 2021 15:39:24 -0400 Subject: [PATCH] Introduce dsl_dir_diduse_transfer_space() Most of dsl_dir_diduse_space() and dsl_dir_transfer_space() CPU time is a dd_lock overhead and time spent in dmu_buf_will_dirty(). Calling them one after another is a waste of time and even more contention. Doing that twice for each rewritten block within dbuf_write_done() via dsl_dataset_block_kill() and dsl_dataset_block_born() created one of the biggest CPU overheads in case of small blocks rewrite. dsl_dir_diduse_transfer_space() combines functionality of these two functions for cases where it is needed, but without double overhead, practically for the cost of dsl_dir_diduse_space() or even cheaper. While there, optimize dsl_dir_phys() calls in dsl_dir_diduse_space() and dsl_dir_transfer_space(). It seems Clang detects some aliasing there, repeating dd->dd_dbuf->db_data dereference multiple times, increasing dd_lock scope and contention. Reviewed-by: Brian Behlendorf Reviewed-by: Matthew Ahrens Author: Ryan Moeller Signed-off-by: Alexander Motin Closes #12300 --- include/sys/dsl_dir.h | 3 ++ module/zfs/dsl_dataset.c | 10 ++-- module/zfs/dsl_dir.c | 110 +++++++++++++++++++++++++++------------ 3 files changed, 85 insertions(+), 38 deletions(-) diff --git a/include/sys/dsl_dir.h b/include/sys/dsl_dir.h index 7cf5093c2c..d635b31404 100644 --- a/include/sys/dsl_dir.h +++ b/include/sys/dsl_dir.h @@ -174,6 +174,9 @@ void dsl_dir_diduse_space(dsl_dir_t *dd, dd_used_t type, int64_t used, int64_t compressed, int64_t uncompressed, dmu_tx_t *tx); void dsl_dir_transfer_space(dsl_dir_t *dd, int64_t delta, dd_used_t oldtype, dd_used_t newtype, dmu_tx_t *tx); +void dsl_dir_diduse_transfer_space(dsl_dir_t *dd, int64_t used, + int64_t compressed, int64_t uncompressed, int64_t tonew, + dd_used_t oldtype, dd_used_t newtype, dmu_tx_t *tx); int dsl_dir_set_quota(const char *ddname, zprop_source_t source, uint64_t quota); int dsl_dir_set_reservation(const char *ddname, zprop_source_t source, diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index 1c03216ef6..f99964511a 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -192,9 +192,8 @@ dsl_dataset_block_born(dsl_dataset_t *ds, const blkptr_t *bp, dmu_tx_t *tx) } mutex_exit(&ds->ds_lock); - dsl_dir_diduse_space(ds->ds_dir, DD_USED_HEAD, delta, - compressed, uncompressed, tx); - dsl_dir_transfer_space(ds->ds_dir, used - delta, + dsl_dir_diduse_transfer_space(ds->ds_dir, delta, + compressed, uncompressed, used, DD_USED_REFRSRV, DD_USED_HEAD, tx); } @@ -291,9 +290,8 @@ dsl_dataset_block_kill(dsl_dataset_t *ds, const blkptr_t *bp, dmu_tx_t *tx, delta = parent_delta(ds, -used); dsl_dataset_phys(ds)->ds_unique_bytes -= used; mutex_exit(&ds->ds_lock); - dsl_dir_diduse_space(ds->ds_dir, DD_USED_HEAD, - delta, -compressed, -uncompressed, tx); - dsl_dir_transfer_space(ds->ds_dir, -used - delta, + dsl_dir_diduse_transfer_space(ds->ds_dir, + delta, -compressed, -uncompressed, -used, DD_USED_REFRSRV, DD_USED_HEAD, tx); } else { dprintf_bp(bp, "putting on dead list: %s", ""); diff --git a/module/zfs/dsl_dir.c b/module/zfs/dsl_dir.c index df2c3d8f06..84caace4db 100644 --- a/module/zfs/dsl_dir.c +++ b/module/zfs/dsl_dir.c @@ -1517,6 +1517,11 @@ dsl_dir_diduse_space(dsl_dir_t *dd, dd_used_t type, { int64_t accounted_delta; + ASSERT(dmu_tx_is_syncing(tx)); + ASSERT(type < DD_USED_NUM); + + dmu_buf_will_dirty(dd->dd_dbuf, tx); + /* * dsl_dataset_set_refreservation_sync_impl() calls this with * dd_lock held, so that it can atomically update @@ -1525,36 +1530,28 @@ dsl_dir_diduse_space(dsl_dir_t *dd, dd_used_t type, * consistently. */ boolean_t needlock = !MUTEX_HELD(&dd->dd_lock); - - ASSERT(dmu_tx_is_syncing(tx)); - ASSERT(type < DD_USED_NUM); - - dmu_buf_will_dirty(dd->dd_dbuf, tx); - if (needlock) mutex_enter(&dd->dd_lock); - accounted_delta = - parent_delta(dd, dsl_dir_phys(dd)->dd_used_bytes, used); - ASSERT(used >= 0 || dsl_dir_phys(dd)->dd_used_bytes >= -used); - ASSERT(compressed >= 0 || - dsl_dir_phys(dd)->dd_compressed_bytes >= -compressed); + dsl_dir_phys_t *ddp = dsl_dir_phys(dd); + accounted_delta = parent_delta(dd, ddp->dd_used_bytes, used); + ASSERT(used >= 0 || ddp->dd_used_bytes >= -used); + ASSERT(compressed >= 0 || ddp->dd_compressed_bytes >= -compressed); ASSERT(uncompressed >= 0 || - dsl_dir_phys(dd)->dd_uncompressed_bytes >= -uncompressed); - dsl_dir_phys(dd)->dd_used_bytes += used; - dsl_dir_phys(dd)->dd_uncompressed_bytes += uncompressed; - dsl_dir_phys(dd)->dd_compressed_bytes += compressed; + ddp->dd_uncompressed_bytes >= -uncompressed); + ddp->dd_used_bytes += used; + ddp->dd_uncompressed_bytes += uncompressed; + ddp->dd_compressed_bytes += compressed; - if (dsl_dir_phys(dd)->dd_flags & DD_FLAG_USED_BREAKDOWN) { - ASSERT(used > 0 || - dsl_dir_phys(dd)->dd_used_breakdown[type] >= -used); - dsl_dir_phys(dd)->dd_used_breakdown[type] += used; + if (ddp->dd_flags & DD_FLAG_USED_BREAKDOWN) { + ASSERT(used >= 0 || ddp->dd_used_breakdown[type] >= -used); + ddp->dd_used_breakdown[type] += used; #ifdef ZFS_DEBUG { dd_used_t t; uint64_t u = 0; for (t = 0; t < DD_USED_NUM; t++) - u += dsl_dir_phys(dd)->dd_used_breakdown[t]; - ASSERT3U(u, ==, dsl_dir_phys(dd)->dd_used_bytes); + u += ddp->dd_used_breakdown[t]; + ASSERT3U(u, ==, ddp->dd_used_bytes); } #endif } @@ -1562,11 +1559,9 @@ dsl_dir_diduse_space(dsl_dir_t *dd, dd_used_t type, mutex_exit(&dd->dd_lock); if (dd->dd_parent != NULL) { - dsl_dir_diduse_space(dd->dd_parent, DD_USED_CHILD, - accounted_delta, compressed, uncompressed, tx); - dsl_dir_transfer_space(dd->dd_parent, - used - accounted_delta, - DD_USED_CHILD_RSRV, DD_USED_CHILD, tx); + dsl_dir_diduse_transfer_space(dd->dd_parent, + accounted_delta, compressed, uncompressed, + used, DD_USED_CHILD_RSRV, DD_USED_CHILD, tx); } } @@ -1578,21 +1573,72 @@ dsl_dir_transfer_space(dsl_dir_t *dd, int64_t delta, ASSERT(oldtype < DD_USED_NUM); ASSERT(newtype < DD_USED_NUM); + dsl_dir_phys_t *ddp = dsl_dir_phys(dd); if (delta == 0 || - !(dsl_dir_phys(dd)->dd_flags & DD_FLAG_USED_BREAKDOWN)) + !(ddp->dd_flags & DD_FLAG_USED_BREAKDOWN)) return; dmu_buf_will_dirty(dd->dd_dbuf, tx); mutex_enter(&dd->dd_lock); ASSERT(delta > 0 ? - dsl_dir_phys(dd)->dd_used_breakdown[oldtype] >= delta : - dsl_dir_phys(dd)->dd_used_breakdown[newtype] >= -delta); - ASSERT(dsl_dir_phys(dd)->dd_used_bytes >= ABS(delta)); - dsl_dir_phys(dd)->dd_used_breakdown[oldtype] -= delta; - dsl_dir_phys(dd)->dd_used_breakdown[newtype] += delta; + ddp->dd_used_breakdown[oldtype] >= delta : + ddp->dd_used_breakdown[newtype] >= -delta); + ASSERT(ddp->dd_used_bytes >= ABS(delta)); + ddp->dd_used_breakdown[oldtype] -= delta; + ddp->dd_used_breakdown[newtype] += delta; mutex_exit(&dd->dd_lock); } +void +dsl_dir_diduse_transfer_space(dsl_dir_t *dd, int64_t used, + int64_t compressed, int64_t uncompressed, int64_t tonew, + dd_used_t oldtype, dd_used_t newtype, dmu_tx_t *tx) +{ + int64_t accounted_delta; + + ASSERT(dmu_tx_is_syncing(tx)); + ASSERT(oldtype < DD_USED_NUM); + ASSERT(newtype < DD_USED_NUM); + + dmu_buf_will_dirty(dd->dd_dbuf, tx); + + mutex_enter(&dd->dd_lock); + dsl_dir_phys_t *ddp = dsl_dir_phys(dd); + accounted_delta = parent_delta(dd, ddp->dd_used_bytes, used); + ASSERT(used >= 0 || ddp->dd_used_bytes >= -used); + ASSERT(compressed >= 0 || ddp->dd_compressed_bytes >= -compressed); + ASSERT(uncompressed >= 0 || + ddp->dd_uncompressed_bytes >= -uncompressed); + ddp->dd_used_bytes += used; + ddp->dd_uncompressed_bytes += uncompressed; + ddp->dd_compressed_bytes += compressed; + + if (ddp->dd_flags & DD_FLAG_USED_BREAKDOWN) { + ASSERT(tonew - used <= 0 || + ddp->dd_used_breakdown[oldtype] >= tonew - used); + ASSERT(tonew >= 0 || + ddp->dd_used_breakdown[newtype] >= -tonew); + ddp->dd_used_breakdown[oldtype] -= tonew - used; + ddp->dd_used_breakdown[newtype] += tonew; +#ifdef ZFS_DEBUG + { + dd_used_t t; + uint64_t u = 0; + for (t = 0; t < DD_USED_NUM; t++) + u += ddp->dd_used_breakdown[t]; + ASSERT3U(u, ==, ddp->dd_used_bytes); + } +#endif + } + mutex_exit(&dd->dd_lock); + + if (dd->dd_parent != NULL) { + dsl_dir_diduse_transfer_space(dd->dd_parent, + accounted_delta, compressed, uncompressed, + used, DD_USED_CHILD_RSRV, DD_USED_CHILD, tx); + } +} + typedef struct dsl_dir_set_qr_arg { const char *ddsqra_name; zprop_source_t ddsqra_source;