Finally tracked down an actual bug I'd been running into. Since I fear you may wish to take small steps and fix strictly what is buggy, I've split this into a separate commit at the front of my repository. This is clearly a bug with vmstat since this violates the sysctl() interface.

Updating to current tree status. At least one serious bug was fixed.

The number of reviewers and subscribers to D32504 keeps increasing, but the review activity is stale. Since the previous approach was refused, there is now a new approach.

Any reviews for this? While it will take a while to make use of this, I have a definite use for this. I was actually hoping for progress, not merely silence on this.

In D36628#837924, @kib wrote:

You did exactly the opposite to what I suggested: you adjusted userspace to kernel interface, instead of fixing the ABI and keeping old vmstat -i working, by fixing kernel. I do not want to enforce my opinion, but at least I thin we need to get the rationale for going this way instead.

Thank you for figuring that out. Though I just lost rather a lot of sanity points.

I just noticed a serious issue. Turns out intrnames is being interpreted in two very distinct fashions in different places. The resultant output is certainly incorrect and may have been broken before intrnames was introduced, yet never fixed.

This needs moving earlier since it needs to alter xen_intr_handle_upcall()'s declaration.

Splitting this. Depending upon how things evolve, there could be a need to have the two files live side-by-side for a bit. The non-split version was okayed in e-mail.

I've tried this and it unsurprisingly passed. Still have the objection, but noting it does appear to work.

@cperciva could I persuade you to try testing D30006? I'm cautiously optimistic it does in fact work for x86, but the problem is I'm setup for testing ARM64, not x86.

Updating to ensure this is consistent with the state of my tree. This has been moved earlier, before D30909.

Updating per @kevans91 on IRC. I would prefer to have !x86, but !i386 !amd64 is good enough.

Nope. This is purely a one-off which happened to show up in front of me when I was looking at something else. I kind of wonder whether intr_bind() should be fixed and turn into an inline static to start encouraging its use. Right now that single use in nexus was the only use and intr_event_bind() is used all over.

Seems I've run into a benefit of this I hadn't been aiming for. This causes all event counters to be reported. As a result this will report events which aren't hooked up to the platform interrupt subsystem.

@mjg aren't you discussing a distinct potential commit here? I don't believe that is how this is supposed to work.

Having a cpu_t seems a bit overkill. Nearly everywhere uses u_int which works fine. Just in this instance I spotted something rather out of date. On the other hand if I was asked to review a diff which created a cpu_t, I would accept it if it looked reasonable.

Updating to present tree status. Like the others in this sequence, suddenly a distinct approach has a massive impact.

Updating Phabricator due to discussions elsewhere. I suspect this will get further adjustment.

Updating. New approach to this situation which may be more likely to be accepted.

Two fixups. First, D34691 makes the ram device need to be surrounded by #ifdef _KERNEL. Second, due to @jhb's work removing device classes from DRIVER_MODULE(), this needs the same treatment.

Stealing D32343 from @mhorne since this feature is rather important for the project I'm working on.

Due to coming up with an alternative approach, this appears certain to disappear.

Checking turned up a single use of intr_bind(), everywhere else intr_event_bind() was used. At this point the u_char is hazardous, so time to nuke the almost unused function.

And I thought I had responded, but hadn't. Sorry.

Certainly my primary goal could be accomplished that way, I'm presently very dubious about that being better though. Notably the only two benefits of having intrcnt and intrnames I observe are: it makes the sysctl() interface extremely simple; and it is architecture-independent. What you're suggesting would add roughly the same amount of complexity to sysctl() and placing the counters in intr_event improves architecture-independence.

In D35607#836545, @mmel wrote:

So if you have time and want to help, please finish it. But unfortunately I can't help you much, due to external conditions I'm only online a few hours a week, usually without the energy to do anything for FreeBSD. And I can't expect it to improve by the end of the year.

That is your word choice. My interpretation: "I can't be bothered to finish this feature, but it has huge potential, so please take care of finishing this feature for me! Oh, I'm going to block your patch since it would interfere with the plans I don't have time to implement myself"
No. That is not how this works. Blocking with that sort of justification is an abuse of this system.

This likely needs discussion elsewhere, but I believe the time has come for this. intrnames simply duplicates ie_fullname (notice how all the intrcnt_setname() functions simply copy). Then in turn forces isrc_release_counters() to be broken.

Switching to completely removing one comment.

Small tweak. Finally managed to break this into a reasonable series. Now need that okay for how this changes things.

Note this is out there at https://gitlab.com/ehem/freebsd-src.git the branch "D36628" (https://gitlab.com/ehem/freebsd-src/-/commits/D36628/).

Bringing in the remaining bits.

I guess this is being done this way.

I'm extremely suspicious how ipi_hardclock()/ipi_preempt() modify td_intr_frame and td_nesting_level on ARM, but not ARM64. Is this actually needed on ARM?

