This is a follow-up of !998 now that we have a better idea of what a design for enabling and disabling IRQs will look like. Currently, the most recent version of the patch series we have for interrupt management is here:
https://lkml.org/lkml/2024/8/1/1454
During the last re-spin of this series though, it was pointed out that there was a situation which I entirely missed. with_irqs_disabled() works great for simple situations where you just want to turn IRQs off for the duration of a callback, and then turn them back on afterwards. However, there's more complicated scenarios that can occur in the kernel. One such example that Benno provided:
local_irq_save(flags);
while (todo) {
todo = do_sth();
if (too_long) {
local_irq_restore(flags);
if (!irqs_disabled())
sleep();
local_irq_save(flags);
}
}
local_irq_restore(flags);
After quite a long discussion in https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/Spinlocks.20with.20IRQs.3F we were able to come up with a pretty decent idea for a solution to this that still allows the more simple with_irqs_disabled in its current form to remain safe code.
Basically: once we start wanting to handle more complicated scenarios, we would:
- Introduce an
OwnedIrqDisabled type which can hand out IrqDisabled<'a> tokens
- Rename
with_irqs_disabled to with_irqs_disabled_always
- Introduce a new unsafe
with_irqs_disabled function that would provide access to an OwnedIrqDisabled. This would come with a rather complicated safety contract.
The safety contract would basically be something like this (I've just copied the summary I typed up already. Note, the examples are not complete!):
/// Run the closure `cb` with interrupts disabled (possibly temporarily) on the local CPU.
///
/// This creates an [`OwnedIrqDisabled`] token and passes it to the closure by-value. Generally, this
/// function should only be used in situations where it is possible that interrupts may be
/// temporarily re-enabled within the closure, as it requires significantly more care than
/// [`with_irqs_disabled_always`] to be used properly.
///
/// Generally, situations in which using this function is the right thing to do will usually involve
/// doing some work with interrupts disabled, re-enabling interrupts and then sleeping until a
/// condition is fulfilled, and then re-disabling interrupts afterwards.
///
/// # Panics
///
/// In debug builds, this function will assert that `irq` matches the current state of interrupts at
/// the beginning of function. This means if interrupts were disabled when the function was called,
/// `irq` should be `Some(OwnedIrqDisabled)`, otherwise it should be `None`. This function will restore
/// the interrupt state to how it was at the start of the function call, and return a corresponding
/// `Option<OwnedIrqDisabled>` token that may be used for further operations that require interrupts
/// disabled (if there are any).
///
/// # Safety
///
/// * The caller must ensure that if interrupts are re-enabled, the [`OwnedIrqDisabled`] token goes out
/// of scope for the duration in which interrupts are enabled.
/// * Any function which calls this function *must* have an argument of the type
/// `Option<OwnedIrqDisabled>` in its signature. This argument must reflect the state of interrupts at
/// the start of the function call.
/// * The `irq` argument of this function must always accurately reflect the state of interrupts
/// being enabled at the time this function is called. If interrupts were enabled, `irq` should be
/// `None`. Otherwise, it should be `Some(OwnedIrqDisabled)`. The argument required by the previous
/// safety requirement or the returned result from this function should be used for this.
/// * The interrupt state at the end of the closure must match the interrupt state at the beginning
/// of the function call.
/// * Additionally, a token accurately representing the state of interrupts at the end of the
/// closure *must* be returned from any function calling this as a `Option<OwnedIrqDisabled>`.
/// * Finally, because the safety requirements of this function must be upheld by the caller and
/// cannot yet be enforced by the compiler - any function calling this function must be `unsafe`
/// and have a safety contract that requires the caller ensure that the `irq` input argument
/// reflects the state of interrupts being enabled at the beginning of the function call.
///
/// # Examples
///
/// Using [`with_irqs_disabled`] to call a function that disables interrupts, temporarily re-enables
/// interrupts, and then returns with interrupts disabled.
///
/// ```
/// use bindings;
/// use kernel::irq::{OwnedIrqDisabled, with_irqs_disabled_always};
///
/// /// Does some work with interrupts explicitly enabled, and returns with them in their original
/// /// state
/// unsafe fn enable_interrupts_work(irq: Option<OwnedIrqDisabled>) -> Option<IrqDisabled> {
/// }
///
/// /// Does something incredible with interrupts disabled, enables them, then disables them again
/// ///
/// /// A new [`OwnedIrqDisabled`] token will be returned from this function if interrupts were disabled
/// /// when this function was called.
/// ///
/// /// # Safety
/// ///
/// /// * The caller must ensure that `irq` reflects the current state of interrupts at the time
/// /// this function is called.
/// unsafe fn interrupt_me_sometimes(irq: Option<OwnedIrqDisabled>) -> Option<IrqDisabled> {
///
/// }
/// ```
Currently this isn't part of the main series for the irq module, as I don't think we really have any users that need this quite yet.
This is a follow-up of !998 now that we have a better idea of what a design for enabling and disabling IRQs will look like. Currently, the most recent version of the patch series we have for interrupt management is here:
https://lkml.org/lkml/2024/8/1/1454
During the last re-spin of this series though, it was pointed out that there was a situation which I entirely missed.
with_irqs_disabled()works great for simple situations where you just want to turn IRQs off for the duration of a callback, and then turn them back on afterwards. However, there's more complicated scenarios that can occur in the kernel. One such example that Benno provided:After quite a long discussion in https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/Spinlocks.20with.20IRQs.3F we were able to come up with a pretty decent idea for a solution to this that still allows the more simple
with_irqs_disabledin its current form to remain safe code.Basically: once we start wanting to handle more complicated scenarios, we would:
OwnedIrqDisabledtype which can hand outIrqDisabled<'a>tokenswith_irqs_disabledtowith_irqs_disabled_alwayswith_irqs_disabledfunction that would provide access to anOwnedIrqDisabled. This would come with a rather complicated safety contract.The safety contract would basically be something like this (I've just copied the summary I typed up already. Note, the examples are not complete!):
Currently this isn't part of the main series for the
irqmodule, as I don't think we really have any users that need this quite yet.