Skip to content

Commit ba76bb3

Browse files
amotintonyhutter
authored andcommitted
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 <[email protected]> Reviewed-by: Matthew Ahrens <[email protected]> Author: Ryan Moeller <[email protected]> Signed-off-by: Alexander Motin <[email protected]> Closes openzfs#12300
1 parent 968dc13 commit ba76bb3

File tree

3 files changed

+86
-39
lines changed

3 files changed

+86
-39
lines changed

include/sys/dsl_dir.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,9 @@ void dsl_dir_diduse_space(dsl_dir_t *dd, dd_used_t type,
174174
int64_t used, int64_t compressed, int64_t uncompressed, dmu_tx_t *tx);
175175
void dsl_dir_transfer_space(dsl_dir_t *dd, int64_t delta,
176176
dd_used_t oldtype, dd_used_t newtype, dmu_tx_t *tx);
177+
void dsl_dir_diduse_transfer_space(dsl_dir_t *dd, int64_t used,
178+
int64_t compressed, int64_t uncompressed, int64_t tonew,
179+
dd_used_t oldtype, dd_used_t newtype, dmu_tx_t *tx);
177180
int dsl_dir_set_quota(const char *ddname, zprop_source_t source,
178181
uint64_t quota);
179182
int dsl_dir_set_reservation(const char *ddname, zprop_source_t source,

module/zfs/dsl_dataset.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,9 +192,8 @@ dsl_dataset_block_born(dsl_dataset_t *ds, const blkptr_t *bp, dmu_tx_t *tx)
192192
}
193193

194194
mutex_exit(&ds->ds_lock);
195-
dsl_dir_diduse_space(ds->ds_dir, DD_USED_HEAD, delta,
196-
compressed, uncompressed, tx);
197-
dsl_dir_transfer_space(ds->ds_dir, used - delta,
195+
dsl_dir_diduse_transfer_space(ds->ds_dir, delta,
196+
compressed, uncompressed, used,
198197
DD_USED_REFRSRV, DD_USED_HEAD, tx);
199198
}
200199

