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.

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

Split deep vmem_alloc()/vmem_xalloc() stacks #94

Merged

Conversation

rottegift
Copy link
Collaborator

In #90
a user reported panics on an M1 with the message

"Invalid kernel stack pointer (probable overflow)."

In at least several of these a deep multi-arena allocation
was in progress (several vmem_alloc/vmem_xalloc reaching
all the way down through vmem_bucket_alloc,
xnu_alloc_throttled, and ultimately to osif_malloc).

The stack frames above the first vmem_alloc were also fairly large.

This commit sets a dynamically sysctl-tunable threshold
(8k default) for remaining stack size as reported by xnu.
If we do not have more bytes than that when vmem_alloc()
is called, then the actual allocation will be done in a
separate worker thread which will start with a nearly
empty stack that is much more likely to hold the various
frames all the way through our code boundary with the
kernel and beyond.

The xnu / mach thread_call API (osfmk/kern/thread_call.h)
is used to avoid circular dependencies with taskq, and the
mechanism is per-arena costing a quick stack-depth check
per vmem_alloc() but allowing for wildly varying stack
depths above the first vmem_alloc() call.

Vmem arenas now have two further kstats: the lowest amount
of available stack space seen at a vmem_alloc() into it,
and the number of times the allocation work has been done
in a thread_call worker.

  • some spl_vmem.c functions are given inline hints

These are small functions with no or very few automatic
variables that were good candidates for clang/llvm's
inlining heuristics before we switched to building
the kext with -finline-hint-functions.

  • remove some (unrelated) unused variables which escaped
    previous commits, eliminating a couple compile-time warnings

Motivation and Context

Description

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@rottegift rottegift requested a review from lundman July 24, 2021 07:38
@rottegift rottegift self-assigned this Jul 24, 2021
In openzfsonosx#90
a user reported panics on an M1 with the message

"Invalid kernel stack pointer (probable overflow)."

In at least several of these a deep multi-arena allocation
was in progress (several vmem_alloc/vmem_xalloc reaching
all the way down through vmem_bucket_alloc,
xnu_alloc_throttled, and ultimately to osif_malloc).

The stack frames above the first vmem_alloc were also fairly large.

This commit sets a dynamically sysctl-tunable threshold
(8k default) for remaining stack size as reported by xnu.
If we do not have more bytes than that when vmem_alloc()
is called, then the actual allocation will be done in a
separate worker thread which will start with a nearly
empty stack that is much more likely to hold the various
frames all the way through our code boundary with the
kernel and beyond.

The xnu / mach thread_call API (osfmk/kern/thread_call.h)
is used to avoid circular dependencies with taskq, and the
mechanism is per-arena costing a quick stack-depth check
per vmem_alloc() but allowing for wildly varying stack
depths above the first vmem_alloc() call.

Vmem arenas now have two further kstats: the lowest amount
of available stack space seen at a vmem_alloc() into it,
and the number of times the allocation work has been done
in a thread_call worker.

* some spl_vmem.c functions are given inline hints

These are small functions with no or very few automatic
variables that were good candidates for clang/llvm's
inlining heuristics before we switched to building
the kext with -finline-hint-functions.

* remove some (unrelated) unused variables which escaped
previous commits, eliminating a couple compile-time
warnings.
@rottegift rottegift force-pushed the vmem-alloc-stack-depth-pr branch 2 times, most recently from 7c7cf85 to 6a150c6 Compare July 25, 2021 13:15
@rottegift
Copy link
Collaborator Author

As this at least made #90 better, accepting.

@rottegift rottegift merged commit 5fad674 into openzfsonosx:development Jul 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant