Service each IBS unit with a valid bit set when fetch and op share an NMI. Treat the extra NMI that follows as expected.
Details
Details
- Reviewers
gnn ali_mashtizadeh.com mhorne - Group Reviewers
pmc - Commits
- rG34b00ed041a4: hwpmc: fix IBS fetch and op NMI handling
Test simultaneous IBS fetch and op sampling repeatedly; verify both streams produce samples and no NMIs are left unhandled.
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Summary: hwpmc: fix IBS fetch and op
Service each IBS unit with a valid bit set when fetch and op share an NMI. Treat the extra NMI that follows as expected.
Test Plan:
Validated on fbsd-dev: three scimark4 fetch+op runs produced about 39k fetch and 322k op samples per run, with zero ignored NMIs.
Reviewers: mhorne, pmc
Comment Actions
hwpmc: fix IBS fetch and op
Service each IBS unit with a valid bit set when fetch and op share an NMI. Treat the extra NMI that follows as expected.
Test Plan:
Validated on fbsd-dev: three scimark4 fetch+op runs produced about 39k fetch and 322k op samples per run, with zero ignored NMIs.
Reviewers: mhorne, pmc
Comment Actions
Sure. I admit that I do not see why this is advantageous one way or the other. In either case two NMIs must be processed, no?
| sys/dev/hwpmc/hwpmc_ibs.c | ||
|---|---|---|
| 734–740 | To me, this expresses the intent more clearly. | |
Comment Actions
The issues is that without this patch we lose op samples. Fetch and op share the same NMI, so when both fire together we get a single NMI with both valid bits set.
Since we now service both units on every NMI, op can fire while the handler is running and get serviced there. Its own NMI still arrives after the handler returns, with nothing left to service. pc_nmi_credit claims that one so it doesn't show up as an unknown NMI and panic the box.
| sys/dev/hwpmc/hwpmc_ibs.c | ||
|---|---|---|
| 734–740 | Yes, that's clearer. One small thing: the extra NMI always comes right after the one that serviced both units, so I think the credit should be cleared on every NMI, even one that only services a single unit. Otherwise a credit that never gets used could eat some unrelated NMI later on. I restructured it along the lines you suggested but kept that behavior. | |
Comment Actions
Hi @mhorne , I submitted the patch with your suggestion, could you take a look? Thanks for your time.