Skip to content
Permalink
Browse files

review comments

  • Loading branch information...
shartse committed Apr 2, 2019
1 parent 78c785b commit fae035da72602de676fc1d2292b2db1f55894f59
@@ -33,9 +33,9 @@ extern void zthr_destroy(zthr_t *t);
extern void zthr_wakeup(zthr_t *t);
extern void zthr_cancel(zthr_t *t);
extern void zthr_resume(zthr_t *t);
extern void zthr_finish(zthr_t *t);
extern void zthr_wait_cycle_done(zthr_t *t);

extern boolean_t zthr_iscancelled(zthr_t *t);
extern boolean_t zthr_isfinishing(zthr_t *t);
extern boolean_t zthr_has_waiters(zthr_t *t);

#endif /* _SYS_ZTHR_H */
@@ -1737,6 +1737,33 @@ Pattern written to vdev free space by \fBzpool initialize\fR.
Default value: \fB16,045,690,984,833,335,022\fR (0xdeadbeefdeadbeee).
.RE

.sp
.ne 2
.na
\fBzfs_livelist_max_entries\fR (ulong)
.ad
.RS 12n
The threshold size (in block pointers) at which we create a new sub-livelist.
Larger sublists are more costly from a memory perspective but the fewer
sublists there are, the lower the cost of insertion.
.sp
Default value: \fB500,000\fR.
.RE

.sp
.ne 2
.na
\fBzfs_livelist_min_percent_shared\fR (int)
.ad
.RS 12n
If the amount of shared space between a snapshot and its clone drops below
this threshold, the clone turns off the livelist and reverts to the old deletion
method. This is in place because once a clone has been overwritten enough
livelists no long give us a benefit.
.sp
Default value: \fB75\fR.
.RE

.sp
.ne 2
.na
@@ -348,10 +348,17 @@ zpool_feature_init(void)
ZFEATURE_FLAG_MOS | ZFEATURE_FLAG_ACTIVATE_ON_ENABLE,
ZFEATURE_TYPE_BOOLEAN, NULL);

{
static const spa_feature_t livelist_deps[] = {
SPA_FEATURE_EXTENSIBLE_DATASET,
SPA_FEATURE_NONE
};
zfeature_register(SPA_FEATURE_LIVELIST,
"com.delphix:livelist", "livelist",
"Improved clone deletion performance.",
ZFEATURE_FLAG_READONLY_COMPAT, ZFEATURE_TYPE_BOOLEAN, NULL);
ZFEATURE_FLAG_READONLY_COMPAT, ZFEATURE_TYPE_BOOLEAN,
livelist_deps);
}

