Page MenuHomeFreeBSD

Add support for more interrupt types
Needs ReviewPublic

Authored by hselasky on Wed, Feb 12, 7:31 PM.

Details

Summary

Add support for more interrupt types, for example INTR_TYPE_LKPI, for use with the LinuxKPI.
Basically switch INTR_TYPE definitions from a bitmask to an enum.

Sponsored by: Mellanox Technologies

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 29329

Event Timeline

hselasky created this revision.Wed, Feb 12, 7:31 PM

This change looks good to me in general. The only problem I see that Mellanox will now won't have batched epoch. And this was one of the drivers that really benefited from it in our testing of batching epoch at interrupt level.

The Mellanox drivers currently manage this internally:
See: https://svnweb.freebsd.org/changeset/base/357294

Usually there are many packets processed per interrupt.

In the future we can also add a variant to the LinuxKPI which use INTR_TYPE_NET to allocate the interrupts.

jhb added a comment.Wed, Feb 12, 8:15 PM

I'm not really sure why we need more interrupt types? This seems like it is better named INTR_TYPE_NET_NOEPOCH or something anyway, and if that's the true purpose I think instead we might need to rethink the design of tying the network epoch to the interrupt type? For example, what if instead of teaching ithreads, task queues, etc. about the network epoch directly, we were to have drivers that want to coalesce the epoch do so explicitly in their ISR (this also would let them drop the epoch when doing voluntarily yielding which is one thing np@ asked about on IRC earlier) and either use an alternate if_input or a new if_flags to avoid the recursion in ether_input? I think that approach would be fairly simple while avoiding hardcoding knowledge about the network epoch in various kernel subsystems.

In D23654#519204, @jhb wrote:

I'm not really sure why we need more interrupt types? This seems like it is better named INTR_TYPE_NET_NOEPOCH or something anyway, and if that's the true purpose I think instead we might need to rethink the design of tying the network epoch to the interrupt type? For example, what if instead of teaching ithreads, task queues, etc. about the network epoch directly, we were to have drivers that want to coalesce the epoch do so explicitly in their ISR (this also would let them drop the epoch when doing voluntarily yielding which is one thing np@ asked about on IRC earlier) and either use an alternate if_input or a new if_flags to avoid the recursion in ether_input? I think that approach would be fairly simple while avoiding hardcoding knowledge about the network epoch in various kernel subsystems.

I see, but do you agree that using one bit for every type of interrupt is a waste of bits and belongs to the times of interrupt priority levels (splxxx())?

The Mellanox drivers currently manage this internally:
See: https://svnweb.freebsd.org/changeset/base/357294
Usually there are many packets processed per interrupt.

I've seen this change. Was it benchmarked? I'm afraid result might be different to what was before, cause batching inside single call into interrupt handler be definition is a smaller batching than on top of it.

The Mellanox drivers currently manage this internally:
See: https://svnweb.freebsd.org/changeset/base/357294
Usually there are many packets processed per interrupt.

I've seen this change. Was it benchmarked? I'm afraid result might be different to what was before, cause batching inside single call into interrupt handler be definition is a smaller batching than on top of it.

I can ensure you that the batching of interrupts is not useful for Mellanox devices, because we ARM the IRQ after emptying the CQ. It is not running freely, like with other cards.

jeff added a comment.Thu, Feb 13, 10:24 AM

I've seen this change. Was it benchmarked? I'm afraid result might be different to what was before, cause batching inside single call into interrupt handler be definition is a smaller batching than on top of it.

epoch is 8-12ns to enter depending on the architecture and assuming it's in cache. If you got 100 packets in your batch it's now less than a cycle per-packet.

It's really going to be much more damaging to stay in epochs for too long, especially with pcbs covered.

kib added a comment.Thu, Feb 13, 1:47 PM
In D23654#519204, @jhb wrote:

I'm not really sure why we need more interrupt types? This seems like it is better named INTR_TYPE_NET_NOEPOCH or something anyway, and if that's the true purpose I think instead we might need to rethink the design of tying the network epoch to the interrupt type? For example, what if instead of teaching ithreads, task queues, etc. about the network epoch directly, we were to have drivers that want to coalesce the epoch do so explicitly in their ISR (this also would let them drop the epoch when doing voluntarily yielding which is one thing np@ asked about on IRC earlier) and either use an alternate if_input or a new if_flags to avoid the recursion in ether_input? I think that approach would be fairly simple while avoiding hardcoding knowledge about the network epoch in various kernel subsystems.

I support this, and I do not like the injection of the network subsystem into lower layers of the kernel.

sys/sys/bus.h
278

Would be nice to fix these to hex values while there.