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

RwLock::read for UFT hits, kstats use AtomicU64 #636

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

FelixMcFelix
Copy link
Collaborator

This PR replaces the per-Port KMutex with a KRwLock. This allows
ioctls to read port state without preventing packets from being served.
This also prevents successful UFT hits from blocking one another on the
port for any length of time.

A requirement is that we move to atomic accesses on kstat values, which
allows us to update kstats from outside a write-lock on the port.

The odd thing is that there doesn't seem to be a performance
improvement (in Mbps). Contention according to lockstat is solved, and
apparently the time spent in xde_rx/xde_mc_tx is reduced -- we're no
longer:

  • S2C -- the 3rd and 10th most spin-happy locks,
  • C2S -- the 2rd and 3rd most spin-happy locks.

Nor are we egregiously blocking on the port lock.

This PR replaces the per-`Port` `KMutex` with a `KRwLock`. This allows
ioctls to read port state without preventing packets from being served.
This also prevents successful UFT hits from blocking one another on the
port for any length of time.

A requirement is that we move to atomic accesses on kstat values, which
allows us to update kstats from outside a write-lock on the port.

The odd thing is that there doesn't seem to be a performance
improvement (in Mbps). Contention according to lockstat is solved, and
apparently the time spent in `xde_rx`/`xde_mc_tx` is reduced -- we're no
longer:

 * S2C -- the 3rd and 10th most spin-happy locks,
 * C2S -- the 2rd and 3rd most spin-happy locks.

Nor are we egregiously blocking on the port lock.
@FelixMcFelix FelixMcFelix self-assigned this Dec 18, 2024
@FelixMcFelix FelixMcFelix added this to the 13 milestone Dec 18, 2024
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