kern: remove clk_intr_event from non-ACPI
Needs ReviewPublic
Actions

Authored by ehem_freebsd_m5p.com on Jun 27 2023, 6:53 PM.

Details

Reviewers

kib
jhb
mav
mjg

Summary

While there may have been many uses of clk_intr_event in the past, now
the ACPI core is the only use. Remove it from all non-ACPI kernels.

Correct the uses of intr_event_handle(). If no handlers are added by
ACPI, invoking intr_event_handle() is incorrect as it is stray.

Fixes: aba10e131fe ("Allow swi_sched() to be called from NMI context.")

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 52301
Build 49192: arc lint + arc unit

Event Timeline

ehem_freebsd_m5p.com created this revision.Jun 27 2023, 6:53 PM

Herald added a subscriber: imp. · View Herald TranscriptJun 27 2023, 6:53 PM

Harbormaster completed remote builds in B52301: Diff 123860.Jun 27 2023, 6:53 PM

ehem_freebsd_m5p.com requested review of this revision.Jun 27 2023, 6:53 PM

Unfortunately there are several overlapping issues interwoven here and choices need to be made.

As @mjg correctly asked in D38448, should interrupts which occur on multiple processors ("PPI" on ARM) share a struct intr_event? This approach is taken by INTRNG and @mav took this approach in D25754 with clk_intr_event. The argument against this approach is IPI and PPI interrupts share characteristics which are not shared with normal interrupts. Notably IPI/PPI need multiple counters or atomics for counters. Additionally normal IPI/PPI only use filters, not handlers. I was starting to think @mjg was correct and they should be split, but D25754 has moved in the direction of merging.
clk_intr_event is presently only used by ACPI for ACPI NMI interrupts. D40776 removes it from non-ACPI. Yet there seems merit of going the opposite direction and reimplementing most of hardclock() as filters on clk_intr_event.
What triggered D40776 is if clk_intr_event has no handlers (ACPI is absent or lacks the appropriate interrupt types) then intr_event_handle(clk_intr_event, NULL); reports stray interrupts (comment in intr_event_handle() declares these to be stray). Perhaps a flag should be added for indicating a particular intr_event being triggered with no handlers shouldn't be reported as strays? (I had been wondering about a mysterious "swi4:").
clk_intr_event has almost no effect on kern_intr.c. I think clk_intr_event needs to be moved to kern_clock.c.

I suspect the first item needs consideration first. Should interrupts expected to occur on multiple processors share a struct intr_event? Things are tending to be combined which heads towards a "yes", but there seem some problematic consequences from this.

I need a very good insecticide to get rid of the crickets inhabiting D40776.

Herald added a subscriber: olce. · View Herald TranscriptMay 11 2024, 12:50 AM

ehem_freebsd_m5p.com abandoned this revision.Dec 13 2024, 3:28 AM

A more recent version of the series is now on Github as #1301. Note that is marked as a draft since something else is higher priority right now.

Not especially urgent. I ended up noticing this while working on what became #1301 (since the series causes counts for SWIs to be reported).