Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore too large stack in case of dsl_deadlist_merge #14524

Merged
merged 1 commit into from
Feb 27, 2023
Merged

Ignore too large stack in case of dsl_deadlist_merge #14524

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions module/zfs/dsl_deadlist.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,7 @@ void
dsl_deadlist_merge(dsl_deadlist_t *dl, uint64_t obj, dmu_tx_t *tx)
{
zap_cursor_t zc, pzc;
zap_attribute_t za, pza;
zap_attribute_t *za, *pza;
dmu_buf_t *bonus;
dsl_deadlist_phys_t *dlp;
dmu_object_info_t doi;
Expand All @@ -875,28 +875,31 @@ dsl_deadlist_merge(dsl_deadlist_t *dl, uint64_t obj, dmu_tx_t *tx)
return;
}

za = kmem_alloc(sizeof (*za), KM_SLEEP);
pza = kmem_alloc(sizeof (*pza), KM_SLEEP);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be more efficient to do a single allocation like this:

za = kmem_alloc(sizeof (*za) * 2, KM_SLEEP);
pza = &za[1];

Then we would free the memory by calling kmem_free(za, sizeof (*za) * 2); at the end of the function.

That said, I do not feel strongly about this, so I am fine with merging this either way. This is purely a suggestion for a way to minimize the runtime overhead of using dynamic memory to reduce stack space usage that stood out to me.


mutex_enter(&dl->dl_lock);
/*
* Prefetch up to 128 deadlists first and then more as we progress.
* The limit is a balance between ARC use and diminishing returns.
*/
for (zap_cursor_init(&pzc, dl->dl_os, obj), i = 0;
(perror = zap_cursor_retrieve(&pzc, &pza)) == 0 && i < 128;
(perror = zap_cursor_retrieve(&pzc, pza)) == 0 && i < 128;
zap_cursor_advance(&pzc), i++) {
dsl_deadlist_prefetch_bpobj(dl, pza.za_first_integer,
zfs_strtonum(pza.za_name, NULL));
dsl_deadlist_prefetch_bpobj(dl, pza->za_first_integer,
zfs_strtonum(pza->za_name, NULL));
}
for (zap_cursor_init(&zc, dl->dl_os, obj);
(error = zap_cursor_retrieve(&zc, &za)) == 0;
(error = zap_cursor_retrieve(&zc, za)) == 0;
zap_cursor_advance(&zc)) {
uint64_t mintxg = zfs_strtonum(za.za_name, NULL);
dsl_deadlist_insert_bpobj(dl, za.za_first_integer, mintxg, tx);
uint64_t mintxg = zfs_strtonum(za->za_name, NULL);
dsl_deadlist_insert_bpobj(dl, za->za_first_integer, mintxg, tx);
VERIFY0(zap_remove_int(dl->dl_os, obj, mintxg, tx));
if (perror == 0) {
dsl_deadlist_prefetch_bpobj(dl, pza.za_first_integer,
zfs_strtonum(pza.za_name, NULL));
dsl_deadlist_prefetch_bpobj(dl, pza->za_first_integer,
zfs_strtonum(pza->za_name, NULL));
zap_cursor_advance(&pzc);
perror = zap_cursor_retrieve(&pzc, &pza);
perror = zap_cursor_retrieve(&pzc, pza);
}
}
VERIFY3U(error, ==, ENOENT);
Expand All @@ -909,6 +912,9 @@ dsl_deadlist_merge(dsl_deadlist_t *dl, uint64_t obj, dmu_tx_t *tx)
memset(dlp, 0, sizeof (*dlp));
dmu_buf_rele(bonus, FTAG);
mutex_exit(&dl->dl_lock);

kmem_free(za, sizeof (*za));
kmem_free(pza, sizeof (*pza));
}

/*
Expand Down