make maximum interrupt number tunable on ARM, ARM64, MIPS, and RISC-V
ClosedPublic
Actions

Authored by gonzo on Dec 30 2020, 7:02 AM.

Details

Reviewers

allanjude
jhb
andrew
mmel
mhorne
scottph
manu

Commits

rG248f0cabca75: make maximum interrupt number tunable on ARM, ARM64, MIPS, and RISC-V

Summary

Use a machdep.nirq tunable intead of compile-time constant NIRQ
as a value for maximum number of interrupts. It allows keep a system
footprint small by default with an option to increase the limit
for large systems like server-grade ARM64

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 36159
Build 33048: arc lint + arc unit

Event Timeline

gonzo created this revision.Dec 30 2020, 7:02 AM

Herald added a reviewer: manu. · View Herald TranscriptDec 30 2020, 7:02 AM

Herald added subscribers: emaste, imp. · View Herald Transcript

gonzo requested review of this revision.Dec 30 2020, 7:02 AM

Harbormaster completed remote builds in B35787: Diff 81362.Dec 30 2020, 7:02 AM

mhorne added inline comments.Jan 4 2021, 7:03 PM

sys/kern/subr_intr.c
146–147	Since we are now sizing this dynamically, it would make more sense to do it based on `mp_ncpus`. You might also drop this constant and just inline it in `intr_irq_init()`, since it is not used elsewhere.
151	`machdep_` is an unusual prefix for a variable --- I don't think anything else is named that way. Maybe `intr_nirq` if not just `nirq`?
411	Is there a reason to use `irq_sources_count` rather than just `machdep_nirq`?
1734	Could push M_WAITOK to the next line if it becomes too long.

gonzo marked 4 inline comments as done.Jan 13 2021, 6:03 AM

gonzo added inline comments.

sys/kern/subr_intr.c
146–147	Fixed the constant. I'd like to leave mp_ncpus for another round of changes.
151	Fixed
411	Just an automatic renaming. Fixed.

Address mhorne's comments

Harbormaster completed remote builds in B36159: Diff 82190.Jan 13 2021, 6:04 AM

A couple of small nitpicks, otherwise LGTM.

sys/kern/subr_intr.c
186	You can drop the extra newline.
411	Thanks, but it looks like `irq_sources_count` can be dropped altogether in favor of `machdep_nirq`.

This revision is now accepted and ready to land.Jan 13 2021, 8:04 PM

gonzo marked 2 inline comments as done.Jan 14 2021, 5:32 AM

Closed by commit rG248f0cabca75: make maximum interrupt number tunable on ARM, ARM64, MIPS, and RISC-V (authored by gonzo). · Explain WhyJan 19 2021, 12:37 AM

This revision was automatically updated to reflect the committed changes.

gonzo added a commit: rG248f0cabca75: make maximum interrupt number tunable on ARM, ARM64, MIPS, and RISC-V.

FYI, I believe that this commit broke the kernel on MIPS64, at least via qemu: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=253051

This is rather a bit overdue, but I note D27844 has a troublesome interaction with D16861. At fd036deac169, vmstat was modified to look for nintrcnt to distinguish kernel core dumps in which intrcnt/intrnames were arrays versus pointers. As a result right now if an ARM[64]/RISC-V kernel core dump was analyzed using vmstat the interrupt counts will be misinterpreted since it will expect an array instead of a pointer.

I had been pondering renaming intrcnt_count to nintrcnt as part of D38305, but had decided that was unnecessary. Yet now noticing this concern I'm wonder whether I should bring that back due to this issue.