Hmm, not quite how I thought this was going to work. I'm wondering what action, if any, I'm supposed to take with this now.

Even though intr_ipi_dispatch() duplicates how the existing implementations act, I must object to having such in INTRNG. Likely intr_ipi_dispatch() should handle the trapframe similar to how intr_event_handle() handles them. Everything else seems fairly minor and could be handled afterwords, but I find intr_ipi_dispatch() unacceptable.

This doesn't do much more than provide some support for D36610. I'm unsure of the approach, but needed something to maintain compatibility with D36610. I won't be surprised by abandoning this if a better approach is suggested.

I was under the impression D35559 has the right reviewers, but no action has been observed. This is meant as a precursor to the updated D32504. This can improve several places, but does need someone to okay (and commit to the main tree). Then there are the child commits.

Appears none of the callers of intr_event_create() nor intr_event_destroy() hold locks, therefore sleeping is acceptable. As such switching to a shared-exclusive lock takes care of the concern with the sysctl. Only remaining issue is the watchdog_fire() function and needing an okay for the rather substantial architecture change.

Potential solution to the sysctl issue. Previous round has been tested and confirmed to work (once D36628 is in). This one hasn't reached testing yet. I'm unsure I've got the right reviewers, but fixing intr_isrc_deregister() seems pretty high value.

Hopefully fusing these together doesn't make your job much harder...

Update with fixes for newly found issues. vmstat wasn't matching the kernel.

The situation with the potential need to sleep during sysctl_old_user() still needs addressing and I need a suggestion.

Updating to match current tree status. I discovered I hadn't fully done the job for PowerPC, so fix that problem. Things have shown during testing...

Naturally finding bugs during full build...

I'm not trying to avoid libxo, but I'm not trying to engage it either; I suspect it is similar to Python's string formatting. Don't think about libxo there though, think like the typical C programmer. intrnames[0] == '\0' => empty string, thus blank followed by numbers.

There is a fair bit of history in vmstat -i. Some portions really needed updates long ago. I think I've fixed the additional major problems. Big one was it was using an odd algorithm to identify interrupt name length which didn't really work too well.

Ugh, type up most of how I wanted to respond and then Phabricator drops it on the floor when one thing needs doing...

Based on what I'm looking at, it appears the sysctl interface needs a flag (perhaps CTLFLAG_NOSLEEP?) to indicate the ->oldfunc()/->newfunc() must not sleep. If that is passed those functions would likely return EAGAIN to indicate they need a sleep to continue. In which case the handler needs to either abort, or release locks and indicate the sysctl functions can do their sleep-requiring work.

Updating since I've concluded what I was suggesting really is rather better.

Update to reflect current status. Unfortunately appears vmstat has added assumptions on what the sysctl output will look like and this breaks them. Now to fix those...

This appears to have been an idea for a really small system where needing multiple handlers for a single interrupt was rare. Given how I'm unable to find any evidence of this ever being deployed and their presence serves to cause confusion, I suggest they be completely nuked.

While at it, appears Herald rule H67 has been broken by the configuration change for arc (defaults to rG, not rS).

Trying for random reviewers since I don't have a good idea of who to ask for review/commit for this.

Looks like the problem is troublesome. At the same time though it looks like the present sysctl interface has issues.

Addressing one minor issue. In other news this implementation has been confirmed to partially work, but it definitely has at least one bug.

Finally figured out what had been confusing here. This portion is troublesome and I now think another approach to this is rather better.

Obsoleted by several other bits.

Obsoleted by other work.

Huh, this was still lying around. After a while I figured out how to adjust things work rather better.

I still think the renaming is appropriate, but since the crucial spot has been resolved by other means I'll simply drop this.

Trying for more reviewers since D32793 has simply been sitting out here gathering dust. Seems pretty appropriate as unsigned is by far the more common use.

Abandoning this, D36610 gets the issue of spreading the interrupt counters apart and likely onto separate cache lines.

The real lurking issue is handled by D36610. Abandoning D35608 as it is unlikely to ever see the light of day.

Huh, looking back at this I'm wondering why the mv_ap806 bits were needed.

Ick, dropping the idea of a common maximum possible interrupt number variable. Had seemed worthwhile, but now I'm unsure.

Fixing a distinct goof.

Also note, I've confirmed this builds, I'm less than certain it actually works yet.

Hmm, then I spot the error...

Guess I should also note how sys/dev/hyperv/vmbus/vmbus.c simply had to drop an entry on the floor if its initialization failed, whereas with this that can be cleaned up.

The implementation for sys/kern/kern_clock.c is based on D36609. I have a suspicion that idea may be shot down, but it was the approach which came to mind (since necessary functionality almost existed). The approach for sys/kern/kern_intr.c seems better for that purpose, but that requires a different bit of functionality not presently provided (allowing the outside to enumerate the events).