While 1db7829e5657 ("x86/hpet: do local APIC EOI after interrupt processing")
helped quite a bit, nested interrupts could still occur. First and foremost
as a result from IRQ migration (where we don't have any control over the
vectors chosen). Hence besides reducing the number of IRQs that can be raised
(first two patches) and possibly the number of invocations of
handle_hpet_broadcast() from the IRQ handler (optional patch 4), the main
goal here is to eliminate the potential for nested IRQs (patch 3). These
patches are imo 4.21 candidates (with patch 4 being questionable altogether;
see there). Patches 5 and onwards likely aren't important enough anymore at
this point of the release cycle, even if those with a Fixes: tag would likely
end up being backported later on.

The one related thing I haven't been able to find a viable solution for is
the elimination of the cpumask_t local variable in handle_hpet_broadcast().
That'll get in the way of possible future increases of the NR_CPUS upper
bound: Much like right now a single level of nesting is already too much,
if the limit was doubled even a single IRQ would end up consuming too much
stack space (together with cpumask_raise_softirq() also having such a
variable). Yet further doubling would not allow any such stack variables
anymore.

01: limit channel changes
02: disable unused channels
03: use single, global, low-priority vector for broadcast IRQ
04: ignore "stale" IRQs
05: avoid indirect call to event handler
06: make another channel flags update atomic
07: move legacy tick IRQ count adjustment
08: shrink IRQ-descriptor locked region in set_channel_irq_affinity()
09: reduce hpet_next_event() call sites
10: don't use hardcoded 0 for "long timeout"

Jan

On Thu, Oct 16, 2025 at 09:30:28AM +0200, Jan Beulich wrote:
> While 1db7829e5657 ("x86/hpet: do local APIC EOI after interrupt processing")
> helped quite a bit, nested interrupts could still occur. First and foremost
> as a result from IRQ migration (where we don't have any control over the
> vectors chosen). Hence besides reducing the number of IRQs that can be raised
> (first two patches) and possibly the number of invocations of
> handle_hpet_broadcast() from the IRQ handler (optional patch 4), the main
> goal here is to eliminate the potential for nested IRQs (patch 3). These
> patches are imo 4.21 candidates (with patch 4 being questionable altogether;
> see there). Patches 5 and onwards likely aren't important enough anymore at
> this point of the release cycle, even if those with a Fixes: tag would likely
> end up being backported later on.
> 
> The one related thing I haven't been able to find a viable solution for is
> the elimination of the cpumask_t local variable in handle_hpet_broadcast().
> That'll get in the way of possible future increases of the NR_CPUS upper
> bound: Much like right now a single level of nesting is already too much,
> if the limit was doubled even a single IRQ would end up consuming too much
> stack space (together with cpumask_raise_softirq() also having such a
> variable). Yet further doubling would not allow any such stack variables
> anymore.

If there's no nesting anymore, you could introduce a per-CPU cpumask_t
to use in the context of handle_hpet_broadcast()?

Thanks, Roger.

On 16.10.2025 12:05, Roger Pau Monné wrote:
> On Thu, Oct 16, 2025 at 09:30:28AM +0200, Jan Beulich wrote:
>> While 1db7829e5657 ("x86/hpet: do local APIC EOI after interrupt processing")
>> helped quite a bit, nested interrupts could still occur. First and foremost
>> as a result from IRQ migration (where we don't have any control over the
>> vectors chosen). Hence besides reducing the number of IRQs that can be raised
>> (first two patches) and possibly the number of invocations of
>> handle_hpet_broadcast() from the IRQ handler (optional patch 4), the main
>> goal here is to eliminate the potential for nested IRQs (patch 3). These
>> patches are imo 4.21 candidates (with patch 4 being questionable altogether;
>> see there). Patches 5 and onwards likely aren't important enough anymore at
>> this point of the release cycle, even if those with a Fixes: tag would likely
>> end up being backported later on.
>>
>> The one related thing I haven't been able to find a viable solution for is
>> the elimination of the cpumask_t local variable in handle_hpet_broadcast().
>> That'll get in the way of possible future increases of the NR_CPUS upper
>> bound: Much like right now a single level of nesting is already too much,
>> if the limit was doubled even a single IRQ would end up consuming too much
>> stack space (together with cpumask_raise_softirq() also having such a
>> variable). Yet further doubling would not allow any such stack variables
>> anymore.
> 
> If there's no nesting anymore, you could introduce a per-CPU cpumask_t
> to use in the context of handle_hpet_broadcast()?

Not quite, as there are other callers, too. But yes, possibly other callers
could be dealt with differently.

Jan

On 10/16/25 9:30 AM, Jan Beulich wrote:
> While 1db7829e5657 ("x86/hpet: do local APIC EOI after interrupt processing")
> helped quite a bit, nested interrupts could still occur. First and foremost
> as a result from IRQ migration (where we don't have any control over the
> vectors chosen). Hence besides reducing the number of IRQs that can be raised
> (first two patches) and possibly the number of invocations of
> handle_hpet_broadcast() from the IRQ handler (optional patch 4), the main
> goal here is to eliminate the potential for nested IRQs (patch 3). These
> patches are imo 4.21 candidates (with patch 4 being questionable altogether;
> see there).

I am not sure that patch 4 is very useful at the current stage.
The first three patches look useful enough for now, so:
  Releaase-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>

~ Oleksii

> Patches 5 and onwards likely aren't important enough anymore at
> this point of the release cycle, even if those with a Fixes: tag would likely
> end up being backported later on.
>
> The one related thing I haven't been able to find a viable solution for is
> the elimination of the cpumask_t local variable in handle_hpet_broadcast().
> That'll get in the way of possible future increases of the NR_CPUS upper
> bound: Much like right now a single level of nesting is already too much,
> if the limit was doubled even a single IRQ would end up consuming too much
> stack space (together with cpumask_raise_softirq() also having such a
> variable). Yet further doubling would not allow any such stack variables
> anymore.
>
> 01: limit channel changes
> 02: disable unused channels
> 03: use single, global, low-priority vector for broadcast IRQ
> 04: ignore "stale" IRQs
> 05: avoid indirect call to event handler
> 06: make another channel flags update atomic
> 07: move legacy tick IRQ count adjustment
> 08: shrink IRQ-descriptor locked region in set_channel_irq_affinity()
> 09: reduce hpet_next_event() call sites
> 10: don't use hardcoded 0 for "long timeout"
>
> Jan