Skip to content

Commit 2696dfa

Browse files
grwilsonbehlendorf
authored andcommitted
Illumos #3642, #3643
3642 dsl_scan_active() should not issue I/O to determine if async destroying is active 3643 txg_delay should not hold the tc_lock Reviewed by: Matthew Ahrens <mahrens@delphix.com> Reviewed by: Adam Leventhal <ahl@delphix.com> Approved by: Gordon Ross <gwr@nexenta.com> References: https://www.illumos.org/issues/3642 https://www.illumos.org/issues/3643 illumos/illumos-gate@4a92375 Ported-by: Richard Yao <ryao@gentoo.org> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue #1775 Porting Notes: 1. The alignment assumptions for the tx_cpu structure assume that a kmutex_t is 8 bytes. This isn't true under Linux but tc_pad[] was adjusted anyway for consistency since this structure was never carefully aligned in ZoL. If careful alignment does impact performance significantly this should be reworked to be portable.
1 parent 7ec0928 commit 2696dfa

File tree

5 files changed

+78
-14
lines changed

5 files changed

+78
-14
lines changed

include/sys/dsl_scan.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
*/
2121
/*
2222
* Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
23-
* Copyright (c) 2012 by Delphix. All rights reserved.
23+
* Copyright (c) 2013 by Delphix. All rights reserved.
2424
*/
2525

