sys/intr.h: formally depend on machine/intr.h

[kevans]

Summary

sys/intr.h originally started life as an extract of arm's intr.h, and
this include was dropped in its place. Changes in flight want to add
some MD definitions that we'll use in the more MI parts of INTRNG.

Let's formally reverse the dependency now since this is way more
common in general. All of the includes switched in this change that I
spot-checked were in-fact wanting declarations historically included in
sys/intr.h anyways.

[imp]

This is the more typical pattern.

See sys/endian.h including machine/endian.h for but one of many examples (though that example is a bit convoluted in spots due to slight differences between endian.h, sys/endian.h and machine/endian.h consumer expectations.

[ehem_freebsd_m5p.com]

NO ABSOLUTELY NOT. The reason is this makes architecture-independent source impossible without a #ifdef INTRNG.

Take a look at the first two commits of GitHub #1286. The first two create an interrupt_t type definition for things which only handle pointers to the architecture type, but don't directly examine the contents. If you have to #include sys/intr.h, then it is literally impossible to write truly machine-independent beyond having a void *.

An as yet not yet ready patch series proposes adding:

#include <machine/a_bikeshed_string_for_sed_to_target.h>

intr_event_create_device(struct intr_event **event, device_t pic, interrupt_t *source, u_int irq, int flags, const char *fmt, ...);

To <sys/interrupt.h>. If you require #include <sys/intr.h> then this degrades the quality as you must do one of two things:

#ifdef INTRNG
#include <sys/intr.h>
#else
#include <machine/intr_machdep.h>
#endif

intr_event_create_device(struct intr_event **event, device_t pic, interrupt_t *source, u_int irq, int flags, const char *fmt, ...);

Or else:

intr_event_create_device(struct intr_event **event, device_t pic, void *source, u_int irq, int flags, const char *fmt, ...);

The former means it is no longer truly architecture-independent, the latter compromises type-safety. This is a BAD idea!

[ehem_freebsd_m5p.com]

In D47002#1071690, @imp wrote:

This is the more typical pattern.

See sys/endian.h including machine/endian.h for but one of many examples (though that example is a bit convoluted in spots due to slight differences between endian.h, sys/endian.h and machine/endian.h consumer expectations.

This is the more typical pattern since most things in sys are truly machine-independent. sys/intr.h though is not machine-independent, as neither PowerPC nor x86 currently use it.

[jrtc27]

In D47002#1071777, @ehem_freebsd_m5p.com wrote:

NO ABSOLUTELY NOT. The reason is this makes architecture-independent source impossible without a #ifdef INTRNG.

Take a look at the first two commits of GitHub #1286. The first two create an interrupt_t type definition for things which only handle pointers to the architecture type, but don't directly examine the contents. If you have to #include sys/intr.h, then it is literally impossible to write truly machine-independent beyond having a void *.

An as yet not yet ready patch series proposes adding:

#include <machine/a_bikeshed_string_for_sed_to_target.h>

intr_event_create_device(struct intr_event **event, device_t pic, interrupt_t *source, u_int irq, int flags, const char *fmt, ...);

To <sys/interrupt.h>. If you require #include <sys/intr.h> then this degrades the quality as you must do one of two things:

#ifdef INTRNG
#include <sys/intr.h>
#else
#include <machine/intr_machdep.h>
#endif

intr_event_create_device(struct intr_event **event, device_t pic, interrupt_t *source, u_int irq, int flags, const char *fmt, ...);

Or else:

intr_event_create_device(struct intr_event **event, device_t pic, void *source, u_int irq, int flags, const char *fmt, ...);

The former means it is no longer truly architecture-independent, the latter compromises type-safety. This is a BAD idea!

You already need to #ifdef INTRNG for machine/intr.h, doing it for sys/intr.h is no worse in that regard. So kindly calm down.

[jrtc27]

In D47002#1071781, @ehem_freebsd_m5p.com wrote:

In D47002#1071690, @imp wrote:

This is the more typical pattern.

See sys/endian.h including machine/endian.h for but one of many examples (though that example is a bit convoluted in spots due to slight differences between endian.h, sys/endian.h and machine/endian.h consumer expectations.

This is the more typical pattern since most things in sys are truly machine-independent. sys/intr.h though is not machine-independent, as neither PowerPC nor x86 currently use it.

sys/sys/ioctl_compat.h is precedent.

[ehem_freebsd_m5p.com]

In D47002#1071782, @jrtc27 wrote:

You already need to #ifdef INTRNG for machine/intr.h, doing it for sys/intr.h is no worse in that regard. So kindly calm down.

Which is why GitHub #1286 first proposes settling on a common header name, then proposes the common type. No, I am not going to be calm when discussion on a topic I brought up is hidden from me by moving it elsewhere and I realize how bad of an idea this is!

[imp]

This is not a bad idea. In fact, it's quite a good idea.

If you need a type that's really MI, define it in a MI header (say machine/types.h or invent machine/interrupt.h). End of problem. There's not even a need for #ifdef anything. Just include machine/interrupt.h which has a well defined interface (because you just defined it and it has no historical issues).

sys/intr.h and machine/intr.h are INTRNG. sys/interrupt.h and machine/interrupt.h would then be MI and if we have to juggle a few things to rework the middle layers of the interrupts to converge them to a common interface, I'm having trouble seeing why that would be a huge deal.

[imp]

In D47002#1071781, @ehem_freebsd_m5p.com wrote:

In D47002#1071690, @imp wrote:

This is the more typical pattern.

See sys/endian.h including machine/endian.h for but one of many examples (though that example is a bit convoluted in spots due to slight differences between endian.h, sys/endian.h and machine/endian.h consumer expectations.

This is the more typical pattern since most things in sys are truly machine-independent. sys/intr.h though is not machine-independent, as neither PowerPC nor x86 currently use it.

It defines the INTRNG interface, which is MI, even if if it's not used on all architectures. So new purely MI interfaces that are for both INTRNG and not INTRNG setups wouldn't use it. Ditto machine/intr.h.

[kevans]

In D47002#1071777, @ehem_freebsd_m5p.com wrote:

NO ABSOLUTELY NOT. The reason is this makes architecture-independent source impossible without a #ifdef INTRNG.

Ok

Take a look at the first two commits of GitHub #1286. The first two create an interrupt_t type definition for things which only handle pointers to the architecture type, but don't directly examine the contents. If you have to #include sys/intr.h, then it is literally impossible to write truly machine-independent beyond having a void *.

Where did someone dictate that you have to include sys/intr.h, and for what? With the little details you've given it sounds like you're imposing restrictions on yourself, there's no hard requirement on INTRNG to include machine/intr.h. I don't see anything in your change offhand that's prevented by INTRNG consumers wanting sys/intr.h instead of machine/intr.h. You can include just the header you actually need.

[ehem_freebsd_m5p.com]

In D47002#1071804, @imp wrote:

If you need a type that's really MI, define it in a MI header (say machine/types.h or invent machine/interrupt.h). End of problem. There's not even a need for #ifdef anything. Just include machine/interrupt.h which has a well defined interface (because you just defined it and it has no historical issues).

The previous time I proposed that, everyone complained about the header being too trivial. As such I'm now going for trying to place the small MI bits in the existing headers and trying to get them to a common name.

[ehem_freebsd_m5p.com]

In D47002#1071806, @kevans wrote:

In D47002#1071777, @ehem_freebsd_m5p.com wrote:

Take a look at the first two commits of GitHub #1286. The first two create an interrupt_t type definition for things which only handle pointers to the architecture type, but don't directly examine the contents. If you have to #include sys/intr.h, then it is literally impossible to write truly machine-independent beyond having a void *.

Where did someone dictate that you have to include sys/intr.h, and for what? With the little details you've given it sounds like you're imposing restrictions on yourself, there's no hard requirement on INTRNG to include machine/intr.h. I don't see anything in your change offhand that's prevented by INTRNG consumers wanting sys/intr.h instead of machine/intr.h. You can include just the header you actually need.

A very important piece is it creates typedef <architecture type> interrupt_t. This then allows portions which don't look inside the type to simply hold a pointer to the architecture type. In order to get interrupt_t in an architecture-independent fashion, there needs to be $some_header which you can include anywhere and get the typedef.

Previously sys/intrcompat.h was rejected. As such the alternative I've got is machine/$something.h, which means choosing between machine/intr.h, machine/intr_machdep.h, or machine/$something_else.h. This breaks that approach as the place to put typedef struct intr_irqsrc interrupt_t; is in sys/intr.h.

[imp]

If it breaks the Gordian Knot, let's create machine/interrupt.h to contain the APIs that are common between INTRNG and !INTRNG, lets do it. Let's see where we wind up, and we'll discover if it's too trivial or not.

But that doesn't make any difference for this commit.

[ehem_freebsd_m5p.com]

In D47002#1071817, @imp wrote:

If it breaks the Gordian Knot, let's create machine/interrupt.h to contain the APIs that are common between INTRNG and !INTRNG, lets do it. Let's see where we wind up, and we'll discover if it's too trivial or not.

I would be up for that, but I suspect it will end up looking very much how I've been suggesting all along. I would suggest machine/interrupt.h should not be restricted to the common bits, but is also be free to #include things which are WIP towards being common. In fact I would be inclined for such to simply include the entire interface as separating out the common bits is a separate step.

[ehem_freebsd_m5p.com]

I guess I'll have to leave it there then.

[ehem_freebsd_m5p.com]

I had thought that it might be better, if you were going to push this through quickly, to simply push past and deal with the aftermath. Issue is you've now taken long enough for me to start organizing my thoughts better.

I hope this is merely meant as a representation of a commit and not the full commit. The reason is the actual commit will be much larger. >95% of #include of the INTRNG headers is by #include <machine/intr.h>, hence this will need to change hundreds of files. The reason Github #1431, commit 2 could be so small, was it was addressing deviations from an existing consensus, rather than overturning a previous consensus.

#include <sys/intr.h> is inherently architecture-dependent. As including from sys is always the same no matter the architecture, that will always get the same header, which breaks on PowerPC and x86. Unless/until INTRNG manages to replace the interrupt infrastructures on PowerPC and x86, #include <sys/intr.h> will require a surrounding #ifdef INTRNG for architecture-independent files.

Presently, #include <machine/intr.h> could very readily be made architecture-independent. All that is needed is to rename the PowerPC and x86 intr_machdep.h headers to intr.h and modifying source files to match.

Presently, #include <machine/intr_machdep.h> could very readily be made architecture-independent. All that is needed is to rename the INTRNG-architecture intr.h headers to intr_machdep.h and modifying source files to match.

Presently, #include <machine/interrupt.h> could very readily be made architecture-independent. All that is needed is to rename the intr.h and intr_machdep.h headers to interrupt.h and modifying source files to match.

Presently, #include <machine/bikeshed.h> could very readily be made architecture-independent. All that is needed is to rename the intr.h and intr_machdep.h headers to bikeshed.h and modifying source files to match.

I am avoiding expressing a preference on which of these I prefer since I don't want to stir the pot. I simply want them to converge on any name since that is high value to me and expressing a preference is trivial. You might notice, Github #1286, commit 1 (alas, likely needs an update due to 2 months and silence). I've been trying to get progress on this trivial issue for more than 2 years.

D47002 increases the architecture interrupt framework divergence. Please keep an eye on convergence.

[jrtc27]

In D47002#1075850, @ehem_freebsd_m5p.com wrote:

I had thought that it might be better, if you were going to push this through quickly, to simply push past and deal with the aftermath. Issue is you've now taken long enough for me to start organizing my thoughts better.

I hope this is merely meant as a representation of a commit and not the full commit. The reason is the actual commit will be much larger. >95% of #include of the INTRNG headers is by #include <machine/intr.h>, hence this will need to change hundreds of files. The reason Github #1431, commit 2 could be so small, was it was addressing deviations from an existing consensus, rather than overturning a previous consensus.

#include <sys/intr.h> is inherently architecture-dependent. As including from sys is always the same no matter the architecture, that will always get the same header, which breaks on PowerPC and x86. Unless/until INTRNG manages to replace the interrupt infrastructures on PowerPC and x86, #include <sys/intr.h> will require a surrounding #ifdef INTRNG for architecture-independent files.

Presently, #include <machine/intr.h> could very readily be made architecture-independent. All that is needed is to rename the PowerPC and x86 intr_machdep.h headers to intr.h and modifying source files to match.

Presently, #include <machine/intr_machdep.h> could very readily be made architecture-independent. All that is needed is to rename the INTRNG-architecture intr.h headers to intr_machdep.h and modifying source files to match.

Presently, #include <machine/interrupt.h> could very readily be made architecture-independent. All that is needed is to rename the intr.h and intr_machdep.h headers to interrupt.h and modifying source files to match.

Presently, #include <machine/bikeshed.h> could very readily be made architecture-independent. All that is needed is to rename the intr.h and intr_machdep.h headers to bikeshed.h and modifying source files to match.

I am avoiding expressing a preference on which of these I prefer since I don't want to stir the pot. I simply want them to converge on any name since that is high value to me and expressing a preference is trivial. You might notice, Github #1286, commit 1 (alas, likely needs an update due to 2 months and silence). I've been trying to get progress on this trivial issue for more than 2 years.

D47002 increases the architecture interrupt framework divergence. Please keep an eye on convergence.

Please, stop sending walls of text full of bold and italic text. Nobody wants to read it, it's not a productive way to communicate and it just makes it tiring to interact with you.

[imp]

This is perfect. Please commit it.