Page MenuHomeFreeBSD

kern/intr: declare interrupt vectors unsigned
ClosedPublic

Authored by ehem_freebsd_m5p.com on Mar 18 2021, 5:06 PM.
Tags
None
Referenced Files
F96162068: D29327.diff
Mon, Sep 23, 10:25 PM
F96153061: D29327.id86036.diff
Mon, Sep 23, 9:21 PM
F96014301: D29327.id85979.diff
Mon, Sep 23, 8:52 AM
F96014291: D29327.id85979.diff
Mon, Sep 23, 8:52 AM
F95991848: D29327.id86036.diff
Mon, Sep 23, 6:43 AM
Unknown Object (File)
Sun, Sep 22, 9:39 PM
Unknown Object (File)
Sun, Sep 22, 6:44 AM
Unknown Object (File)
Mon, Sep 16, 4:33 AM

Details

Summary

These should never get values large enough for sign to matter, but one
of them becoming negative could cause problems.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 37949
Build 34838: arc lint + arc unit

Event Timeline

With a handy build complete, now I know of an extra instance. The
access in sys/arm64/arm64/gic_v3.c seems likely to be uneffected.

Hmm, that title was less than wonderful.

ehem_freebsd_m5p.com retitled this revision from kern/intr: declare many variables unsigned to kern/intr: declare interrupt vectors unsigned.Mar 19 2021, 8:42 PM

There actually aren't (yet) many places where this matters. One place it does matter is isrc_alloc_irq() which deals with the situation by having the "maxirqs" variable shadow intr_nirq. Since intr_nirq will never sensibly be negative it seems better for INTR_IRQ_INVALID to be a large positive value and testing whether an interrupt has a valid value be >= intr_nirq.

I note the isrc_irq entry of struct intr_irqsrc is unsigned. This actually effects D29310, since it means the test used needs a slight adjustment. This also allows D29326, which is getting rid of the shadow variable isrc_alloc_irq() has.

Any chance of review of D29327 and D29310? Presently I'm simply waiting doing nothing...

The 1 month anniversary of D29327. I think I've got appropriate people as reviewers, but no action...

I think this change is fine. I will tinderbox and commit it in a couple of days if no one objects.

With respect to D29310, I suspect it's not getting attention because it does indeed seems like a bikeshed. If the table is full, the problem will manifest as drivers not being able to attach, probably during boot, resulting in an unambiguous failure mode. Interrupt allocation isn't a dynamic operation, its performance is not important and doesn't warrant much scrutiny. Much more so for the unlikely case of interrupt allocation _after_ the table has been filled and some interrupts are freed.

This revision is now accepted and ready to land.Apr 19 2021, 9:19 PM
This revision was automatically updated to reflect the committed changes.