2626
#ifndef _SYS_DSL_SCAN_H
@@ -82,6 +82,7 @@ typedef struct dsl_scan {
8282

8383
/* for freeing blocks */
8484
boolean_t scn_is_bptree;
85+
boolean_t scn_async_destroying;
8586

8687
/* for debugging / information */
8788
uint64_t scn_visited_this_txg;

include/sys/txg_impl.h

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@
2323
* Use is subject to license terms.
2424
*/
2525

26+
/*
27+
* Copyright (c) 2013 by Delphix. All rights reserved.
28+
*/
29+
2630
#ifndef _SYS_TXG_IMPL_H
2731
#define _SYS_TXG_IMPL_H
2832

@@ -33,14 +37,55 @@
3337
extern "C" {
3438
#endif
3539

40+
/*
41+
* The tx_cpu structure is a per-cpu structure that is used to track
42+
* the number of active transaction holds (tc_count). As transactions
43+
* are assigned into a transaction group the appropriate tc_count is
44+
* incremented to indicate that there are pending changes that have yet
45+
* to quiesce. Consumers evenutally call txg_rele_to_sync() to decrement
46+
* the tc_count. A transaction group is not considered quiesced until all
47+
* tx_cpu structures have reached a tc_count of zero.
48+
*
49+
* This structure is a per-cpu structure by design. Updates to this structure
50+
* are frequent and concurrent. Having a single structure would result in
51+
* heavy lock contention so a per-cpu design was implemented. With the fanned
52+
* out mutex design, consumers only need to lock the mutex associated with
53+
* thread's cpu.
54+
*
55+
* The tx_cpu contains two locks, the tc_lock and tc_open_lock.
56+
* The tc_lock is used to protect all members of the tx_cpu structure with
57+
* the exception of the tc_open_lock. This lock should only be held for a
58+
* short period of time, typically when updating the value of tc_count.
59+
*
60+
* The tc_open_lock protects the tx_open_txg member of the tx_state structure.
61+
* This lock is used to ensure that transactions are only assigned into
62+
* the current open transaction group. In order to move the current open
63+
* transaction group to the quiesce phase, the txg_quiesce thread must
64+
* grab all tc_open_locks, increment the tx_open_txg, and drop the locks.
65+
* The tc_open_lock is held until the transaction is assigned into the
66+
* transaction group. Typically, this is a short operation but if throttling
67+
* is occuring it may be held for longer periods of time.
68+
*/
3669
struct tx_cpu {
37-
kmutex_t tc_lock;
70+
kmutex_t tc_open_lock; /* protects tx_open_txg */
71+
kmutex_t tc_lock; /* protects the rest of this struct */
3872
kcondvar_t tc_cv[TXG_SIZE];
3973
uint64_t tc_count[TXG_SIZE];
4074
list_t tc_callbacks[TXG_SIZE]; /* commit cb list */
41-
char tc_pad[16];
75+
char tc_pad[8]; /* pad to fill 3 cache lines */
4276
};
4377

78+
/*
79+
* The tx_state structure maintains the state information about the different
80+
* stages of the pool's transcation groups. A per pool tx_state structure
81+
* is used to track this information. The tx_state structure also points to
82+
* an array of tx_cpu structures (described above). Although the tx_sync_lock
83+
* is used to protect the members of this structure, it is not used to
84+
* protect the tx_open_txg. Instead a special lock in the tx_cpu structure
85+
* is used. Readers of tx_open_txg must grab the per-cpu tc_open_lock.
86+
* Any thread wishing to update tx_open_txg must grab the tc_open_lock on
87+
* every cpu (see txg_quiesce()).
88+
*/
4489
typedef struct tx_state {
4590
tx_cpu_t *tx_cpu; /* protects right to enter txg */
4691
kmutex_t tx_sync_lock; /* protects tx_state_t */

module/zfs/dsl_destroy.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -761,12 +761,16 @@ dsl_destroy_head_sync_impl(dsl_dataset_t *ds, dmu_tx_t *tx)
761761
zil_destroy_sync(dmu_objset_zil(os), tx);
762762

763763
if (!spa_feature_is_active(dp->dp_spa, async_destroy)) {
764+
dsl_scan_t *scn = dp->dp_scan;
765+
764766
spa_feature_incr(dp->dp_spa, async_destroy, tx);
765767
dp->dp_bptree_obj = bptree_alloc(mos, tx);
766768
VERIFY0(zap_add(mos,
767769
DMU_POOL_DIRECTORY_OBJECT,
768770
DMU_POOL_BPTREE_OBJ, sizeof (uint64_t), 1,
769771
&dp->dp_bptree_obj, tx));
772+
ASSERT(!scn->scn_async_destroying);
773+
scn->scn_async_destroying = B_TRUE;
770774
}
771775

772776
used = ds->ds_dir->dd_phys->dd_used_bytes;

module/zfs/dsl_scan.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,15 @@ dsl_scan_init(dsl_pool_t *dp, uint64_t txg)
9191
scn = dp->dp_scan = kmem_zalloc(sizeof (dsl_scan_t), KM_SLEEP);
9292
scn->scn_dp = dp;
9393

94+
/*
95+
* It's possible that we're resuming a scan after a reboot so
96+
* make sure that the scan_async_destroying flag is initialized
97+
* appropriately.
98+
*/
99+
ASSERT(!scn->scn_async_destroying);
100+
scn->scn_async_destroying = spa_feature_is_active(dp->dp_spa,
101+
&spa_feature_table[SPA_FEATURE_ASYNC_DESTROY]);
102+
94103
err = zap_lookup(dp->dp_meta_objset, DMU_POOL_DIRECTORY_OBJECT,
95104
"scrub_func", sizeof (uint64_t), 1, &f);
96105
if (err == 0) {
@@ -1362,13 +1371,10 @@ dsl_scan_active(dsl_scan_t *scn)
13621371
if (spa_shutting_down(spa))
13631372
return (B_FALSE);
13641373

1365-
if (scn->scn_phys.scn_state == DSS_SCANNING)
1374+
if (scn->scn_phys.scn_state == DSS_SCANNING ||
1375+
scn->scn_async_destroying)
13661376
return (B_TRUE);
13671377

1368-
if (spa_feature_is_active(spa,
1369-
&spa_feature_table[SPA_FEATURE_ASYNC_DESTROY])) {
1370-
return (B_TRUE);
1371-
}
13721378
if (spa_version(scn->scn_dp->dp_spa) >= SPA_VERSION_DEADLISTS) {
13731379
(void) bpobj_space(&scn->scn_dp->dp_free_bpobj,
13741380
&used, &comp, &uncomp);
@@ -1424,6 +1430,7 @@ dsl_scan_sync(dsl_pool_t *dp, dmu_tx_t *tx)
14241430

14251431
if (err == 0 && spa_feature_is_active(spa,
14261432
&spa_feature_table[SPA_FEATURE_ASYNC_DESTROY])) {
1433+
ASSERT(scn->scn_async_destroying);
14271434
scn->scn_is_bptree = B_TRUE;
14281435
scn->scn_zio_root = zio_root(dp->dp_spa, NULL,
14291436
NULL, ZIO_FLAG_MUSTSUCCEED);
@@ -1444,6 +1451,7 @@ dsl_scan_sync(dsl_pool_t *dp, dmu_tx_t *tx)
14441451
VERIFY0(bptree_free(dp->dp_meta_objset,
14451452
dp->dp_bptree_obj, tx));
14461453
dp->dp_bptree_obj = 0;
1454+
scn->scn_async_destroying = B_FALSE;
14471455
}
14481456
}
14491457
if (scn->scn_visited_this_txg) {

module/zfs/txg.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ txg_init(dsl_pool_t *dp, uint64_t txg)
127127
int i;
128128

129129
mutex_init(&tx->tx_cpu[c].tc_lock, NULL, MUTEX_DEFAULT, NULL);
130+
mutex_init(&tx->tx_cpu[c].tc_open_lock, NULL, MUTEX_DEFAULT,
131+
NULL);
130132
for (i = 0; i < TXG_SIZE; i++) {
131133
cv_init(&tx->tx_cpu[c].tc_cv[i], NULL, CV_DEFAULT,
132134
NULL);
@@ -169,6 +171,7 @@ txg_fini(dsl_pool_t *dp)
169171
for (c = 0; c < max_ncpus; c++) {
170172
int i;
171173

174+
mutex_destroy(&tx->tx_cpu[c].tc_open_lock);
172175
mutex_destroy(&tx->tx_cpu[c].tc_lock);
173176
for (i = 0; i < TXG_SIZE; i++) {
174177
cv_destroy(&tx->tx_cpu[c].tc_cv[i]);
@@ -303,10 +306,12 @@ txg_hold_open(dsl_pool_t *dp, txg_handle_t *th)
303306
tc = &tx->tx_cpu[CPU_SEQID];
304307
kpreempt_enable();
305308

306-
mutex_enter(&tc->tc_lock);
307-
309+
mutex_enter(&tc->tc_open_lock);
308310
txg = tx->tx_open_txg;
311+
312+
mutex_enter(&tc->tc_lock);
309313
tc->tc_count[txg & TXG_MASK]++;
314+
mutex_exit(&tc->tc_lock);
310315

311316
th->th_cpu = tc;
312317
th->th_txg = txg;
@@ -319,7 +324,8 @@ txg_rele_to_quiesce(txg_handle_t *th)
319324
{
320325
tx_cpu_t *tc = th->th_cpu;
321326

322-
mutex_exit(&tc->tc_lock);
327+
ASSERT(!MUTEX_HELD(&tc->tc_lock));
328+
mutex_exit(&tc->tc_open_lock);
323329
}
324330

325331
void
@@ -356,10 +362,10 @@ txg_quiesce(dsl_pool_t *dp, uint64_t txg)
356362
int c;
357363

358364
/*
359-
* Grab all tx_cpu locks so nobody else can get into this txg.
365+
* Grab all tc_open_locks so nobody else can get into this txg.
360366
*/
361367
for (c = 0; c < max_ncpus; c++)
362-
mutex_enter(&tx->tx_cpu[c].tc_lock);
368+
mutex_enter(&tx->tx_cpu[c].tc_open_lock);
363369

364370
ASSERT(txg == tx->tx_open_txg);
365371
tx->tx_open_txg++;
@@ -372,7 +378,7 @@ txg_quiesce(dsl_pool_t *dp, uint64_t txg)
372378
* enter the next transaction group.
373379
*/
374380
for (c = 0; c < max_ncpus; c++)
375-
mutex_exit(&tx->tx_cpu[c].tc_lock);
381+
mutex_exit(&tx->tx_cpu[c].tc_open_lock);
376382

377383
/*
378384
* Quiesce the transaction group by waiting for everyone to txg_exit().

0 commit comments

Comments
 (0)