{
static const spa_feature_t large_blocks_deps[] = {
@@ -277,7 +277,6 @@ bpobj_iterate_blkptrs(bpobj_info_t *bpi, bpobj_itor_t func, void *arg,
for (int64_t i = bpo->bpo_phys->bpo_num_blkptrs - 1; i >= start; i--) {
uint64_t offset = i * sizeof (blkptr_t);
uint64_t blkoff = P2PHASE(i, bpo->bpo_epb);
boolean_t bp_freed;

if (dbuf == NULL || dbuf->db_offset > offset) {
if (dbuf)
@@ -294,7 +293,7 @@ bpobj_iterate_blkptrs(bpobj_info_t *bpi, bpobj_itor_t func, void *arg,
blkptr_t *bparray = dbuf->db_data;
blkptr_t *bp = &bparray[blkoff];

bp_freed = BP_GET_FREE(bp);
boolean_t bp_freed = BP_GET_FREE(bp);
err = func(arg, bp, bp_freed, tx);
if (err)
break;
@@ -822,7 +822,7 @@ dsl_livelist_iterate(void *arg, const blkptr_t *bp, boolean_t free,
zthr_t *t = lia->t;
ASSERT(tx == NULL);

if ((t != NULL) && (zthr_isfinishing(t) || zthr_iscancelled(t)))
if ((t != NULL) && (zthr_has_waiters(t) || zthr_iscancelled(t)))
return (EINTR);
if (free) {
livelist_entry_t *node = kmem_alloc(sizeof (livelist_entry_t),
@@ -2280,27 +2280,27 @@ dsl_dir_remove_livelist(dsl_dir_t *dd, dmu_tx_t *tx, boolean_t total)
if (ll_condense_thread != NULL &&
(to_condense.ds != NULL) && (to_condense.ds->ds_dir == dd)) {
/*
* We use zthr_finish instead of zthr_cancel because we
* don't want to destroy the zthr, just have it skip
* it's current task.
* We use zthr_wait_cycle_done instead of zthr_cancel
* because we don't want to destroy the zthr, just have
* it skip it's current task.
*/
spa->spa_to_condense.cancelled = B_TRUE;
zthr_finish(ll_condense_thread);
zthr_wait_cycle_done(ll_condense_thread);
/*
* If we've returned from zthr_finish without clearing
* the to_condense data structure it's either because
* the no-wait synctask has started (which is indicated
* by 'syncing' field of to_condense) and we can expect
* it to clear to_condense on its own. Otherwise, we
* returned before the zthr ran. The checkfunc will
* now fail as cancelled == B_TRUE so we can safely NULL
* out ds, allowing a different dir's livelist to be
* condensed.
* If we've returned from zthr_wait_cycle_done without
* clearing the to_condense data structure it's either
* because the no-wait synctask has started (which is
* indicated by 'syncing' field of to_condense) and we
* can expect it to clear to_condense on its own.
* Otherwise, we returned before the zthr ran. The
* checkfunc will now fail as cancelled == B_TRUE so we
* can safely NULL out ds, allowing a different dir's
* livelist to be condensed.
*
* We can be sure that the to_condense struct will not
* be repopulated during the execution of this function
* because dsl_livelist_try_condense excutes in syncing
* context.
* be repopulated at this stage because both this
* function and dsl_livelist_try_condense execute in
* syncing context.
*/
if ((spa->spa_to_condense.ds != NULL) &&
!spa->spa_to_condense.syncing) {
@@ -2570,7 +2570,7 @@ void
spa_livelist_condense_cb(void *arg, zthr_t *t)
{
while (zfs_livelist_condense_zthr_pause &&
!(zthr_isfinishing(t) || zthr_iscancelled(t)))
!(zthr_has_waiters(t) || zthr_iscancelled(t)))
delay(1);

int err;
@@ -2588,7 +2588,7 @@ spa_livelist_condense_cb(void *arg, zthr_t *t)

if (err == 0) {
while (zfs_livelist_condense_sync_pause &&
!(zthr_isfinishing(t) || zthr_iscancelled(t)))
!(zthr_has_waiters(t) || zthr_iscancelled(t)))
delay(1);

dmu_tx_t *tx = dmu_tx_create_dd(spa_get_dsl(spa)->dp_mos_dir);
@@ -208,14 +208,14 @@ struct zthr {
boolean_t zthr_cancel;

/* flag set to true if we are waiting for the zthr to finish */
boolean_t zthr_finish;
kcondvar_t zthr_finish_cv;
boolean_t zthr_haswaiters;
kcondvar_t zthr_wait_cv;
/*
* maximum amount of time that the zthr is spent sleeping;
* if this is 0, the thread doesn't wake up until it gets
* signaled.
*/
hrtime_t zthr_wait_time;
hrtime_t zthr_sleep_timeout;

/* consumer-provided callbacks & data */
zthr_checkfunc_t *zthr_checkfunc;
@@ -242,17 +242,17 @@ zthr_procedure(void *arg)
* order to prevent this process from incorrectly
* contributing to the system load average when idle.
*/
if (t->zthr_wait_time == 0) {
if (t->zthr_sleep_timeout == 0) {
cv_wait_sig(&t->zthr_cv, &t->zthr_state_lock);
} else {
(void) cv_timedwait_sig_hires(&t->zthr_cv,
&t->zthr_state_lock, t->zthr_wait_time,
&t->zthr_state_lock, t->zthr_sleep_timeout,
MSEC2NSEC(1), 0);
}
}
if (t->zthr_finish) {
t->zthr_finish = B_FALSE;
cv_broadcast(&t->zthr_finish_cv);
if (t->zthr_haswaiters) {
t->zthr_haswaiters = B_FALSE;
cv_broadcast(&t->zthr_wait_cv);
}
}

@@ -287,13 +287,13 @@ zthr_create_timer(zthr_checkfunc_t *checkfunc, zthr_func_t *func,
mutex_init(&t->zthr_state_lock, NULL, MUTEX_DEFAULT, NULL);
mutex_init(&t->zthr_request_lock, NULL, MUTEX_DEFAULT, NULL);
cv_init(&t->zthr_cv, NULL, CV_DEFAULT, NULL);
cv_init(&t->zthr_finish_cv, NULL, CV_DEFAULT, NULL);
cv_init(&t->zthr_wait_cv, NULL, CV_DEFAULT, NULL);

mutex_enter(&t->zthr_state_lock);
t->zthr_checkfunc = checkfunc;
t->zthr_func = func;
t->zthr_arg = arg;
t->zthr_wait_time = max_sleep;
t->zthr_sleep_timeout = max_sleep;

t->zthr_thread = thread_create(NULL, 0, zthr_procedure, t,
0, &p0, TS_RUN, minclsyspri);
@@ -311,7 +311,7 @@ zthr_destroy(zthr_t *t)
mutex_destroy(&t->zthr_request_lock);
mutex_destroy(&t->zthr_state_lock);
cv_destroy(&t->zthr_cv);
cv_destroy(&t->zthr_finish_cv);
cv_destroy(&t->zthr_wait_cv);
kmem_free(t, sizeof (*t));
}

@@ -405,7 +405,7 @@ zthr_resume(zthr_t *t)
ASSERT3P(&t->zthr_checkfunc, !=, NULL);
ASSERT3P(&t->zthr_func, !=, NULL);
ASSERT(!t->zthr_cancel);
ASSERT(!t->zthr_finish);
ASSERT(!t->zthr_haswaiters);

/*
* There are 4 states that we find the zthr in at this point
@@ -463,12 +463,12 @@ zthr_iscancelled(zthr_t *t)

/*
* Wait for the zthr to finish its current function. Similar to
* zthr_iscancelled, you can use zthr_isfinishing to have the zthr_func end
* zthr_iscancelled, you can use zthr_has_waiters to have the zthr_func end
* early. Unlike zthr_cancel, the thread is not destroyed. If the zthr was
* sleeping or cancelled, return immediately.
*/
void
zthr_finish(zthr_t *t)
zthr_wait_cycle_done(zthr_t *t)
{
mutex_enter(&t->zthr_state_lock);

@@ -492,17 +492,16 @@ zthr_finish(zthr_t *t)
* control back we expect that the zthr has completed it's
* zthr_func.
*/

if (t->zthr_thread != NULL) {
t->zthr_finish = B_TRUE;
t->zthr_haswaiters = B_TRUE;

/* broadcast in case the zthr is sleeping */
cv_broadcast(&t->zthr_cv);

while ((t->zthr_finish) && (t->zthr_thread != NULL))
cv_wait(&t->zthr_finish_cv, &t->zthr_state_lock);
while ((t->zthr_haswaiters) && (t->zthr_thread != NULL))
cv_wait(&t->zthr_wait_cv, &t->zthr_state_lock);

ASSERT(!t->zthr_finish);
ASSERT(!t->zthr_haswaiters);
}

mutex_exit(&t->zthr_state_lock);
@@ -517,7 +516,7 @@ zthr_finish(zthr_t *t)
* returns FALSE otherwise.
*/
boolean_t
zthr_isfinishing(zthr_t *t)
zthr_has_waiters(zthr_t *t)
{
ASSERT3P(t->zthr_thread, ==, curthread);

@@ -528,7 +527,7 @@ zthr_isfinishing(zthr_t *t)
* zthr_state_lock so that the zthr itself can use this
* to check for the request.
*/
boolean_t finish = t->zthr_finish;
boolean_t has_waiters = t->zthr_haswaiters;
mutex_exit(&t->zthr_state_lock);
return (finish);
return (has_waiters);
}
@@ -3560,24 +3560,6 @@ function mdb_get_uint32
return 0
}

#
# Get the hexadecimal value of global variable using mdb.
#
function mdb_get_hex
{
typeset variable=$1
typeset value

value=$(mdb -k -e "$variable::print -x")
if [[ $? -ne 0 ]]; then
log_fail "Failed to get value of '$variable' from mdb."
return 1
fi

echo $value
return 0
}

#
# Set global uint32_t variable to a decimal value using mdb.
#

0 comments on commit fae035d

Please sign in to comment.
You can’t perform that action at this time.