Re: Interrupts vs signals

Hi,

On 2025-03-06 02:47:38 +0200, Heikki Linnakangas wrote:
> > I was chatting with Heikki about this patch and he mentioned that he recalls a
> > patch that did some work to unify the signal replacement, procsignal.h and
> > CHECK_FOR_INTERRUPTS(). Thomas, that was probably from you? Do you have a
> > pointer, if so?
> > 
> > It does seem like we're going to have to do some unification here. We have too
> > many different partially overlapping, partially collaborating systems here.
> > 
> > 
> > - procsignals - kinda like the interrupts here, but not quite. Probably can't
> >   just merge them 1:1 into the proposed mechanism, or we'll run out of bits
> >   rather soon.  I don't know if we want the relevant multiplexing to be built
> >   into interrupt.h or not.
> > 
> >   Or we ought to redesign this mechanism to deal with more than 32 types of
> >   interrupt.
> 
> In this patch, 29 out of 32 bits are used. Yeah, that doesn't leave much
> headroom.

That does seem way too tight, particularly because the patch has several uses
of GENERAL that shouldn't really be using that.


> Some ideas for addressing that:
> 
> 1. Use 64-bit atomics. Are there any architectures left where that would not
> be acceptable?

I had thought that armv7 would be a problem, but it isn't - it just doesn't
have 8 byte atomicity for reads/writes that aren't explicitly atomic.  Afaict
that leaves us with <= i486 and <= armv6 as not supporting 64bit atomics -
which I think we could desupport.  I guess I'll start a a thread about that.


> 2. Use 64-bit integers, but for the actual signaling part, split it into two
> 32-bit atomic words. Waiting on the interrupts would need to check both, but
> that seems fine from a performance point of view.
> 
> 3. If we need > 64 bits, things get a bit awkward as you can no longer have
> a single integer to represent a bitmask that includes all the bits. That
> makes the function signatures more ugly, you need to have two arguments like
> interruptMask1 and interruptMask2 or somehting. But should basically work
> otherwise.
> 
> 4. Multiplex the interrupts so that we need fewer of them. I think all the
> recovery conflict interrupts could be merged into one, for example, if we
> had a separate bitmask somewhere else in PGPROC to indicate which ones are
> set. The limitation would be that if interrupts are multiplexed together
> into one bit, you could only wait for all or none of them.

If we go that way, I suspect we should bake this into the interrupt.[ch]
system itself, rather than open-code it in a bunch of places.  We could
e.g. split the current InterruptType into a class and type portion - that
actually also might help with writing more efficient interrupt processing
functions.  I'm imagining that the main atomic would just contain bits for
each interrupt class, with an array of atomics for the sub-portions of a
class. That would mean you couldn't wait on sub-parts of a class, but that
seems fine.

Another advantage of having something like interrupt classes could be that it
would make it a bit easier for code to specify what interrupts can be
processed. Instead of having to know about all the different
INTERRUPT_*_TIMEOUT or INTERRUPT_*{MESSAGE,LOG}* interrupt IDs code code just
specify whether it can deal with e.g. messages being processed.


> I'd like to not have to treat these bits as a scarce resource. It'd be nice
> to use even more distinct interrupts than what's in the patch now for
> different things, just for clarity. And I'd like to make it possible for
> extensions to allocate interrupt bits too. (Not included in these patches
> yet)

> I'm leaning towards 1. or 2. at the moment. 64 bits should be enough for a
> long time.

I'm not so sure about that, particularly not if interrupts can be allocated in
extensions.


I'm not really in love how this looks like yet, tbh. Aspects of it seem to buy
further into some of our already existing design mistakes. I have two main
concerns:

1) It seems rather bonkers that we have to go to every process type and add
code for INTERRUPT_LOG_MEMORY_CONTEXT or INTERRUPT_CONFIG_RELOAD.

2) The fact that a lot of code specifies explicit interrupt masks makes that
code harder to compose, because whether we can process some interrupt or not
depends on higher level state.  That's e.g. why INTERRUPT_CFI_MASK() has to be
a bit magic and decid ewhat interrupt mask to use and why we need a separate
interrupt handling functions for interrupts happening while waiting for
network IO than normally.


For 1), I suspect that we should have a central mapping array between
InterruptTypes and the code to handle each of the interrupt types.  That
mapping table also could serve as the registry for extensions to register
their own interrupt ids.

For 2), I wonder if we ought to have a global mask of interrupt kinds that can
be processed in some context. Instead of having INTERRUPT_CFI_MASK() compute
what mask to use, we could have things like HOLD_CANCEL_INTERRUPTS be defined
as something like

if (InterruptHoldCount[CANCEL]++ == 0)
   InterruptMask &= ~CANCEL;

which would allow CHECK_FOR_INTERRUPTS to just use InterruptMask to check for
to-be-processed interrupts.


Greetings,

Andres Freund