x86/intr: introduce an isrc lock

[royger]

Summary

This is needed so interrupt handlers can be removed while the PIC is
resuming, it was previously not possible due to intr_resume holding the
intr_table_lock and intr_remove_handler recursing on it.

A per interrupt source mutex could be added, but this seems too much.

Sponsored by: Citrix Systems R&D

[kib]

Isn't adding yet another mutex there is an overkill ? The interrupt source registration and deregistration is rather rare events. I consider it acceptable to check for intr_table_lock ownership in the intr_remove_handler() function.

I do not object against the patch strongly, just find it wasteful both WRT memory and the brain cells to understand the more complex locking scheme.

[skra]

WRT arm intrng I'm working on, I just wonder why the one who wants to remove intr handler cannot wait until PICs are resumed?

[royger]

In D4114#86644, @onwahe-gmail-com wrote:

WRT arm intrng I'm working on, I just wonder why the one who wants to remove intr handler cannot wait until PICs are resumed?

When running on Xen all event channels are completely removed on suspend/resume, so we must clear them from the PIC. Waiting for the PIC to be resumed just adds unnecessary complexity and will leave the system in a broken state.

[skra]

In D4114#86645, @royger wrote:

In D4114#86644, @onwahe-gmail-com wrote:

WRT arm intrng I'm working on, I just wonder why the one who wants to remove intr handler cannot wait until PICs are resumed?

When running on Xen all event channels are completely removed on suspend/resume, so we must clear them from the PIC. Waiting for the PIC to be resumed just adds unnecessary complexity and will leave the system in a broken state.

So, you are saying that you can manage internally on PIC that pic_disable/enable_intr() and pic_disable/enable_source() are call under intr_isrc_lock and pic_resume/suspend() are call under intr_table_lock?

In other words, that for example an implementation of pic_disable_intr() won't wait on some lock used by an implementation of pic_resume() in some PIC?

[royger]

Do as suggested by kib and check for mutex ownership in intr_remove_handler.

[kib]

Otherwise two small notes, I am fine with the change.

sys/x86/x86/intr_machdep.c

205 ↗

(On Diff #10099)

Style requires having all local declarations placed at the starting brace.

207 ↗

(On Diff #10099)

Please add a comment explaining why the recursion is possible.