@@ -291,9 +290,8 @@ dsl_dataset_block_kill(dsl_dataset_t *ds, const blkptr_t *bp, dmu_tx_t *tx,
291290
delta = parent_delta(ds, -used);
292291
dsl_dataset_phys(ds)->ds_unique_bytes -= used;
293292
mutex_exit(&ds->ds_lock);
294-
dsl_dir_diduse_space(ds->ds_dir, DD_USED_HEAD,
295-
delta, -compressed, -uncompressed, tx);
296-
dsl_dir_transfer_space(ds->ds_dir, -used - delta,
293+
dsl_dir_diduse_transfer_space(ds->ds_dir,
294+
delta, -compressed, -uncompressed, -used,
297295
DD_USED_REFRSRV, DD_USED_HEAD, tx);
298296
} else {
299297
dprintf_bp(bp, "putting on dead list: %s", "");

module/zfs/dsl_dir.c

Lines changed: 79 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1517,6 +1517,11 @@ dsl_dir_diduse_space(dsl_dir_t *dd, dd_used_t type,
15171517
{
15181518
int64_t accounted_delta;
15191519

1520+
ASSERT(dmu_tx_is_syncing(tx));
1521+
ASSERT(type < DD_USED_NUM);
1522+
1523+
dmu_buf_will_dirty(dd->dd_dbuf, tx);
1524+
15201525
/*
15211526
* dsl_dataset_set_refreservation_sync_impl() calls this with
15221527
* dd_lock held, so that it can atomically update
@@ -1525,48 +1530,38 @@ dsl_dir_diduse_space(dsl_dir_t *dd, dd_used_t type,
15251530
* consistently.
15261531
*/
15271532
boolean_t needlock = !MUTEX_HELD(&dd->dd_lock);
1528-
1529-
ASSERT(dmu_tx_is_syncing(tx));
1530-
ASSERT(type < DD_USED_NUM);
1531-
1532-
dmu_buf_will_dirty(dd->dd_dbuf, tx);
1533-
15341533
if (needlock)
15351534
mutex_enter(&dd->dd_lock);
1536-
accounted_delta =
1537-
parent_delta(dd, dsl_dir_phys(dd)->dd_used_bytes, used);
1538-
ASSERT(used >= 0 || dsl_dir_phys(dd)->dd_used_bytes >= -used);
1539-
ASSERT(compressed >= 0 ||
1540-
dsl_dir_phys(dd)->dd_compressed_bytes >= -compressed);
1535+
dsl_dir_phys_t *ddp = dsl_dir_phys(dd);
1536+
accounted_delta = parent_delta(dd, ddp->dd_used_bytes, used);
1537+
ASSERT(used >= 0 || ddp->dd_used_bytes >= -used);
1538+
ASSERT(compressed >= 0 || ddp->dd_compressed_bytes >= -compressed);
15411539
ASSERT(uncompressed >= 0 ||
1542-
dsl_dir_phys(dd)->dd_uncompressed_bytes >= -uncompressed);
1543-
dsl_dir_phys(dd)->dd_used_bytes += used;
1544-
dsl_dir_phys(dd)->dd_uncompressed_bytes += uncompressed;
1545-
dsl_dir_phys(dd)->dd_compressed_bytes += compressed;
1546-
1547-
if (dsl_dir_phys(dd)->dd_flags & DD_FLAG_USED_BREAKDOWN) {
1548-
ASSERT(used > 0 ||
1549-
dsl_dir_phys(dd)->dd_used_breakdown[type] >= -used);
1550-
dsl_dir_phys(dd)->dd_used_breakdown[type] += used;
1540+
ddp->dd_uncompressed_bytes >= -uncompressed);
1541+
ddp->dd_used_bytes += used;
1542+
ddp->dd_uncompressed_bytes += uncompressed;
1543+
ddp->dd_compressed_bytes += compressed;
1544+
1545+
if (ddp->dd_flags & DD_FLAG_USED_BREAKDOWN) {
1546+
ASSERT(used >= 0 || ddp->dd_used_breakdown[type] >= -used);
1547+
ddp->dd_used_breakdown[type] += used;
15511548
#ifdef ZFS_DEBUG
15521549
{
15531550
dd_used_t t;
15541551
uint64_t u = 0;
15551552
for (t = 0; t < DD_USED_NUM; t++)
1556-
u += dsl_dir_phys(dd)->dd_used_breakdown[t];
1557-
ASSERT3U(u, ==, dsl_dir_phys(dd)->dd_used_bytes);
1553+
u += ddp->dd_used_breakdown[t];
1554+
ASSERT3U(u, ==, ddp->dd_used_bytes);
15581555
}
15591556
#endif
15601557
}
15611558
if (needlock)
15621559
mutex_exit(&dd->dd_lock);
15631560

15641561
if (dd->dd_parent != NULL) {
1565-
dsl_dir_diduse_space(dd->dd_parent, DD_USED_CHILD,
1566-
accounted_delta, compressed, uncompressed, tx);
1567-
dsl_dir_transfer_space(dd->dd_parent,
1568-
used - accounted_delta,
1569-
DD_USED_CHILD_RSRV, DD_USED_CHILD, tx);
1562+
dsl_dir_diduse_transfer_space(dd->dd_parent,
1563+
accounted_delta, compressed, uncompressed,
1564+
used, DD_USED_CHILD_RSRV, DD_USED_CHILD, tx);
15701565
}
15711566
}
15721567

@@ -1578,21 +1573,72 @@ dsl_dir_transfer_space(dsl_dir_t *dd, int64_t delta,
15781573
ASSERT(oldtype < DD_USED_NUM);
15791574
ASSERT(newtype < DD_USED_NUM);
15801575

1576+
dsl_dir_phys_t *ddp = dsl_dir_phys(dd);
15811577
if (delta == 0 ||
1582-
!(dsl_dir_phys(dd)->dd_flags & DD_FLAG_USED_BREAKDOWN))
1578+
!(ddp->dd_flags & DD_FLAG_USED_BREAKDOWN))
15831579
return;
15841580

15851581
dmu_buf_will_dirty(dd->dd_dbuf, tx);
15861582
mutex_enter(&dd->dd_lock);
15871583
ASSERT(delta > 0 ?
1588-
dsl_dir_phys(dd)->dd_used_breakdown[oldtype] >= delta :
1589-
dsl_dir_phys(dd)->dd_used_breakdown[newtype] >= -delta);
1590-
ASSERT(dsl_dir_phys(dd)->dd_used_bytes >= ABS(delta));
1591-
dsl_dir_phys(dd)->dd_used_breakdown[oldtype] -= delta;
1592-
dsl_dir_phys(dd)->dd_used_breakdown[newtype] += delta;
1584+
ddp->dd_used_breakdown[oldtype] >= delta :
1585+
ddp->dd_used_breakdown[newtype] >= -delta);
1586+
ASSERT(ddp->dd_used_bytes >= ABS(delta));
1587+
ddp->dd_used_breakdown[oldtype] -= delta;
1588+
ddp->dd_used_breakdown[newtype] += delta;
15931589
mutex_exit(&dd->dd_lock);
15941590
}
15951591

1592+
void
1593+
dsl_dir_diduse_transfer_space(dsl_dir_t *dd, int64_t used,
1594+
int64_t compressed, int64_t uncompressed, int64_t tonew,
1595+
dd_used_t oldtype, dd_used_t newtype, dmu_tx_t *tx)
1596+
{
1597+
int64_t accounted_delta;
1598+
1599+
ASSERT(dmu_tx_is_syncing(tx));
1600+
ASSERT(oldtype < DD_USED_NUM);
1601+
ASSERT(newtype < DD_USED_NUM);
1602+
1603+
dmu_buf_will_dirty(dd->dd_dbuf, tx);
1604+
1605+
mutex_enter(&dd->dd_lock);
1606+
dsl_dir_phys_t *ddp = dsl_dir_phys(dd);
1607+
accounted_delta = parent_delta(dd, ddp->dd_used_bytes, used);
1608+
ASSERT(used >= 0 || ddp->dd_used_bytes >= -used);
1609+
ASSERT(compressed >= 0 || ddp->dd_compressed_bytes >= -compressed);
1610+
ASSERT(uncompressed >= 0 ||
1611+
ddp->dd_uncompressed_bytes >= -uncompressed);
1612+
ddp->dd_used_bytes += used;
1613+
ddp->dd_uncompressed_bytes += uncompressed;
1614+
ddp->dd_compressed_bytes += compressed;
1615+
1616+
if (ddp->dd_flags & DD_FLAG_USED_BREAKDOWN) {
1617+
ASSERT(tonew - used <= 0 ||
1618+
ddp->dd_used_breakdown[oldtype] >= tonew - used);
1619+
ASSERT(tonew >= 0 ||
1620+
ddp->dd_used_breakdown[newtype] >= -tonew);
1621+
ddp->dd_used_breakdown[oldtype] -= tonew - used;
1622+
ddp->dd_used_breakdown[newtype] += tonew;
1623+
#ifdef ZFS_DEBUG
1624+
{
1625+
dd_used_t t;
1626+
uint64_t u = 0;
1627+
for (t = 0; t < DD_USED_NUM; t++)
1628+
u += ddp->dd_used_breakdown[t];
1629+
ASSERT3U(u, ==, ddp->dd_used_bytes);
1630+
}
1631+
#endif
1632+
}
1633+
mutex_exit(&dd->dd_lock);
1634+
1635+
if (dd->dd_parent != NULL) {
1636+
dsl_dir_diduse_transfer_space(dd->dd_parent,
1637+
accounted_delta, compressed, uncompressed,
1638+
used, DD_USED_CHILD_RSRV, DD_USED_CHILD, tx);
1639+
}
1640+
}
1641+
15961642
typedef struct dsl_dir_set_qr_arg {
15971643
const char *ddsqra_name;
15981644
zprop_source_t ddsqra_source;

0 commit comments

Comments
 (0)